Repository navigation
Conversation
…rompt inputs Native slash commands (/skill, /plan, /queue), the /thread starter and voice-channel input are human turns, so each re-pins the session-context prompt and the channel prompt. They built their event without the fields on_message sets: - a DM slash turn had no chat name, so the "origin" line rendered the channel id instead of the user's name; - a thread slash turn and the /thread starter dropped the parent channel from the thread label, and the starter took the parent channel's topic; - a spoken turn named the speaker by numeric id, not display name; - none carried the channel's bound skill, so a session opened by one of them never loaded it. Each switch between one of these turns and a typed message re-rendered the pinned context prompt and back again, a prompt-cache miss each way. They now use the same formatters (_format_thread_chat_name, _get_effective_topic on the thread, the member display name) and _resolve_channel_skills as the message path.
|
Independent verification on the PR head (c2727ba): discord suites 19/19 green across 2 files on Linux. Slash, thread, and voice turns carrying message-turn prompt inputs is intact. No findings. |
alt-glitch
added
type/bug
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Review: LGTM on correctness; approving the code. Merge is Teknium's call (no merge from me).
Premise verified on origin/main @ 8d30c4ea: _handle_message builds a DM's chat_name from message.author.name, a thread's from _format_thread_chat_name, and passes auto_skill=self._resolve_channel_skills(...); _build_slash_event did none of that and _dispatch_thread_session derived name/topic/parent from the interaction's channel rather than the thread. _ephemeral_change_key keys the pin on chat_name, chat_topic, user_name (gateway/run_agent_cache.py::_ephemeral_change_key), so each message↔slash/voice switch really did re-render the pinned context prompt. Same class as #128722 (Slack), same shape of fix.
What I checked
- The new helpers you route through (
_format_thread_chat_name,_get_parent_channel_id,_get_effective_topic(thread, is_thread=True)) are exactly what the message path calls on the same channel object, so the derived fields match by construction, not by coincidence. _thread_creatednow returning theThreadobject:_create_thread's dict has one consumer (_handle_thread_create_slash), nothing serializes it;test_discord_thread_slash_expired_defermocks the dict withoutthreadbut never passes a starter, soresult["thread"]isn't reached.- Voice
auto_skill: consumed only on a new session (run_turn.py:if _is_new_session and _auto), so a spoken turn that opens the session loads the bound skill and later turns are unaffected. Correct. - Tests: red on base (5 failed, the 4
assert 2 == 1pin flips + the channel row'sNone == ['triage']), green on head. 63 Discord/voice files: 415 passed, 0 failed, 6 skipped. Ruff and the Windows-footgun check clean.
Known residual (not blocking, you already flagged it): _ephemeral_change_key also keys "1" if src.message_id else "0" for Discord; a slash/voice turn has no message id, so with the opt-in discord toolset loaded the "Triggering message" line still differs between typed and slash turns and the pin flips on that axis. That is a renderer decision (c0c76a47), fine as a follow-up.
Live repro: before — the parity test's message→slash→message through the real _pinned_session_context_prompt yields 2 distinct pinned prompts on main; after — 1.
Approved CI runs for head c2727ba5.
| guild = getattr(adapter, "_client", None) and adapter._client.get_guild(guild_id) | ||
| member = guild.get_member(int(user_id)) if guild else None | ||
| display_name = getattr(member, "display_name", None) | ||
| user_name = display_name if isinstance(display_name, str) and display_name else str(user_id) |
There was a problem hiding this comment.
Note for the record: guild.get_member() is cache-only, so a speaker who isn't in the member cache still gets the numeric id here and the typed↔spoken pin flip survives for them. In practice voice-state updates populate the cache for anyone in the voice channel, so this is fine; just don't read the fallback as parity.
There was a problem hiding this comment.
Addressed in b993d20b24: on a member-cache miss, the speaker who ran /voice join keeps the name bound at join, so the pin no longer flips for them. Any other uncached speaker still gets their id and never borrows the joiner's name. The speaker-uncached row pins both.
| text=text, message_type=msg_type, source=source, raw_message=interaction, | ||
| channel_prompt=self._resolve_channel_prompt(channel_id, parent_id or None), | ||
| # Bound skills load only when a session starts, and "/skill x" or "/queue" can start one. | ||
| auto_skill=self._resolve_channel_skills(channel_id, parent_id or None), |
There was a problem hiding this comment.
This is the user-visible half of the fix (a session opened by /skill x or /queue never loaded the channel's bound skill). run_turn.py loads auto_skill only when _is_new_session, so this can't double-load on an existing session. Good.
There was a problem hiding this comment.
P0 review — Discord turn parity remains a partial repair
This same-head reconciliation consolidates the existing F1–F3 review and F4–F6 second pass. All six changed-RIGHT findings remain in place; no duplicate inline charges are added.
SSA map · Diagrams · Repair ledger · Verification and composition · Acceptance
P0 reconciliation — field parity is repaired selectively; the turn contract remains open
The designated P0 remains unchanged. John Paul Soliva's patch repairs specific Discord event fields; that is not equivalent to closing the complete admission, conversation ownership, instruction-selection and prompt-stability contract. The six existing findings remain part of this review. Acknowledgement, an approved head, a separate open PR and a promised follow-up are different states from an implemented and verified repair.
This reconciliation supplements the existing F1–F6 record without replacing those comments or treating earlier test runs as fresh execution. In particular, F6 receives a defensive source/lifecycle audit here, not a new cross-conversation reproduction. Runtime evidence from its earlier review remains attributed to that review.
Revision custody
| Object | Exact identity |
|---|---|
| Reviewed head | c2727ba500f22249dd729aba6365af3bb3c5e970 |
| True merge base and sole parent | 5dc1c084e932d6c2aa05713e55765f6c01ebe71c |
PR response's base.sha snapshot |
38d3dbce4adbb9074a34214eb330237a129e5904 |
Independently fetched live refs/heads/main snapshot |
12e4d3e2dd5281adf289f70e5d6c7f33427d6982 |
| Reviewed change | One commit; gateway/run_voice.py, plugins/platforms/discord/adapter.py, and two Discord test files |
| Existing discussion census | Nine submitted reviews, eight review threads, fourteen inline objects, four PR conversation comments, and the linked issue's one comment; both thread pagination levels terminate |
The original triage used 8d30c4ea, but that is not this PR's true merge base. A direct main-ref lookup also differs from the PR response's base.sha; both are recorded rather than silently treating either as the implementation baseline. Git confirms the same true merge base against the independently fetched main ref. Among the ten inspected production files, that main snapshot differs from the API-base snapshot only by an auto-thread title-retry addition in run_turn_runner.py; it does not change the six controlling mechanisms at this reviewed head. The local related-PR composition starts at the reviewed head, not the moving-main snapshot. The linked issue reports isolated reproduction, not a captured production request trace or measured provider-cache telemetry.
Subsystem structure and effect boundaries
Component and transition link map · Finding ledger · Verification
The relevant unit is a human turn in one resolved conversation, not a builder returning matching strings. Four producers converge on the gateway, but they acquire metadata, authority and state at different times.
| Boundary | Producer, owner and lifetime | Actual consumer and controlling source |
|---|---|---|
| Typed input | Discord message carries current member and channel objects. Message admission produces the per-event role flag. Auto-threading can change the destination before source construction. | Admission, routing, source and event. |
| Native slash | Interaction authorization returns a boolean. The event builder independently reconstructs a source and now resolves channel skills. | Slash gate, builder. Passing the first gate does not carry a grant into the second. |
| Created-thread starter | The created Discord thread object, rather than the interaction's parent channel, now supplies the thread's name, topic and parent. | Thread dispatch, shared metadata helpers. |
| Voice binding | Adapter-owned dictionaries are indexed by guild. The join command records the text-channel ID and serialized source once; these are connection-lifetime memory, not the durable transcript. | Binding writer, adapter binding and teardown. |
| Voice source | Current speaker name comes from the receiving bot's guild member cache. Other metadata comes from the join snapshot. Transport provenance is restored and routing identity recalculated. | Source and callback. Re-resolving speaker identity is a healthy control, but does not refresh conversation metadata or establish capture-time ownership. |
| Second admission gate | The gateway consumes a current source and accepts a delegated role grant only when its flag is literally True. |
Gateway admission, principal check. |
| Binding selection | Configuration belongs to the receiving adapter. Prompt lookup orders exact channel before parent; skill lookup at this head instead scans configuration order across both IDs. | Both resolvers. |
| Instruction loading | auto_skill is an event field. The actual payload is loaded into the first human message only when session creation/reset is recognized; it is not a continually applied system-prompt field. |
New-session decision and payload loader, consumer gate. |
| Prompt pins | Conversation state holds context bytes/key and channel inputs. A human event updates them; internal events can reuse them. A differing human source is deliberately re-rendered. | Pin ownership and invalidation. The pin cannot infer that a producer supplied stale metadata. |
| Durable recovery | Before agent execution, the coherent context/channel pair is persisted against an expected session ID. Internal restart recovery can adopt it. This stores what was supplied, not a canonical replacement for a bad producer. | Persist before execution, fenced storage. |
| Agent effect | Context, channel prompt and channel override are combined. The resulting bytes participate in the agent-cache signature and are handed to a fresh agent when reuse fails. | Combination, signature and agent selection. Local prompt differences establish changed inputs, not measured billing or remote cache misses. |
Diagram node/edge register and reverse finding links
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TD
subgraph CORE[" "]
direction TB
M["Typed Discord message"] --> MB["Current metadata and admission result"]
S["Native slash or created-thread starter"] --> SB["Interaction gate then source builder"]
J["Voice join"] --> B[("Adapter memory: bound channel and source")]
V["Completed speech transcript"] --> VB["Voice source reconstruction"]
B --> VB
MB --> G["Gateway identity and admission"]
SB --> G
VB --> G
G --> E["Admitted human MessageEvent"]
E --> SK["New session only: load bound skill payload"]
E --> P["Context and channel pins"]
P --> D[("SessionStore: fenced prompt snapshot")]
D -. "internal restart recovery" .-> P
P --> C["Combined ephemeral prompt"]
C --> A["Agent signature: reuse or rebuild"]
SK --> T["User text and transcript"]
E --> T
T --> R["Agent run with selected instance"]
A --> R
end
classDef entry fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px
classDef authority fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
classDef transition fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
classDef durable fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa,stroke-width:2px
classDef success fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
classDef failure fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
classDef context fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px
class M,S,J,V entry
class MB,SB,VB,SK,C,R transition
class G,A authority
class B,P,T context
class D durable
class E success
click M href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L6111-L6197" "Pinned implementation; companion paths in SSA register"
click MB href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L6111-L6197" "Pinned implementation; companion paths in SSA register"
click S href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L4700-L4743" "Pinned implementation; companion paths in SSA register"
click SB href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L4700-L4743" "Pinned implementation; companion paths in SSA register"
click J href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_voice.py#L148-L185" "Pinned implementation; companion paths in SSA register"
click B href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L1089-L1109" "Pinned implementation; companion paths in SSA register"
click V href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L3850-L3918" "Pinned implementation; companion paths in SSA register"
click VB href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_voice.py#L214-L306" "Pinned implementation; companion paths in SSA register"
click G href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_inbound.py#L205-L299" "Pinned implementation; companion paths in SSA register"
click E href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/platforms/event.py#L35-L90" "Pinned implementation; companion paths in SSA register"
click P href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L665-L738" "Pinned implementation; companion paths in SSA register"
click D href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/session_prompt_pin.py#L49-L81" "Pinned implementation; companion paths in SSA register"
click SK href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_turn.py#L512-L603" "Pinned implementation; companion paths in SSA register"
click T href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_turn.py#L2087-L2139" "Pinned implementation; companion paths in SSA register"
click C href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_turn_runner.py#L1839-L1853" "Pinned implementation; companion paths in SSA register"
click A href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L99-L140" "Pinned implementation; companion paths in SSA register"
click R href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_turn_runner.py#L1685-L1727" "Pinned implementation; companion paths in SSA register"
CORE ~~~ LEGEND
LEGEND["Legend<br/>Blue: entry / incoming<br/>Purple: ownership / policy<br/>Amber: transition / pending<br/>Teal: durable state<br/>Green: success<br/>Gray: context / transient"]
style CORE fill:#0d1117,stroke:#30363d,color:#e6edf3
style LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
Color key: blue = entry points and incoming observations; purple = ownership and policy checks; amber = transitions and pending work; teal = durable state; green = successful outcomes; red = failures; gray = contextual or transient state.
The diagram is a source-derived dependency model. The execution ledger below identifies which edges were actually exercised; provider and Discord network operation are not implied by an arrow.
Root cause and failed controls
There is no single authoritative event-normalization boundary. Typed, native slash, created-thread and voice input each assemble a source. Shared formatters reduce lexical drift, but field equality still depends on their arguments, metadata age, admission proof and resolved conversation. This patch improves three producers while leaving different ownership errors downstream and alongside them.
- F1 — P1, incomplete voice inheritance: both voice resolvers receive only the text-channel ID, although the stored source retains its parent. The existing prompt omission remains; the new skill assignment has the same missing-parent limitation. Exact-channel success cannot disprove a parent-only failure. A shared parent-aware lookup is required, together with F3's precedence repair.
- F2 — P1, inherited snapshot-lifetime mismatch: source serialization freezes names and topic at join time. The new speaker refresh changes only the user fields. Typed messages observe current channel metadata while speech returns to the old snapshot. The pin is behaving as written when it re-renders; disabling invalidation would hide real edits rather than repair the producer.
- F3 — P2, inherited resolver defect newly consumed by slash: treating exact and parent IDs as an unordered matching set makes the first matching configuration row win. Passing the parent everywhere without repairing the resolver produces consistent but wrong instruction selection.
- F4 — P1, inherited grant-handoff failure: the adapter's successful admission decision and the gateway's accepted per-event grant are separate states. Slash and thread sources omit the flag. The serialized voice source intentionally does not preserve it. The repair must carry or recompute a current-event decision, not make authorization durable or copy the joiner's authority.
- F5 — P2, inherited deduplication scope error: the callback preserves its receiving adapter, but the runner-wide transcript store is keyed only by guild and speaker. Correct transport provenance after reconstruction cannot recover work already discarded by another transport's dedup state.
- F6 — P1, inherited asynchronous ownership gap: capture processing and later dispatch do not share an immutable binding identity. The existing record reports wrongly associated work; the fresh source audit confirms the ownership interface still lacks that invariant. A generation-bound capture/dispatch contract and explicit stale-work policy are required. No new exploit, cross-conversation race or live privacy-impact experiment was performed here.
F1, F2 and F3 are linked but not duplicates: missing inheritance, stale metadata and wrong precedence need different repairs. F4 and F5 can prevent an otherwise well-formed event from reaching any prompt consumer. F6 decides which conversation owns the work; improving the current conversation's labels does not settle that ownership.
Why the existing pins do not repair F2
Participants: current metadata N01 · voice snapshot N04 · pins N09 · agent signature N14. Every transition and source · F2 disposition.
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TD
subgraph CORE[" "]
direction TB
J["Join: store channel metadata"] --> B[("Voice binding snapshot")]
J --> E["Legitimate channel rename or topic edit"]
E --> T["Typed turn: current metadata"]
T --> P1["Prompt pins: current context bytes"]
P1 --> A1["Agent signature: current bytes"]
A1 --> S["Next spoken turn"]
B -->|"join-time metadata"| S
S --> P2["Prompt pins: old context replaces current"]
P2 --> A2["Agent signature: old bytes"]
A2 --> T2["Next typed turn: current metadata"]
T2 --> P3["Prompt pins: current bytes restored"]
P3 --> A3["Agent signature: current bytes restored"]
P2 -.-> N["Pinning detects different inputs;<br/>cannot identify their age"]
B -.-> R["Required repair: refresh metadata<br/>for the established owner"]
end
classDef entry fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px
classDef authority fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
classDef transition fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
classDef durable fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa,stroke-width:2px
classDef success fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
classDef failure fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
classDef context fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px
class J,E,T,S,T2 entry
class B,P1,A1 context
class P2,A2 failure
class P3,A3 success
class N authority
class R transition
click J href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_voice.py#L148-L185" "Pinned implementation"
click B href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L1089-L1109" "Pinned implementation"
click E href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L6111-L6197" "Pinned implementation"
click T href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L6111-L6197" "Pinned implementation"
click P1 href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L665-L738" "Pinned implementation"
click A1 href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L99-L140" "Pinned implementation"
click S href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_voice.py#L214-L306" "Pinned implementation"
click P2 href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L665-L738" "Pinned implementation"
click A2 href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L99-L140" "Pinned implementation"
click T2 href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L6111-L6197" "Pinned implementation"
click P3 href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L665-L738" "Pinned implementation"
click A3 href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L99-L140" "Pinned implementation"
click N href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L665-L738" "Pinned implementation"
click R href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_voice.py#L214-L306" "Current reconstruction owner; repair obligation described in F2"
CORE ~~~ LEGEND
LEGEND["Legend<br/>Blue: entry / incoming<br/>Purple: ownership / policy<br/>Amber: transition / pending<br/>Green: success<br/>Red: failure<br/>Gray: context / transient"]
style CORE fill:#0d1117,stroke:#30363d,color:#e6edf3
style LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
One legitimate metadata edit may require one new context render. Repeated typed/spoken reversions after that edit are a different phenomenon. Idle cache eviction, rebuilding the agent or saving the pin cannot normalize a stale source. Rejoining can replace a snapshot, but is not a durable repair and does not establish ownership for work already being processed. A new-session reset reopens skill loading; an ordinary later turn does not retroactively supply a missing first-turn skill.
Distinct state lifetimes
State-owner and transition register · durable controls V3 · acceptance obligations
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TD
subgraph CORE[" "]
direction TB
CFG[("Adapter configuration")] --> LOOK["Binding lookup<br/>Prompt ordered, skill order-dependent"]
LIVE["Current channel and member metadata"] --> EVENT["One human event"]
JOIN[("Connection-lifetime join snapshot")] --> EVENT
LOOK --> EVENT
EVENT --> NEW{"New session?"}
NEW -->|yes| PAY["Skill payload appended to user text"]
NEW -->|no| HIST["Existing history retained"]
EVENT --> PIN[("Conversation prompt pins")]
PIN --> STORE[("Durable expected-session snapshot")]
STORE -. "internal recovery only" .-> PIN
RESET["Conversation reset"] --> NEW
RESET --> CLEAR["Clear conversation state"]
CLEAR --> PIN
end
classDef entry fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px
classDef authority fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
classDef transition fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
classDef durable fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa,stroke-width:2px
classDef success fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
classDef failure fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
classDef context fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px
class CFG,HIST,STORE durable
class LIVE,EVENT,RESET entry
class LOOK,PAY,CLEAR transition
class NEW authority
class JOIN,PIN context
click CFG href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/platforms/base.py#L1810-L1844" "Pinned implementation; companion paths in SSA register"
click LOOK href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/platforms/base.py#L1810-L1844" "Pinned implementation; companion paths in SSA register"
click LIVE href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L6111-L6197" "Pinned implementation; companion paths in SSA register"
click JOIN href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/plugins/platforms/discord/adapter.py#L1089-L1109" "Pinned implementation; companion paths in SSA register"
click EVENT href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/platforms/event.py#L35-L90" "Pinned implementation; companion paths in SSA register"
click NEW href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_turn.py#L512-L603" "Pinned implementation; companion paths in SSA register"
click PAY href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_turn.py#L2087-L2139" "Pinned implementation; companion paths in SSA register"
click HIST href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_turn.py#L2087-L2139" "Pinned implementation; companion paths in SSA register"
click PIN href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L665-L738" "Pinned implementation; companion paths in SSA register"
click STORE href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/session_prompt_pin.py#L49-L81" "Pinned implementation; companion paths in SSA register"
click RESET href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L352-L379" "Pinned implementation; companion paths in SSA register"
click CLEAR href "https://github.com/NousResearch/hermes-agent/blob/c2727ba500f22249dd729aba6365af3bb3c5e970/gateway/run_agent_cache.py#L352-L379" "Pinned implementation; companion paths in SSA register"
CORE ~~~ LEGEND
LEGEND["Legend<br/>Blue: entry / incoming<br/>Purple: ownership / policy<br/>Amber: transition / pending<br/>Teal: durable state<br/>Gray: context / transient"]
style CORE fill:#0d1117,stroke:#30363d,color:#e6edf3
style LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
SessionSource.to_dict/from_dict preserves labels and optional message/parent IDs but deliberately excludes the per-event role grant (wire layout). The voice binding and durable prompt snapshot are not interchangeable stores. The reset funnel clears conversation state (reset implementation); it does not change how any Discord producer builds its next event.
Test adequacy
The supplied tests do traverse the real prompt-pin functions and check one context/channel prompt pair across typed → other → typed. They are useful regression tests for the fields changed here, not comprehensive turn-contract tests.
Their adapter fixture replaces Discord channel classes, binds each tested ID directly, disables auto-threading and omits message.guild from the typed fake. The slash cases replace authorization with success. The voice case seeds the binding from a slash event and substitutes gateway authorization. It does not exercise the join writer, a post-join edit, parent-only voice configuration, competing binding order, a second receiving adapter, restart recovery or a provider request.
The assertions also do not execute the new-session payload-loading consumer, agent reuse/rebuild or persisted-pin readback. Those boundaries require their own evidence; a passing auto_skill field assertion does not prove that the payload was applied. Default top-level routing is a separate equivalence class: a typed mention can create a thread while native slash remains in its invocation channel. Parity must be compared after resolving the same conversation, not assumed from equal user text.
The opted-in Discord-tools renderer still conditions its trigger-message note on message-ID presence (renderer, pin-key axis). That acknowledged residual is not closed by a tools-unavailable control. A cache-only member lookup can still fall back to the numeric speaker ID; neither its existence nor absence of an incident trace establishes how often that fallback occurs.
SSA link map — components, transitions, findings and proof
Navigation: Architecture and state owners · Producer/effect diagram · Metadata sequence · State lifetimes · Finding ledger · Verification ledger · Acceptance.
The node IDs below identify every participant/component in the diagrams, including conceptual boundary boxes. A node's source link resolves to its real implementation rather than a broad file landing page. Transition rows link both ends and the controlling call/assignment. Evidence links lead to the explicitly bounded verification record; they do not imply that every source edge was dynamically exercised.
Node register
| Node / diagram labels | Pinned controlling implementation | Findings and verification / closure |
|---|---|---|
N01 — Typed input and current metadata. Diagram 1 M, MB; diagram 2 C; diagram 3 LIVE. |
Message admission, typed source/event construction, metadata helpers. | F2, F4; V1/V2. Must resolve the same conversation before parity comparisons. |
N02 — Native interaction and source builders. Diagram 1 S, SB. |
Slash authorization, slash builder, created-thread builder. | F3, F4; V1/V2/V4. Native admission and downstream acceptance are separate predicates. |
N03 — Voice join. Diagram 1 J. |
Join command and binding publication. | F1, F2, F6; V2/V6. The current join writer does not establish immutable ownership for pending audio. |
N04 — Connection-lifetime binding snapshot. Diagram 1 B; diagram 2 B; diagram 3 JOIN. |
Dictionary initialization, writer, serialization layout, leave/cleanup. | F2, F4, F6; V2/V6. Distinct from durable prompt storage, N10. |
N05 — Completed speech processing. Diagram 1 V. |
Listen loop and transcription callback. | F6; V6 is source-only in this pass. Ownership must survive asynchronous processing; no new race reproduction is claimed. |
N06 — Voice reconstruction and pre-dispatch controls. Diagram 1 VB. |
Source reconstruction, routing, admission, deduplication, bindings and dispatch. | F1, F2, F4, F5, F6; V2/V4/V6. Reconstructed transport identity is not a substitute for each of those controls. |
N07 — Gateway identity and admission. Diagram 1 G. |
Gateway intake gate, principal-grant consumer. | F4; V2/V4. A source can be well formatted yet refused before session preparation. |
N08 — Admitted human event. Diagram 1 E; diagram 3 EVENT. |
MessageEvent shape/defaults, admission result, prepare-turn caller. | F1–F6; V1/V2. Voice is a human event (internal=False), not an internal wake exempt from human pin updates. |
N09 — Context/channel pins. Diagram 1 P; diagram 2 P; diagram 3 PIN. |
Human update versus internal reuse, conversation-owned fields. | F1, F2; V2/V3. A matching key reuses bytes; divergent human inputs intentionally replace them. |
N10 — Durable prompt snapshot. Diagram 1 D; diagram 3 STORE. |
Snapshot persistence and rehydration, expected-session read/write fence. | F2 recovery boundary; V3. Preserves the supplied pin, not fresh Discord metadata. |
N11 — New-session skill loading. Diagram 1 SK; diagram 3 NEW, PAY, HIST. |
New-session decision and payload loader, event consumer gate. | F1, F3; V2. Existing history is the retention mechanism after the first-turn decision, not repeated injection. |
N12 — User text/history. Diagram 1 T; diagram 3 PAY, HIST. |
Payload prepends text, history load and inbound text preparation, turn-context fields. | F1, F3; V2/V3. These feed execution, not the agent configuration signature. |
N13 — Combined ephemeral prompt. Diagram 1 C. |
Context + channel prompt + channel override combiner. | F1, F2; V3, including real combiner assertions. |
N14 — Agent signature and selection. Diagram 1 A; diagram 2 A. |
Signature inputs, cache lookup/fresh-agent construction. | F1, F2; V2/V3. Consumer code is traced; no live provider cache-hit or billing claim. |
N15 — Actual agent execution boundary. Diagram 1 R. |
Agent invocation. | Final effect boundary for F1–F6; V6. Provider execution is not exercised by these isolated review probes. |
N16 — Adapter configuration and binding lookup. Diagram 3 CFG, LOOK. |
Adapter resolver wrappers, prompt/skill lookup algorithms. | F1, F3; V1/V2/V4. Prompt precedence is ordered; the head's skill precedence is configuration-order-dependent. |
N17 — Conversation reset and clearing. Diagram 3 RESET, CLEAR. |
Conversation-state funnel, fresh-session flag consumption. | Recovery consideration for F1, F2; V3. Resets clear pins and reopen loading; they do not change producer correctness. |
Transition register
Grouped rows enumerate every diagram arrow. Diagram IDs disambiguate repeated shorthand labels; reverse links lead back to both state owners.
| Diagram edges | Controlling call, assignment or data dependency | Evidence and obligation |
|---|---|---|
D1 M→MB; D2 C→C |
N01 obtains current typed metadata after message admission and uses current channel/member fields. The D2 self-edge denotes an external metadata edit, not a product mutation performed by this PR. | F2, V2; transport is substituted. |
D1 S→SB |
N02: interaction authorization precedes its event builders. | F4, V1/V2/V4. |
D1 J→B; D2 C→B |
N03 writes N04 through the join-time assignment and serialization. D2 summarizes the same source snapshot. | F2, F6, V2/V6. |
D1 V→VB, B→VB; D3 JOIN→EVENT, LIVE→EVENT |
N05 calls N06; reconstruction reads N04, refreshing speaker but not channel metadata in the voice path. Typed N01 reads current metadata. | F2, F6, V2/V6. |
D1 MB→G, SB→G, VB→G, G→E |
Producer events from N01, N02, N06 converge on N07; gateway admission returns an admitted N08. Voice also has its earlier admission/dedup checks. | F4, F5, V2/V4; admission success is not model execution. |
D1 E→SK; D3 EVENT→NEW, NEW→PAY, NEW→HIST |
N08 reaches N11: new-session decision, followed by conditional payload loading. A non-new turn proceeds to existing history. | F1, F3, V2. |
D1 SK→T, E→T; D3 PAY and HIST outcomes |
N11 adds skill text to N12 at the payload consumer; ordinary user text/history also enter the turn. | F1, F3, V2/V3. |
D1 E→P; D3 EVENT→PIN |
N08 updates N09 through prepare-turn context pinning and channel pinning before run. | F1, F2, V2/V3. |
D1 P→D, D⇢P; D3 PIN→STORE, STORE⇢PIN |
N09 and N10: persist/rehydrate methods, same-session storage fence, and internal-only rehydrate caller. | V3 and real durable assertions. Healthy recovery does not close F2. |
D1 P→C, C→A; D2 P→A three times |
N09 supplies N13, then N14: combination, signature/selection. D2 abbreviates this two-edge path. | F1, F2, V2/V3; provider effect remains bounded. |
D2 C→P twice, B→P |
Fresh typed N01 or stored voice N04, via N06, supplies N09; human pin updates treat differing metadata as an input change. | F2, V2. The diagram's repair note is an acceptance requirement, not implemented behavior. |
D1 T→R, A→R |
N12 supplies text/history and N14 supplies the selected instance to N15, via the invocation boundary. | V6: source-traced final boundary, no provider call. |
D3 CFG→LOOK, LOOK→EVENT |
N16's wrappers use shared algorithms; slash, thread, typed and voice attach their results to N08. | F1, F3, V1/V2/V4. |
D3 RESET→NEW, RESET→CLEAR, CLEAR→PIN |
N17 clears N09 using the reset funnel; fresh flags reopen N11's new-session decision. | V3; recovery cannot retroactively repair prior omitted instructions or change F2's source lifetime. |
Reverse finding map
| Existing finding | State owners / causal transitions | Repair evidence required |
|---|---|---|
| F1 | N03 join → N04 source → N06 voice → N16 selection → N11 payload / N13 combined prompt. | Acceptance A1, V2/V4. |
| F2 | N01 current metadata versus N04 snapshot, through N09 pins → N14 signature. | Acceptance A2, V2/V3. |
| F3 | N16 resolver → N02 native event / N06 voice → N11 payload. | Acceptance A1, V2/V4. |
| F4 | N02 native gate/source and N04 serialization / N06 reconstruction → N07 grant consumer. | Acceptance A3, V2/V4. |
| F5 | N06 callback's receiving adapter versus runner-owned dedup store; discard happens before N08 reaches consumers. | Acceptance A4, V2. |
| F6 | N03 binding publication, N04 lifetime, N05 asynchronous work, N06 dispatch. | Acceptance A5, V6. Fresh analysis remains source-only. |
Prior-claim and repair ledger
SSA state owners and transitions · Verification · Acceptance
Still present below refers to the reviewed head, not to whether a related branch contains a candidate repair. Fresh execution, source-only verification and earlier published runtime evidence are separated in the verification ledger. The original findings retain their identifiers and priorities.
| Claim and source | Current disposition | Deciding evidence / remaining acceptance obligation |
|---|---|---|
| Issue #128787: DM native slash loses the chat label; thread slash omits parent context. | Repair implemented at this head. | Shared message-path naming is now used. Supplied tests must be RED at the true parent and GREEN here. This does not cover the tools-enabled trigger-note axis. |
Same issue: /thread starter derives metadata from its invocation channel rather than the created thread. |
Repair implemented at this head. | The created thread object now supplies name/topic/parent. Both text-channel and forum-parent semantics matter; a text parent topic is not automatically a thread topic. |
| Same issue: voice uses the speaker's numeric ID and lacks a bound skill. | Partial repair. | Cached-member display name and exact-channel auto_skill are added. F1, F2, F4 and member-cache fallback remain distinct boundaries. |
| Commit message: “none carried the channel's bound skill.” | Overbroad for the /thread starter; source disproves that particular attribution. |
The parent's thread builder already resolves and attaches skills. The new /thread repair is metadata ownership, not introduction of its first skill field. Slash and voice really do add the missing field. |
| F1: parent-bound voice prompt/skill omitted. | Still present; partial repair, P1. | Both voice calls still omit the stored parent. Closure needs parent-only, exact-thread override and plain-channel controls through the new-session payload consumer, not just resolver-call assertions. SSA owners, selection, payload, V2. |
| F2: join-time conversation metadata survives legitimate edits. | Still present; inherited, P1. | Only speaker fields refresh. Closure needs current metadata for the established conversation, explicit cache-miss fallback and one post-edit context across typed/spoken switches. SSA snapshot, pin consumer, sequence, V2/V3. |
| F3: earlier parent binding overrides exact thread binding. | Still present at this head; inherited resolver defect newly used by slash, P2. | #129303 changes exact/parent precedence. A separate open branch is not a repair in this head. Acceptance requires both configuration orders plus exact-absent inheritance through the consumer. SSA resolver, payload, V2/V4. |
| F4: role-authorized slash/thread/voice turns lose their grant. | Still present; inherited, P1. | The native builders omit the flag; source serialization does not carry it; voice does not recompute it. #125620 addresses both native builders; #118966 is a slash-only alternative. Neither repairs spoken admission. Closure requires legitimately admitted native and voice turns to reach the gateway under their receiving adapter's policy. SSA builders, grant consumer, V2/V4. |
| F5: one receiving bot consumes another's deduplication budget. | Still present; inherited, P2. | Runner-wide key remains (guild_id, user_id) in the helper. Closure requires receiving-transport and binding scope while preserving same-binding duplicate suppression. SSA pre-dispatch controls, V2. |
| F6: asynchronous speech work lacks immutable conversation ownership. | Still present in source; inherited, P1; earlier runtime evidence remains historical. | This pass does not rerun the cross-conversation scenario. Binding identity/generation and stale-work rejection or retained-origin semantics remain the acceptance contract; label/name refresh alone is not closure. SSA join, binding, processing, dispatch, V6. |
| Author's first response: F1/F2 will be a follow-up; F3 is #129303. | Acknowledged, not repaired at reviewed head. | Source is unchanged. Promised implementation is not counted as code, nor is author acknowledgement counted as a new runtime test. |
| Author's second response: F4–F6 predate the patch and are deferred; native role handoff is already tracked. | Inherited attribution supported; closure still open. | Inheritance explains provenance, not acceptance. The reply explicitly says these cases were not rerun. A complete repair ledger includes all six findings and the distinct native/voice halves of F4. |
| PR body and first review: trigger-note rendering and message-ID presence are left to #127944. | Still present at this head; complementary work remains separate. | Renderer, pin key and stale source message ID are distinct from display-name normalization. Composition evidence is reported below, not assumed from a clean textual merge. |
| Cache-only name lookup note: a missing member falls back to numeric ID. | Conditional source-supported residual. | No claim that all speakers miss cache, and no live incidence estimate. Acceptance needs a defined same-speaker fallback without borrowing another participant's name. No duplicate inline finding was added. |
| Second review: default typed auto-thread routing can differ from native slash routing. | Valid scope limit, not a newly invented defect. | Message routing runs before its event builder. The supplied test disables auto-threading. Equality is meaningful for turns resolved to the same conversation; native routing policy still requires explicit coverage. |
Same review: earlier channel=None plus independently nonempty channel-ID fixture was not a faithful SDK interaction. |
Rejected fixture remains rejected; not a current finding. | Its history is preserved. The earlier actual-Interaction cache-miss result is prior evidence, not a freshly rerun SDK matrix. No unsupported cache-miss defect is resurrected. |
| Approval: derived fields match shared helpers “by construction.” | True only with equivalent objects, arguments and lifetimes. | F1 varies parent arguments; F2 varies metadata lifetime; F4 concerns admission before the consumer. Shared function names are insufficient to establish these equivalences. |
| Same approval: thread-result dictionary has one production consumer and no serialization boundary. | Source-supported at head. | Repository call-site search finds _handle_thread_create_slash as the only production caller of _create_thread; it forwards the new thread object to _dispatch_thread_session. The changed internal signature still needs sibling-test reconciliation. |
| Author's 434/6 result, approval's 415/6 result, earlier review's 419/2 result, and 19/19 “no findings” report. | Historical run claims, not interchangeable totals or class-closure evidence. | Different selections/platforms/substitutions are not normalized by matching a head SHA. Fresh selections and their exact outcomes appear below. Green selected tests do not negate the independent contract failures. |
| Earlier review CI statement: queued checks, no completed failure. | Superseded by fresh hosted state. | Completed required-check aggregate and three Desktop jobs now fail. Cancelled/action-required work is not a successful execution. Attribution of those Desktop failures to this patch remains unverified. |
| Issue triage: P1 rather than P0 because no data loss/outage. | Not adopted as a reclassification. | The live PR retains P0. Absence of a measured production incident does not establish a bound on accumulated cost or settle the recorded ownership finding. Subsidiary findings retain their explicit evidence-based priorities. |
| Conversation comment challenges the approval's authority. | Provenance verified; broader authority assertion not established by review prose. | The actual approval is authored by teknium1, with GitHub author_association=COLLABORATOR, state APPROVED, at this head. Neither its approval nor an unsupported account-authority assertion changes the repair evidence. This pass makes no maintainer-authority or merge decision. |
| PR body's cross-adapter claim: Telegram, Mattermost, Teams, Matrix, WeCom, LINE, DingTalk, Google Chat, WhatsApp and Feishu each have one turn-building path. | Not verified by this Discord review. | It is a broader census claim, not a prerequisite proved by these four changed files. Slack is a conceptual sibling; its separate PRs are not Discord repairs. |
Required acceptance evidence
The six original inline findings already exist on meaningful changed-RIGHT lines at this exact head. Their anchors and bodies are retained rather than duplicated. Closure requires:
- A1: Parent-aware voice prompt and skill selection, with exact-thread precedence independent of row order. F1, F3, SSA N16→N11, V2/V4.
- A2: Metadata refresh for the established conversation, including explicit missing-cache behavior, without reopening capture ownership. F2, SSA sequence, V2/V3.
- A3: Current-event native and spoken admission evidence carried through the actual gateway gate. F4, SSA N07, V2/V4.
- A4: Deduplication scoped to the receiving transport and binding lifetime, with normal same-binding suppression preserved. F5, SSA N06, V2.
- A5: Immutable ownership of asynchronous voice work, with an explicit stale-work outcome and verification before public closure. F6, SSA N03–N06, V6.
- A6: The acknowledged message-ID/renderer obligation resolved or explicitly left open with its precise scope; unrelated partial repairs cannot be substituted for it. SSA N09/N14, V4.
- A7: Same-pin regression and composition evidence, plus completed successful required CI. No label, approval or follow-up promise substitutes for these observations. Verification ledger, full SSA.
Verification ledger — observed results and explicit limits
SSA component and transition map · Findings · Acceptance
All repository tests used scripts/run_tests.sh through native Git Bash with isolated home/application/temp directories, one worker and zero file retries. Source-import probes bound imports to the exact worktrees. Functional probes used Win32 CPython 3.14.7, discord.py 2.7.1, pytest 9.1.1; composition and persistence controls used a separate read-only Win32 CPython 3.14.3 environment. Neither environment was installed into the live runtime. Different selections below are not pooled into a misleading “whole suite” total.
| ID / selection | True baseline | Reviewed head / local composition | What this establishes |
|---|---|---|---|
| V1a — Same-byte supplied parity probe | 5 failed, exit 1 | 5 passed, exit 0 | Genuine RED→GREEN for the supplied DM, channel, thread, starter and voice cases. Same archived test SHA-256 b3865fab2c934207dc11f6c6a61311600d0a8073e67f8100df2668ccca897ed9. |
| V1b — Focused existing repository tests | Seven existing files: 95 passed, 4 skipped, exit 0 | Eight files including the added parity file: 100 passed, 4 skipped, exit 0 | Selection is slash commands/auth, channel prompts/skills, role-DM scope, voice command and voice-mode platform isolation. The unequal file sets are explicit, not an A/B regression-count claim. |
| V2a — Same-byte F1–F5 functional probes | 1 passed, 6 failed, exit 1 | 1 passed, 6 failed, exit 1 | Matching failing node-ID sets. F3 has both a helper and an integrated node; six failed assertions are not six additional findings. Base's integrated slash case stops earlier on its missing skill field. |
| V2b — Actual new-session payload consumer | 2 passed, 2 failed, exit 1 | 3 passed, 1 failed, exit 1 | Exact-thread voice payload is repaired; parent-only new-session payload remains absent. Both ongoing-session controls pass. Real _hmwa_prepare_turn, _hmwa_open_session, skill reader and payload formatter execute; a deliberate stop occurs at the subsequent turn-lease boundary. |
V3a — test_session.py -k prompt_pin |
4 passed, exit 0 | 4 passed, exit 0 | Real local persistence/restart/reset/compression-route controls, including rejection of an old-session write. These are healthy store controls, not the Discord voice ownership reproduction. |
V3b — test_internal_event_pin_wiring.py |
12 passed, exit 0 | 12 passed, exit 0 | Real handler and combiner wiring with substituted SessionStore/final agent. Internal events preserve the human pin. Voice is not an internal event. |
| V4 — Five-file related-PR union | Not a baseline arm | Initial resolved-source union: 46 passed, 1 failed. After explicit test-interface adaptation: 47 passed, exit 0; a fresh five-file rerun also 47 passed. | A thread-test signature collision was real and had to be reconciled. The final synthetic union is not the reviewed head, moving main or hosted CI. |
| V5 — Hosted checks at reviewed head | No Desktop baseline replay | 57 check objects: 29 success, 15 skipped, 9 cancelled, 4 failure | Required aggregate and three Desktop leaves fail. Their association with the head is established; causation by this Discord diff is not. |
| V6 — Limits | Earlier published evidence kept separate | F6 fresh source-only; no live Discord/audio/provider operation, billing/cache telemetry, full-suite run or moving-main composition | Missing evidence remains visible. No production-incident root cause or complete repair is inferred. |
V2: observable functional consequences
The archived review_probe_functional.py has SHA-256 85bc225aeb916cb43ae596471cb0acdde17d20a967a322ce99fe814ddca64c4a. Identical bytes ran at both pins. Its seven executed nodes include the exact-first passing control and the following failing contracts:
| Finding / node suffix | Observed head result | Control and execution boundary |
|---|---|---|
F1, test_f1_voice_parent_inherited_prompt_and_skill |
Typed parent-bound thread event: ('Parent prompt', ['parent-skill']); spoken event: (None, None). |
Typed parent inheritance passes. The head's supplied exact-channel control passes. Real builders/resolvers/callback, fake channel objects and final dispatch. Binding is seeded through real source serialization rather than live join. SSA. |
F2, test_f2_voice_refreshes_bound_conversation_metadata |
After rename/topic edit, typed context is Hermes Server / #renamed / New topic; voice remains Hermes Server / #ops / Incident triage. Real typed→spoken→typed pins produce two distinct strings. |
No-edit name/topic equality passes. This fresh probe edits channel name and topic together; the earlier review's separate guild/thread edits remain historical evidence. SSA sequence. |
F3, test_f3_shared_resolver_precedence_independent_of_order[ids1] and test_f3_exact_thread_skill_wins_both_binding_orders |
Parent-first config yields parent-default; exact-first yields thread-specific. Head typed/native builders consume the wrong selection in the parent-first case. |
Exact-first helper and parent-only fallback controls pass. Base integrated slash fails earlier on auto_skill=None; the helper pair isolates the inherited precedence defect. SSA. |
F4, test_f4_legitimate_role_member_adapter_accepts_gateway_rejects |
The legitimate member's real adapter gate accepts the command; typed grant passes gateway authorization, native slash fails it, and the spoken callback emits no additional dispatch. | Matching guild/member role fixture, no pairing grant and no allow-all. Actual authorization predicates run; the full gateway command handler and thread-starter case are not freshly executed in this probe. The related composition does exercise the repaired native thread grant. SSA. |
F5, test_f5_two_registered_bots_each_receive_same_utterance |
Bot A receives the original utterance. Bot B receives a different utterance but loses the original one. Totals A=1/B=1 rather than A=1/B=2. | Same-bot repetition is suppressed as intended. Both adapter objects and served-profile entries are registered in the local fixture; callback/canonicalization/allowlist/dedup are real. No external second-bot connection is claimed. SSA. |
These fresh fixtures are not the earlier review's real-discord.py object matrix. They replace channel/member/client objects, network/voice transport, profile-discovery entries and final dispatch as stated. They invoke production builders, authorization predicates, resolver and pin functions. That distinction is retained rather than borrowing the earlier run's higher SDK fidelity.
Payload effect, not only field equality: the additional review_probe_payload_consumer.py drives a voice event into real session preparation and reads a harmless temporary skill from disk. For a new exact-thread binding, the sentinel appears in the head's prepared user text but not the base's; for a new parent-only binding, the session-start hook fires but the sentinel remains absent at both pins. Existing-session controls contain no new sentinel at either pin. The skill-location root, configuration read and task-context setter are isolated; the test stops immediately after loading, before lease acquisition, transcript mutation or provider execution. This strengthens F1 without claiming an end-to-end model response or fresh F3 payload proof.
V4: related work and executed composition
| Input and contributor | Verified live pin / actual changed scope | Relationship and outcome |
|---|---|---|
#129303, John Paul Soliva (jonpol01) |
4005d55edd7a113fcdc53b6b9cf3f0a47419350b; shared resolver and channel-skill test |
F3's helper precedence repair. Pairwise merge-tree clean. Both final resolver/test blobs exactly match the related input, and both binding-order tests pass in the union. It cannot supply F1's omitted parent argument. |
#127944, matttmcfarland |
dce6efedfca2ae5960e05ed17325938bd6d10322; voice source, adapter name lookup, renderer, pin key and voice test |
Pairwise merge-tree conflicts in run_voice.py; adapter auto-merges. Local resolution preserves this PR's skill propagation plus the related name fallback and message-ID repair. Renderer/voice tests pass, but F1/F2/F5 remain in the composed source. |
#125620, Christopher-Schulze |
1aaa74052237490022b1312ad6bcc0eb3fd4d01a; two native builders and slash-auth tests |
Pairwise merge-tree did not complete because the partial clone lacked lazy objects. The bounded API patch was composed with an explicit second-hunk resolution; no clean-merge claim. Both legitimate native role cases pass after adapting the thread test. Spoken F4 is not repaired. |
#118966, KoNit-K |
5f183b3649bd71e7729d9e3978ca0a858c8e7579; adapter and slash-command tests |
Alternative native-slash role repair, not composed on top of another repair to the same contract. Its diff does not repair the created-thread or spoken paths. |
#129317, Sahilvishnaliya |
f45457a364148ad42b22ae52448528230fb4e690; Discord adapter/voice plus jiter_preload, venv sync and config-router changes |
Competing implementation of #128787, not proof this head changed. Its live file set includes unrelated mechanisms and no changed test files. It was not mixed into the complementary union or treated as closure. |
The exact local union tree is ebc85bf8e409a89512f57de32e9015e005dd04dc, eight paths different from the target head. All eight raw-file hashes and tree blob IDs were read back. No commit, remote merge or implementation change to the reviewed branch was made.
The decisive collision was test_role_member_thread_starter_passes_gateway_authz: #125620's original test calls the old four-argument helper, while this PR's signature accepts an interaction, a thread object and text. The first union failed with TypeError before testing the grant. The disposable test was adapted to call the gate, pass the thread object and assert both destination ID and authorization. That is a test-interface reconciliation, not suppression of a failing product assertion. The original failure, exact resolution diff and later passing logs are all retained.
The five-file command selected test_discord_slash_commands.py, test_discord_turn_prompt_parity.py, test_discord_channel_skills.py, test_voice_session_prompt.py and test_discord_slash_auth.py under tests/gateway/. Passing that union supports the tested helper/renderer/native-admission contributions. It does not establish F1/F2/F5/F6 closure or full runtime safety of every alternative PR. Acceptance ledger.
V5: hosted-check interpretation
The required aggregate reports failed groups e2e-desktop-core, e2e-desktop-update and tests. The separate Python tests / Run tests leaf succeeded; cancelled E2E leaves and failed aggregate groups must not be renamed a Python-unit-test failure.
Downloaded hosted logs show Desktop core 13 failed / 2 passed, first-run 1 failed / 1 passed, and app-update 1 failed. The latter two time out waiting for a scripted assistant response. This pass did not run an equivalent Desktop baseline, so their root cause is unverified, not attributed to the Discord patch. Six label-rerun workflows remain action_required; they did not execute successfully. No required-CI-green conclusion follows from the focused local passes.
Result: the submitted nominal repairs have fresh RED/GREEN evidence; F1–F5 still have current-head observations, F6 retains its explicitly bounded source/history disposition, and the related repairs close only their demonstrated portions. The P0 class is not closed. This is a COMMENT review, not an approval, request-changes verdict or merge decision.
| resolved = resolver(str(text_ch_id)) | ||
| channel_prompt = resolved if isinstance(resolved, str) else None | ||
| if callable(skills := getattr(adapter, "_resolve_channel_skills", None)): | ||
| bound = skills(str(text_ch_id)) |
There was a problem hiding this comment.
F1 — [P1] Pass the bound thread's parent to both voice resolvers
Please carry the parent through this lookup and the adjacent prompt lookup. A supported /voice join from a thread binds that thread as text_ch_id; _handle_voice_channel_join also persists its parent_chat_id. Typed messages and the slash/thread-starter paths pass both IDs, whereas these voice calls pass only the exact thread.
Reproduce: use a thread 800 under text channel 700, with only the parent configured:
channel_prompts:
"700": "Answer in haiku."
channel_skill_bindings:
- id: "700"
skill: triageStart the voice binding from thread 800, then compare a spoken turn with a message in that same thread. At this head, my probe through the real join writer → voice callback observed:
typed: auto_skill=['triage'], channel_prompt='Answer in haiku.'
spoken: auto_skill=None, channel_prompt=None
The exact-channel and exact-thread controls passed. A separate voice-first probe invoking the actual _hmwa_auto_load_skills consumer loaded a sentinel skill payload for the exact-thread binding and omitted it for the parent-only binding. This is the new skill propagation's unresolved inheritance case; the matching prompt omission is already present on the base.
Consequence: if speech opens the session, the parent's skill is absent from the session-start instructions. Later turns cannot repair that absence because loading is gated on a new session. For the channel prompt, every typed/spoken switch changes the ephemeral system instruction, independently of the known Discord trigger-note issue.
Repair: resolve the bound parent once from the source/current channel and pass it to both _resolve_channel_prompt and _resolve_channel_skills. Preserve exact-thread override semantics; correcting the shared precedence issue in F3 is necessary too.
Session-preparation verification: the second pass also executes _hmwa_prepare_turn, the real _hmwa_open_session and session:start decision, prompt pins and the actual payload loader. A fresh exact-thread turn loads the sentinel skill; a fresh parent-only turn omits it. Both ongoing-session controls avoid reinjection. The probe stops at the turn-lease boundary after skill loading and does not invoke a model provider.
Regression: bind voice through the real join path inside a thread with parent-only instructions. Assert that the first spoken event carries the same skill/prompt as a typed event, the skill payload actually loads at session start, and typed → spoken → typed yields one pinned channel prompt. Include exact-thread override and plain-channel controls. The reproduction used real discord.py 2.7.1 channel/member objects with fake voice connection and agent dispatch; it did not exercise a Discord audio socket.
There was a problem hiding this comment.
Confirmed by reading this head: both voice lookups pass only text_ch_id, while the message, slash and /thread starter paths pass the parent as well. The prompt half is on main already; the skill half is new here and inherits the same gap. The stored join source carries parent_chat_id, so the repair is passing it to both resolvers, with #129303 (your F3) making the exact-thread entry win.
I have not pushed it to this branch: c2727ba5 carries a maintainer approval and I would rather not move an approved head unasked. It goes into a follow-up PR on top of this one as soon as it merges, with the join-path regression you describe (parent-only binding, exact-thread override, plain-channel control). If a maintainer prefers it here, I will push it here instead.
There was a problem hiding this comment.
Fixed in b993d20b24: both voice lookups now get the stored parent_chat_id. The thread-under-bound-parent row (voice joined from thread 800, prompt and skill bound only on parent 700) is red on c2727ba500 and green here. Exact-thread precedence is #129303.
| if source_data := getattr(adapter, "_voice_sources", {}).get(guild_id): | ||
| source = SessionSource.from_dict(source_data) | ||
| source.user_id = source.user_name = str(user_id) | ||
| source.user_id, source.user_name = str(user_id), user_name |
There was a problem hiding this comment.
F2 — [P1] Refresh conversation metadata as well as the speaker
This restores the speaker's current display name, but the remainder of source is a join-time snapshot. _handle_voice_channel_join writes event.source.to_dict() once; subsequent speech reconstructs that snapshot and updates only the two user fields here. A message in the bound channel/thread recomputes its label and topic from current Discord objects.
Reproduce: join voice from #ops (topic Incident triage); rename it to #renamed while leaving the voice connection active; alternate a typed message, spoken input, and typed message from the same member. I executed the actual join writer, typed builder and voice callback against a real discord.py Guild/TextChannel/Member state, then the real prompt pins:
typed chat_name: Hermes Server / #renamed
spoken chat_name: Hermes Server / #ops
distinct pinned context prompts across typed → spoken → typed: 2
Independent topic, guild-name and thread-name edits also failed. With no edit, the same probe passed with one prompt. The topic case produced New topic for typing and Incident triage for speech. Discord tools were unavailable in these cases, so the failure does not rely on the separately acknowledged trigger-note renderer difference.
Consequence: a legitimate edit is repeatedly reversed by speech and reinstated by typing for the remainder of the binding's lifetime. The model receives obsolete context on spoken turns and loses prefix stability at each switch. This source-lifetime bug predates the PR; fixing the speaker alone does not close the metadata-parity class.
Repair: keep the bound conversation ID and receiving-adapter provenance, but refresh mutable channel/thread name and topic from the current bound channel using the same _format_thread_chat_name / _get_effective_topic logic as message intake. Preserve parent/guild/thread routing fields and the existing per-speaker identity re-resolution. Define the cache-miss fallback explicitly rather than treating stored metadata as eternally current.
Regression: begin with the actual join producer, change each supported metadata field after binding, and assert equal source-derived context plus one stable prompt for typed → spoken → typed after the edit. A single rename may invalidate the old pin once; changing input transport must not toggle it back. Include a no-edit control and a bound thread. The probe substituted voice transport, admission and final model dispatch; the observed result is changed rendered/pinned bytes, not a measured provider latency.
There was a problem hiding this comment.
Confirmed by reading: _handle_voice_channel_join stores event.source.to_dict() once and _voice_input_source rebuilds from it, so chat name and topic stay at their join-time values while a typed message recomputes them. As you say, it predates this PR; the change here only stops the speaker's name from being a bare id.
Not addressed in this PR, for two reasons: the head is maintainer-approved (see the F1 thread), and the same function is being reworked in #127944, so a third edit to the voice source would have to be reconciled with both. It belongs with the F1 follow-up: refresh name/topic from the bound channel through _format_thread_chat_name / _get_effective_topic, keep the bound conversation id and the receiving adapter, and fall back to the stored values when the channel is not in cache. I will build it against whichever of the two lands first.
There was a problem hiding this comment.
Fixed in b993d20b24: chat name and topic come from the bound channel's current state through _guild_channel_labels, which computes them the way the message path does. The stored copy is used only when the channel isn't cached. The renamed-after-join row (rename + topic edit, then typed → spoken → typed) leaves one pinned prompt.
| text=text, message_type=msg_type, source=source, raw_message=interaction, | ||
| channel_prompt=self._resolve_channel_prompt(channel_id, parent_id or None), | ||
| # Bound skills load only when a session starts, and "/skill x" or "/queue" can start one. | ||
| auto_skill=self._resolve_channel_skills(channel_id, parent_id or None), |
There was a problem hiding this comment.
F3 — [P2] Repair exact-thread skill precedence in the shared resolver
This new slash startup lookup adopts a resolver whose implementation contradicts its exact channel first, then parent contract. gateway.platforms.base.resolve_channel_skills puts both IDs in ids_to_check, scans bindings in their original order, and returns the first match. Consequently an earlier parent entry shadows an exact thread entry.
A minimal runnable reproduction from the checkout/test environment:
from gateway.platforms.base import resolve_channel_skills
extra = {"channel_skill_bindings": [
{"id": "700", "skill": "parent-default"},
{"id": "800", "skill": "thread-specific"},
]}
actual = resolve_channel_skills(extra, "800", "700")
assert actual == ["thread-specific"], actual
# AssertionError: ['parent-default']I also ran both entry orders through the actual typed builder, native slash builder, public /thread starter path and voice callback. At this head:
| Binding order | Typed | Slash | /thread starter | Voice |
|---|---|---|---|---|
| Exact, parent | thread-specific | thread-specific | thread-specific | thread-specific |
| Parent, exact | parent-default | parent-default | parent-default | thread-specific |
The last-column disagreement occurs because F1 currently omits the parent. Passing the parent to voice without repairing precedence would make all four consistently select the wrong binding.
Consequence: a new native-slash session now receives the parent skill's instructions despite an explicit thread override. Configuration reordering changes behavior without changing either binding. The shared helper defect is pre-existing (also reproduced on the implementation base); native slash consumption of it is added here.
Repair: enforce ordered resolution at resolve_channel_skills: search exact channel/thread entries first, then parent entries, preserving skill-list normalization/deduplication and explicit behavior for absent/empty entries. Use the shared fix for Discord messages, slash startup, thread starters, voice, and Slack consumers.
Regression: permute the parent/exact entry order and require the exact binding to win in both; require parent inheritance when no exact binding exists; observe the selected payload at new-session startup. Keep unrelated-channel and absent-binding controls. My composed probe used real discord.py channel/member objects and fake thread creation/voice transport; it proves builder selection, not a live Discord round trip.
There was a problem hiding this comment.
Reproduced on main (f42f579cf8) with your snippet: resolve_channel_skills(extra, "800", "700") returns ['parent-default'] with the parent entry listed first, ['thread-specific'] with the order swapped.
It is in the shared helper and independent of this diff, so it is its own change: issue #129302, fix in #129303 (exact id across all bindings first, then the parent id, as resolve_channel_prompt already does; one invariant test over both entry orders, red on main). Credited to your report there. Per-entry parsing is unchanged, and Slack passes no parent so its behaviour does not move.
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Second-pass review of c2727ba500f22249dd729aba6365af3bb3c5e970
The first review stopped short of three consequential boundaries: real authorization, deduplication across receiving bots, and the binding lifetime across asynchronous transcription. I missed F4–F6 below. All three reproduce both on this head and its implementation base 5dc1c084e932d6c2aa05713e55765f6c01ebe71c; they are unresolved defects in this Discord turn-construction class, not regressions introduced by the new display-name or skill assignments. This adds three distinct inline findings without reposting F1–F3.
F4 — P1: role-authorized turns lose their admission proof
With DISCORD_ALLOWED_ROLES=1234, no user allowlist or allow-all grant, and a real guild member holding that role, typed intake carries source.role_authorized=True and passes the real gateway admission function. Native /queue passes the adapter's authorization check but builds a source with that flag false; the gateway then rejects it. The created-thread starter is rejected for the same reason. After a typed turn establishes voice, speech is also dropped: reconstruction deliberately strips the wire-invisible role flag and the voice path never revalidates the current speaker's guild role.
Observed at both revisions: typed admitted true; slash admitted false; thread starter admitted false; spoken dispatches 0. An otherwise identical user-ID grant admits all four. The new bound-skill field therefore cannot reach session preparation for role-only users. Repair the per-event grant handoff for slash/thread startup and recompute voice authorization against the current guild member; do not serialize or blindly copy a joiner's authorization onto later speakers.
F5 — P2: one bot's speech deduplication suppresses another bot's turn
The voice callback correctly receives its adapter and preserves transport provenance, but _is_duplicate_voice_transcript uses one runner-wide dictionary keyed only by (guild_id, user_id). Two independently registered Discord bots receiving the same utterance in the same guild are separate admitted turns. The first callback records the transcript; the second returns before constructing its event, applying its prompt/skill bindings or dispatching through its own bot.
My composed probe registered the second adapter under its own served multiplex profile. The first utterance dispatched once on bot A and zero times on bot B. A distinct utterance dispatched once on B, proving B's route was live; a duplicate within the same bot was correctly suppressed. This is a scope error in the dedup owner, not failed secondary-bot registration. Key deduplication by receiving transport plus bound conversation/generation, retaining same-binding repeat suppression.
F6 — P1: pending transcription adopts a later conversation binding
_process_voice_input awaits WAV conversion and STT, then invokes the adapter's current callback with only guild/user/transcript. The gateway reads the current _voice_text_channels and _voice_sources. Repeating /voice join from another text channel/thread while a captured utterance is transcribing rewrites those maps. The old utterance then becomes a turn in the new conversation, with its source, prompt and skills.
I held STT at a deterministic barrier: capture while bound to text channel 700, run the real join-binding writer for thread 800, release STT. The dispatched source was 800, with NEW_CONVERSATION_PROMPT and new-conversation-skill. With no rebind, it remained 700, with the original prompt/skill. This also reproduces on the implementation base. A legitimate rebind must not silently move already captured work between conversations. Capture binding identity/generation before asynchronous work, then either retain that authorized origin or reject/cancel the stale generation. Refreshing channel names at callback time alone, as in F2, does not solve this ownership race.
Evidence beyond the first pass
The second probe file ran through scripts/run_tests.sh with CPython 3.14.7 and real discord.py 2.7.1:
| Revision | Cases | Passed | Failed |
|---|---|---|---|
Reviewed head c2727ba5 |
14 | 9 | 5 |
Implementation base 5dc1c084 |
14 | 5 | 9 |
At the head, the five failing cases are the three new findings, the existing F1 parent-only skill omission reached through real session preparation, and the routing-boundary observation below. These are diagnostic expectation failures, not a claim of five new defects. No collection errors remain. The base also lacks this PR's already repaired exact-binding skill propagation, so its unchanged-binding STT control fails only on that missing skill after correctly retaining channel 700; the rebinding case fails on the wrong channel at both revisions.
F1's consumer proof is now stronger: the added probe executes _hmwa_prepare_turn, the real _hmwa_open_session and its session:start hook decision, the actual prompt pins, and the real skill payload loader. A fresh exact-thread voice turn loads the sentinel payload; a fresh parent-only turn does not. Both ongoing-session controls avoid reinjection. Execution stops at the turn-lease boundary after skill loading; it does not invoke a provider. I updated the existing F1 inline rather than adding a duplicate.
Real interaction cache controls passed: both a cached thread and an actual discord.Interaction constructed from a complete thread payload on a thread-cache miss retain the expected name, parent, prompt and skill at this head. The SDK derives channel_id from its channel object, so an earlier synthetic channel=None plus an independent nonempty channel_id was not a valid SDK counterexample; it is withdrawn and not published as a finding.
The routing boundary was also missing from my first coverage: with auto-threading enabled, a typed top-level /queue investigate routes to thread 800, while the native command remains in parent 700. Their session keys differ; disabling auto-threading makes them match. The probe substitutes only the thread-creation network result, then executes the real message/slash builders and key function. I am treating this as a limit on the asserted parity invariant, not inventing a fourth new defect: equality must be defined after routing to the same conversation, and the configured native-versus-typed routing policy needs explicit coverage. The supplied tests disable auto-threading and do not establish default top-level routing parity.
Boundaries: role authorization and gateway admission in F4 are real; voice connection and thread-creation HTTP are substituted. F5 uses real adapter/source routing with explicit served-profile discovery, substituted authorization admission and isolated downstream dispatch. F6 executes the real asynchronous processing method and join writer, with voice connection, speaker admission, final dispatch, STT and WAV conversion substituted at a controlled barrier; it verifies the dispatched origin, not a live audio/socket run. The earlier 419-pass standard suite remains the earlier run, not a newly rerun full suite.
#127944 remains at dce6efedfca2ae5960e05ed17325938bd6d10322; its current diff addresses the already documented speaker/message-ID seams and does not carry role grants, isolate dedup state, or fence captured work against rebinding. CI at this exact head still has 15 queued checks out of 52 and no completed check reported as failure; this is not an all-green commit receipt.
| text=text, message_type=msg_type, source=source, raw_message=interaction, | ||
| channel_prompt=self._resolve_channel_prompt(channel_id, parent_id or None), | ||
| # Bound skills load only when a session starts, and "/skill x" or "/queue" can start one. | ||
| auto_skill=self._resolve_channel_skills(channel_id, parent_id or None), |
There was a problem hiding this comment.
F4 — [P1] Preserve the role grant through slash/thread admission and recheck it for speech
This event can carry the correct bound skill and still never reach the agent. The adapter's slash gate accepts DISCORD_ALLOWED_ROLES, but _build_slash_event calls build_source without the per-event role_authorized grant. The typed path receives that grant from _discord_message_admission. The downstream _hm_admit_event rejects a role-only principal whose source does not carry it.
Reproduce: configure DISCORD_ALLOWED_ROLES=1234, leave DISCORD_ALLOWED_USERS empty, disable Discord/global allow-all, and use a guild member holding role 1234. Compare a typed turn with native /queue investigate; then create a /thread with a starter and establish voice through a typed /voice join. The adapter's native authorization check passes.
The composed probe used real Discord Guild/Member/Role objects and the actual gateway admission functions, without overriding _is_user_authorized:
grant typed admitted native admitted thread starter admitted speech dispatches
user ID True True True 1
role True False False 0
Voice is the sibling manifestation: SessionSource.to_dict/from_dict deliberately omit the wire-invisible role flag, and _voice_input_source updates the current speaker's name/ID without recomputing their current guild-scoped role authorization. The role-only speaker is dropped before event dispatch. A typed join can establish the connection; a native join is itself subject to the admission failure.
Consequence: role-based deployments cannot use these agent-producing turn types even though Discord's gate admits the user. Adding auto_skill does not close the producer → admission → consumer contract.
Repair: carry the adapter-verified, current-event grant into slash and created-thread sources through a shared admission result. For speech, verify the current speaker against the receiving bot's current policy and guild member, then attach only that event's grant. Do not serialize the grant, set it merely because a role list is configured, or inherit the joining speaker's authorization.
Regression: run authorized-role, user-ID, unrelated-role, revoked-role and different-speaker cases through adapter admission and the real gateway gate, then assert actual skill-bearing dispatch/session startup. The composed role/user-ID cases above are executed; the negative cases are the required repair contract. Voice connection and thread HTTP were replaced, not authorization. This failure also reproduces on the implementation base; it predates this PR.
There was a problem hiding this comment.
Confirmed by reading this head and main; predates this PR.
- Slash and
/threadstarter:_build_slash_eventand_dispatch_thread_sessioncallbuild_sourcewithoutrole_authorized, while the message path passes the verdict from_discord_message_admission._principal_authorizedhonours a role grant only throughsource.role_authorized is True, so a role-only member passes_check_slash_authorizationand is then refused by the gateway. This half is already tracked as [Bug]: Discord slash commands from role-authorized users are rejected by the gateway (slash event never carries role_authorized) #118958, with fix(discord): preserve slash role authorization #118966 and fix(discord): carry the role verdict into slash command events #125620 open against it (fix(discord): carry the role verdict into slash command events #125620 touches both builders). This PR's body names fix(discord): carry the role verdict into slash command events #125620 as a textual neighbour in_build_slash_event; I am not opening a third PR for the same two lines. - Voice: not covered by either of those. The listen loop admits the speaker through
_is_allowed_user(roles included, guild-scoped), then_handle_voice_channel_inputre-checks a source rebuilt bySessionSource.from_dict, which never carries the flag, so a role-only speaker is dropped at_is_user_authorized_for_source. Not re-run here.
The voice half goes into the voice follow-up with F1/F2, with the grant recomputed for the current speaker against the guild member at dispatch, not serialized and not inherited from whoever ran /voice join. Nothing is pushed to this branch: c2727ba5 is the approved head.
There was a problem hiding this comment.
Voice half: #130302 / #130303. It recomputes the grant for the current speaker against the current guild member, and is pinned through the real gateway _is_user_authorized. Refusals are pinned too: a speaker without the role, and one who left the guild. Native slash and /thread starter half: #118958 (#125620, #118966).
| source=source, text=transcript, message_type=MessageType.VOICE, | ||
| raw_message=SimpleNamespace(guild_id=guild_id, guild=None), | ||
| channel_prompt=channel_prompt) | ||
| channel_prompt=channel_prompt, auto_skill=auto_skill) |
There was a problem hiding this comment.
F5 — [P2] Scope voice deduplication to the receiving bot and bound conversation
A valid second bot's turn can be discarded before reaching the new prompt/skill construction. _handle_voice_channel_input receives adapter and rebuilds transport provenance, but calls _is_duplicate_voice_transcript(guild_id, user_id, transcript). That helper owns one runner-wide dictionary keyed by only (guild_id, user_id).
Reproduce: serve two independent Discord adapter/profile owners in the same guild, with active voice bindings. Submit the same captured utterance for user 42 to both callbacks within the 12-second window. My probe registered the second adapter under its own served multiplex profile and exercised the actual callback and source routing:
same utterance, bot A: dispatched 1
same utterance, bot B: dispatched 0
different utterance, bot B: dispatched 1
same-bot duplicate control: total dispatched 1
Thus the second transport is live; it is suppressed by another bot's dedup state. The helper cannot distinguish those captures because its interface has no transport or conversation discriminator. The first completed transcription wins regardless of which bot/profile should handle the other turn.
Consequence: a receiving bot silently loses legitimate speech and never applies its own bound prompt/skills. Transport provenance in the eventual source does not help if execution returns before constructing that event.
Repair: include the receiving transport identity and bound conversation/binding generation in the dedup key, and keep retention bounded. A repeated capture within the same binding should still be suppressed; another bot or a newly bound conversation must not borrow the previous binding's history.
Regression: two registered bots, identical guild/user/transcript, distinct transport owners must both dispatch. Repeat within one binding must dispatch once. Cover different bound text channels, rebind/reset and different users, and ensure no cross-profile mutable dedup state is consulted. My test executes source routing and the callback with real Discord objects, substitutes served-profile discovery, authorization admission and downstream dispatch, and does not claim a live audio run. This is also present on the implementation base.
There was a problem hiding this comment.
Confirmed by reading this head and main; predates this PR, which does not touch the dedup.
_bind_voice_input_callback binds _handle_voice_channel_input per adapter, but _is_duplicate_voice_transcript(guild_id, user_id, transcript) takes no adapter and keeps one runner-wide store keyed by (guild_id, user_id). Two bots served by one gateway and listening in the same guild share one 12 s window, so the second bot's copy of an utterance returns before its event is built. The helper came in with #19428 for the single-bot case. Not re-run here.
It goes into the voice follow-up: key by receiving adapter plus the bound text channel, so a repeat inside one binding is still suppressed and another bot or a new binding starts clean.
| if source_data := getattr(adapter, "_voice_sources", {}).get(guild_id): | ||
| source = SessionSource.from_dict(source_data) | ||
| source.user_id = source.user_name = str(user_id) | ||
| source.user_id, source.user_name = str(user_id), user_name |
There was a problem hiding this comment.
F6 — [P1] Bind captured speech before asynchronous transcription
This source is reconstructed from the current voice binding, not necessarily the binding that captured the utterance. In the adapter, _process_voice_input awaits WAV conversion/STT and then consults its current _voice_input_callback, passing only guild ID, user ID and transcript. The callback reads the current _voice_text_channels and this current _voice_sources entry.
Reproduce: establish voice from text channel 700; start processing an utterance and pause STT; issue another /voice join from thread 800 while the same bot's voice connection remains active; release the old transcription. The join writer replaces the binding while the captured work is in flight.
I ran the actual DiscordAdapter._process_voice_input, real callback binding, real join-source writer and gateway voice callback, with an explicit STT barrier:
no rebind: captured in 700 -> dispatched source.chat_id 700
rebind during STT: captured in 700 -> dispatched source.chat_id 800
channel_prompt='NEW_CONVERSATION_PROMPT'
auto_skill=['new-conversation-skill']
Voice connection, speaker admission and final dispatch are substituted. The barrier replaces WAV conversion and transcription; it does not replace the binding mutation or dispatched-source construction. The same failure occurs on the implementation base.
Consequence: an old utterance is silently treated as a request in a different conversation, receiving that conversation's prompt, skills and routing. Recomputing fresh names/topics at completion (F2) would still select the wrong owner. The new skill propagation makes the current binding's skill available to this wrongly relocated turn.
Repair: capture immutable binding identity/generation when accepting the audio work, carry it through STT and the callback, and validate it at dispatch. Choose an explicit stale-generation policy: preserve the authorized original conversation or discard/cancel stale work. Never silently redirect it through the replacement binding. Apply the same fence to leave/rejoin/disconnect and any pending capture/transcription tasks; preserve receiving-adapter provenance and recheck the current speaker's admission.
Regression: deterministic barriers around conversion and STT; rebind, leave/rejoin and different-bot cases; unchanged-binding control. Assert dispatched conversation/profile and selected prompt/skill payload, or explicit stale-work rejection, rather than merely asserting that a callback returned. No live Discord audio/socket is claimed.
There was a problem hiding this comment.
Confirmed by reading this head and main; predates this PR.
_process_voice_input awaits WAV conversion and STT, then calls the callback with only guild, user and transcript. _handle_voice_channel_input reads _voice_text_channels[guild_id] and _voice_sources[guild_id] at that moment. A second /voice join while the bot is already connected returns early from join_voice_channel, and _handle_voice_channel_join then overwrites both maps, so an utterance captured under the old binding is dispatched into the new conversation with its prompt and skills. Not re-run here.
The window is one STT round trip, but the outcome is a turn in the wrong conversation, so it is worth a fence. In the voice follow-up: capture the binding when the utterance is accepted, carry it through STT into the callback, and either keep the captured conversation or drop the stale work, never redirect it; leave/rejoin goes through the same check. Agreed that this also decides F2: the metadata refresh has to be for the captured binding, not whatever is current at completion.
There was a problem hiding this comment.
|
@andrexibiza thanks. All three findings check out; details are in the inline threads.
Nothing is pushed to this branch: On scope, agreed: this PR's claim is parity for the cases its tests exercise (DM/thread slash labels, the |
|
@andrexibiza thanks for the second pass. F4–F6 all hold by reading on this head and on
None of these were re-run here; the conclusions above come from the code paths, not from your probe file. Nothing is pushed to this branch. On the routing boundary: agreed, as a limit of the claim and not a fourth defect. A typed mention that auto-threads runs in the new thread; a native slash runs in the channel it was invoked in. The parity this PR asserts is for turns that land in the same conversation, which is why its tests disable auto-threading. Default top-level routing is not covered by it. |
|
@jonpol01 Six findings are on the record here, not three. F4-F6 were posted before your first reply, so "all three findings check out" was answering half the board. Regarding the second, "All three predate this PR" is a location report, not a resolution. Four of the six are P1 admission and authority defects, each reproduced at base and at this head with production code in the loop. This PR touches exactly the code where this class lives, which makes this the cheapest it will ever be to fix. Deferring the class to a follow-up that doesn't exist yet is how findings become incident reviews. On the approval you're keeping stable: read its own first line. "LGTM on correctness. Merge is Teknium's call (no merge from me)." That is a correctness approval that explicitly declines the merge decision, from an account that does not speak as the maintainer. It cannot carry the weight you're putting on it. The unit of repair is the class, not the diff. I don't think you want to be the person responsible for another half-fix to this problem. |
…e channel's current labels A spoken turn rebuilds its source from the copy `/voice join` stored, and three of its prompt inputs still differed from a typed message in the same conversation: - voice joined from a thread looked up the channel prompt and bound skills for the thread alone, while a typed message there falls back to the parent channel's entry, so a parent-only binding never reached speech; - the chat name and topic stayed at their join-time values, so after a rename or topic edit every typed/spoken switch re-rendered the pinned context prompt and back; - `guild.get_member()` is cache-only, and on a miss the speaker was named by numeric id even when they were the member who ran `/voice join`. The resolvers now get the stored parent id, the name and topic come from the bound channel through the same formatters a message uses (the stored values stay as the fallback when the channel is not cached), and an uncached joiner keeps the name bound at join; any other uncached speaker still gets their id, never the joiner's name. Reported by andrexibiza (F1, F2) and teknium1 (get_member) on NousResearch#128788. Co-authored-by: Matt McFarland <mattttmcfarland@gmail.com>
|
Cross-reference from the #132758 review, reproduced on this PR's head With Fix that composes with this PR's |
…e_id presence With the discord toolset loaded, a typed turn rendered the per-turn triggering-message line and keyed the agent cache on message_id presence, while slash, /thread and voice turns (no triggering message) did not, so the pinned session-context prompt re-rendered on every switch between them. The line is now static and the presence bit is out of the key; the real id still rides the user message when there is one. Reported by teknium1 in the NousResearch#132758 review; fix shape from andrexibiza's NousResearch#132758. Co-authored-by: Andrex Ibiza, MBA <andrexibiza@gmail.com>
|
Thanks, reproduced and folded into this PR at 2a3f04a.
Ran: |
ehz0ah
left a comment
ehz0ah
left a comment
There was a problem hiding this comment.
Verdict: approve. GitHub does not allow this account to submit a formal approval without explicit repository access, so this is the same exact-head verdict as a commit-bound review comment.
Verified current head 2a3f04a67d.
The latest commit closes the remaining Discord-tools parity gap. _discord_platform_notes() now renders the triggering-message guidance for every Discord turn, while the actual ID still rides only on the per-turn user message. _ephemeral_change_key() drops the message-ID presence bit, so the cache key now follows the bytes that are rendered. Alternating between typed, slash, /thread, and voice turns no longer re-pins the session prompt for this reason, and reply/react/pin guidance remains available.
I ran the updated parity test against parent b993d20b24: the four Discord-tools cases fail there and pass on this head. On 2a3f04a67d, the focused gateway run finished with 100 passed and 4 skipped across the parity, pin-wiring, prompt-freeze, triggering-note, slash-command, and voice-command files. The existing message-ID stability test also passed. The head composes cleanly with current main at af90026aa0.
No new blocking findings.
There was a problem hiding this comment.
P0 re-review — 2a3f04a6 closes the renderer seam; the Discord ingress class remains open
This is a current-head reconciliation of the existing F1–F6 review record against 2a3f04a67d1aa6dc43628ce0959ddb699baa84f8. I am not adding duplicate inline findings: the existing changed-RIGHT F3–F5 threads remain the implementation anchors, while the older F1/F2/F6 anchors are retained as provenance.
Revision custody
| Object | Current state |
|---|---|
| Reviewed head | 2a3f04a67d1aa6dc43628ce0959ddb699baa84f8 |
| PR base snapshot | 38d3dbce4adbb9074a34214eb330237a129e5904 |
Delta since the last reviewed head b993d20b24 |
exactly one commit; only gateway/session.py, gateway/run_agent_cache.py, and tests/gateway/test_discord_turn_prompt_parity.py changed |
| Nix flake check | completed, action_required |
| Docker Build, Test, and Publish | completed, action_required |
| CI | completed, action_required |
| Combined commit statuses | no status contexts returned |
The new commit repairs the renderer/key half correctly
The new 2a3f04a6 delta removes the last message-id-presence difference from the pinned Discord context. _discord_platform_notes now emits one stable “Triggering message” instruction and describes the ID as per-turn data when available; _ephemeral_change_key correspondingly no longer keys the pinned prompt on message_id presence. Those producer/consumer semantics now agree.
That closes the renderer/cache-key seam previously left to #127944. It does not alter the mechanisms controlling F3–F6.
Repair ledger at the current head
| Finding | Current-head disposition | Controlling evidence |
|---|---|---|
| F1 — voice parent prompt/skill inheritance | repaired here | _handle_voice_channel_input now passes source.parent_chat_id into both resolvers. |
| F2 — voice channel name/topic frozen at join | repaired here | _voice_input_source refreshes labels from the currently bound channel while retaining the join snapshot only as fallback. |
| F3 — exact thread skill loses to earlier parent config row | still absent from this head | resolve_channel_skills still collapses exact + parent IDs into an unordered membership set and returns the first matching config entry. #129303 remains open. |
| F4 — admitted role-only slash/thread/voice actor loses the grant at gateway admission | still absent from this head | _build_slash_event and _dispatch_thread_session still build sources without the per-actor role verdict; _principal_authorized accepts adapter role delegation only when source.role_authorized is True. #130303 remains open. |
| F5 — voice dedup is shared across receiving bots/conversations | still absent from this head | _is_duplicate_voice_transcript still owns one runner store keyed only by (guild_id, user_id); _handle_voice_channel_input still consults it without receiving adapter or bound conversation identity. #130308 remains open. |
F6 — speech captured before STT can adopt a later /voice join binding |
still absent from this head | _process_voice_input still crosses conversion/STT and invokes the callback with only guild/user/transcript; _handle_voice_channel_input then reads the adapter's current bound text channel. #130312 remains open. |
All four companion repairs are still open and unmerged: #129303, #130303, #130308, and #130312. A repair existing on another branch is evidence about the repair shape; it is not behavior present in this PR.
SSA / class-closure map
%%{init: {'theme':'dark','flowchart':{'nodeSpacing':12,'rankSpacing':28},'themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TD
subgraph CORE[" "]
direction TB
T["Typed Discord message"] --> N["Current metadata +<br/>admission"]
S["Native slash / thread<br/>starter"] --> B["Build source + prompt<br/>inputs"]
V["Voice capture"] --> STT["Async conversion / STT"]
STT --> CB["Callback reads current<br/>binding"]
N --> G{"Gateway admission"}
B --> G
CB --> D{"Runner-wide voice dedup"}
D --> G
CFG["Channel skill bindings"] --> R["resolve_channel_skills"]
R --> B
R --> CB
G --> P["Prompt pins / session<br/>start"]
P --> A["Agent turn"]
FIX["2a3f04a6: stable<br/>triggering-message note<br/>+ key"] --> P
B -. "F4: role grant<br/>omitted" .-> X4["Allowed actor refused"]
R -. "F3: config order can<br/>shadow exact thread" .-> X3["Wrong bound skill"]
D -. "F5: no bot /<br/>conversation<br/>discriminator" .-> X5["Peer bot turn<br/>suppressed"]
STT -. "F6: no capture<br/>generation" .-> X6["Old utterance follows<br/>new binding"]
C1["#129303 open"] -.-> R
C2["#130303 open"] -.-> B
C3["#130308 open"] -.-> D
C4["#130312 open"] -.-> STT
end
CORE ~~~ Legend
Legend["Legend<br/>Blue: entry / incoming<br/>Purple: ownership / policy<br/>Amber: transition / pending<br/>Green: success<br/>Red: failure<br/>Gray: context / transient"]
style CORE fill:#0d1117,stroke:#30363d,color:#e6edf3
style Legend fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
classDef context fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px
classDef normal fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px
classDef repaired fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
classDef authority fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
classDef failure fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
classDef pending fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
class T,S,V normal
class N,B,STT,CB,A pending
class CFG context
class R,P authority
class G,D authority
class FIX repaired
class X3,X4,X5,X6 failure
class C1,C2,C3,C4 pending
click B href "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/plugins/platforms/discord/adapter.py#L4700-L4798" "Current native Discord builders"
click G href "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/authz_mixin.py#L614-L675" "Current gateway admission"
click D href "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/run_voice.py#L211-L287" "Current voice dedup and callback"
click STT href "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/plugins/platforms/discord/adapter.py#L3870-L3905" "Current asynchronous STT handoff"
click R href "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/platforms/base.py#L1818-L1844" "Current skill resolver"
click FIX href "https://github.com/NousResearch/hermes-agent/commit/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8" "Current renderer/key repair"
click C1 href "https://github.com/NousResearch/hermes-agent/pull/129303" "Unmerged F3 repair"
click C2 href "https://github.com/NousResearch/hermes-agent/pull/130303" "Unmerged F4 repair"
click C3 href "https://github.com/NousResearch/hermes-agent/pull/130308" "Unmerged F5 repair"
click C4 href "https://github.com/NousResearch/hermes-agent/pull/130312" "Unmerged F6 repair"
click S "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/plugins/platforms/discord/adapter.py#L4700-L4798" "Native slash and thread source builders"
click T "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/plugins/platforms/discord/adapter.py" "Pinned source for existing review diagram; evidence limits remain in text"
click N "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/plugins/platforms/discord/adapter.py" "Pinned source for existing review diagram; evidence limits remain in text"
click V "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/plugins/platforms/discord/adapter.py#L3870-L3905" "Pinned source for existing review diagram; evidence limits remain in text"
click CB "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/run_voice.py#L265-L273" "Pinned source for existing review diagram; evidence limits remain in text"
click CFG "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/platforms/base.py#L1818-L1844" "Pinned source for existing review diagram; evidence limits remain in text"
click P "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/run_agent_cache.py#L718-L742" "Pinned source for existing review diagram; evidence limits remain in text"
click A "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/run_turn_runner.py" "Pinned source for existing review diagram; evidence limits remain in text"
click X3 "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/platforms/base.py#L1818-L1844" "Pinned source for existing review diagram; evidence limits remain in text"
click X4 "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/authz_mixin.py#L623-L659" "Pinned source for existing review diagram; evidence limits remain in text"
click X5 "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/run_voice.py#L211-L228" "Pinned source for existing review diagram; evidence limits remain in text"
click X6 "https://github.com/NousResearch/hermes-agent/blob/2a3f04a67d1aa6dc43628ce0959ddb699baa84f8/gateway/run_voice.py#L265-L273" "Pinned source for existing review diagram; evidence limits remain in text"
The unit of review is one admitted human Discord turn in one owned conversation. The class is not closed when field parity is correct but instruction precedence, admission proof, duplicate ownership, and capture-generation ownership still live on four separate unmerged heads.
Acceptance
Keep the F1/F2 and 2a3f04a6 repairs. Then fold #129303, #130303, #130308 and #130312 into this PR (or implement equivalent complete repairs here) while preserving their contributors and regression coverage. The final head needs one composed acceptance run that exercises:
- exact-thread skill precedence over parent fallback in either config order;
- real gateway admission for role-only native slash,
/threadstarter and voice actors, with negative-role controls; - identical speech received by two bots/conversations dispatching independently while same-binding duplicates remain suppressed;
- an STT barrier across rebind, leave/rejoin and unchanged-binding controls, proving captured work cannot silently change conversation generation;
- the supplied typed ↔ slash/thread/voice prompt-parity matrix with Discord tools both enabled and disabled; and
- hosted CI from the actual final head, not a sibling branch.
Evidence boundary
The earlier F3–F6 review objects contain the runnable observed reproductions. This pass refreshed the exact head, controlling source, review threads, companion-PR merge state, one-commit delta, and hosted workflow state. I did not relabel those older runtime outputs as a fresh 2a3f04a6 execution receipt. Because the sole b993d20b24..2a3f04a6 delta does not touch any F3–F6 controlling mechanism, the defensible current-head statement is narrower and sufficient for reconciliation: their repairs are not present in this PR, their owning threads remain unresolved, and class closure has not been demonstrated at this head.
Rendering note: this execution host does not expose mmdc; the diagram above is delivered as GitHub-native Mermaid source and is not claimed as a fresh CLI-render receipt.
…e channel's current labels A spoken turn rebuilds its source from the copy `/voice join` stored, and three of its prompt inputs still differed from a typed message in the same conversation: - voice joined from a thread looked up the channel prompt and bound skills for the thread alone, while a typed message there falls back to the parent channel's entry, so a parent-only binding never reached speech; - the chat name and topic stayed at their join-time values, so after a rename or topic edit every typed/spoken switch re-rendered the pinned context prompt and back; - `guild.get_member()` is cache-only, and on a miss the speaker was named by numeric id even when they were the member who ran `/voice join`. The resolvers now get the stored parent id, the name and topic come from the bound channel through the same formatters a message uses (the stored values stay as the fallback when the channel is not cached), and an uncached joiner keeps the name bound at join; any other uncached speaker still gets their id, never the joiner's name. Reported by andrexibiza (F1, F2) and teknium1 (get_member) on #128788. Co-authored-by: Matt McFarland <mattttmcfarland@gmail.com> (cherry picked from commit b993d20)
unsupportedpastels
left a comment
unsupportedpastels
left a comment
There was a problem hiding this comment.
Thank you @jonpol01: your fix landed in #133463, with your commits cherry-picked and your authorship kept. We added type hints to the test and folded some comments so adapter.py stays under its code-health size cap. Thanks also to @andrexibiza for the review. Closing in favour of #133463.
…r bot's turn _is_duplicate_voice_transcript keeps one runner-wide store keyed by (guild_id, user_id). When one gateway serves several Discord bots (multiplexed profiles) listening in the same guild, each bot transcribes its own copy of an utterance; the first callback records it and the second returns before building its event, so that bot never answers. A `/voice join` from another text channel also inherited the previous binding's 12-second history. The key is now the receiving bot's profile, the guild, the bound text channel and the speaker. A repeated capture inside one binding is still suppressed, as #19428 intended. Reported by andrexibiza (F5) on #128788. (cherry picked from commit 0b889d0)
… not posted to the new channel _process_voice_input awaits WAV conversion and STT, then calls the gateway callback with only guild, user and transcript, and the callback routes by the guild's binding as it is at that moment. A `/voice join` from another text channel while the bot is already connected rewrites _voice_text_channels and _voice_sources, so an utterance captured for the old conversation became a turn in the new one, with its channel prompt, bound skills, transcript echo and replies. The bound text channel is captured when the utterance is accepted and compared after STT; if the binding moved, the stale work is dropped and logged. Rejoining from the same channel keeps the conversation and is dispatched as before. Nothing awaits between that check and the callback's own read of the binding. Reported by andrexibiza (F6) on NousResearch#128788. (cherry picked from commit 47d515e)
… not posted to the new channel _process_voice_input awaits WAV conversion and STT, then calls the gateway callback with only guild, user and transcript, and the callback routes by the guild's binding as it is at that moment. A `/voice join` from another text channel while the bot is already connected rewrites _voice_text_channels and _voice_sources, so an utterance captured for the old conversation became a turn in the new one, with its channel prompt, bound skills, transcript echo and replies. The bound text channel is captured when the utterance is accepted and compared after STT; if the binding moved, the stale work is dropped and logged. Rejoining from the same channel keeps the conversation and is dispatched as before. Nothing awaits between that check and the callback's own read of the binding. Reported by andrexibiza (F6) on #128788. (cherry picked from commit 47d515e)
…aches the agent With DISCORD_ALLOWED_ROLES and no user allowlist, the voice listen loop admits a role member through the adapter's guild-scoped role check, but _handle_voice_channel_input rebuilds the turn's source from the `/voice join` copy. SessionSource.to_dict() deliberately drops the per-event role_authorized grant, so the gateway's own check saw a principal with no grant and dropped every spoken turn from a role-only member, while the same member's typed messages were answered. The grant is now recomputed for the current speaker against the guild's current member, with the meaning the message path gives it (the adapter has a role allowlist and this user passes its gate). It is never serialized and never inherited from whoever ran `/voice join`: a speaker without the role, or no longer in the guild, is still refused. Reported by andrexibiza (F4, voice half) on NousResearch#128788. (cherry picked from commit 6985ea6)
… for its own actor only The salvaged NousResearch#125620 commits stamp role_authorized on _build_slash_event and _dispatch_thread_session from bool(_allowed_role_ids), which is only right because today's two callers run _check_slash_authorization first. Neither builder checks its actor itself, so any later caller that builds an event before (or without) the gate would mint a grant for a member the gate refuses. The grant is now re-evaluated from the interaction in both builders (_slash_role_grant): a role allowlist exists AND this actor passes the slash gate, the meaning _discord_message_admission gives the flag. A role-only member still reaches the agent, and an unrelated-role actor's event is refused by the gateway even when built directly. The two salvaged tests fold into one parametrized invariant over both builders with the refusal row. Recomputing the verdict from the actor follows NousResearch#118966 by KoNit-K. Reported by andrexibiza (F4, native half) on NousResearch#128788 and NousResearch#130303. Co-authored-by: KoNit-K <konit.block@protonmail.com> (cherry picked from commit 357e77d)
…aches the agent With DISCORD_ALLOWED_ROLES and no user allowlist, the voice listen loop admits a role member through the adapter's guild-scoped role check, but _handle_voice_channel_input rebuilds the turn's source from the `/voice join` copy. SessionSource.to_dict() deliberately drops the per-event role_authorized grant, so the gateway's own check saw a principal with no grant and dropped every spoken turn from a role-only member, while the same member's typed messages were answered. The grant is now recomputed for the current speaker against the guild's current member, with the meaning the message path gives it (the adapter has a role allowlist and this user passes its gate). It is never serialized and never inherited from whoever ran `/voice join`: a speaker without the role, or no longer in the guild, is still refused. Reported by andrexibiza (F4, voice half) on #128788. (cherry picked from commit 6985ea6)
… for its own actor only The salvaged #125620 commits stamp role_authorized on _build_slash_event and _dispatch_thread_session from bool(_allowed_role_ids), which is only right because today's two callers run _check_slash_authorization first. Neither builder checks its actor itself, so any later caller that builds an event before (or without) the gate would mint a grant for a member the gate refuses. The grant is now re-evaluated from the interaction in both builders (_slash_role_grant): a role allowlist exists AND this actor passes the slash gate, the meaning _discord_message_admission gives the flag. A role-only member still reaches the agent, and an unrelated-role actor's event is refused by the gateway even when built directly. The two salvaged tests fold into one parametrized invariant over both builders with the refusal row. Recomputing the verdict from the actor follows #118966 by KoNit-K. Reported by andrexibiza (F4, native half) on #128788 and #130303. Co-authored-by: KoNit-K <konit.block@protonmail.com> (cherry picked from commit 357e77d)
* fix(discord): capture the voice binding when a poll batch is collected
The listen loop transcribes a check_silence() batch serially, so a later
utterance read its binding after a /voice join landed during an earlier
utterance's STT and was posted to the new channel. Capture the binding
once per batch (and per leave flush) and pass it in.
(cherry picked from commit e4b020a0fd21833a89fc1fa7f904db489d797d9c)
* refactor(discord): move voice-channel info helpers to a sibling mixin; annotate the stale-binding test
get_voice_channel_info and get_voice_channel_context move unchanged into adapter_voice_info.py, and the new comments and docstring are folded, so adapter.py stays within its code-health file cap. Add type hints to the new test.
* fix(discord): a /voice join to another text channel drops speech the receiver still holds for the old one
The receiver buffers PCM per speaker with no conversation identity, and a same-voice-channel /voice join keeps that receiver. The poll and the leave flush then read the binding after draining, so audio captured for channel A was stamped with B and passed the post-STT check. The gateway join handler now discards the receiver's unemitted audio, under its lock, before moving the binding to a different text channel; a first join or same-channel rejoin keeps it. Nothing awaits between the discard and the binding write.
Reported by andrexibiza (F1) on #133471 and #130312.
* test(discord): typed stand-ins in the batch stale-binding test
The two-utterance batch test assigned lambdas over adapter methods and a fake receiver into a VoiceReceiver map; use MagicMock and cast, as the newer rebind tests do, so the file adds no ty diagnostics.
* feat(browser): public captured CDP call seam for trusted plugins
Plugins that drive the agent's browser below the browser tools (Secure ENV,
catalog review on #132272) had no public way to send a raw CDP command over a
task's supervisor connection, so they bumped CDPSupervisor._next_call_id,
inserted futures into _pending_calls and sent on _ws. That breaks silently on
any internal change and collides between plugins.
SUPERVISOR_REGISTRY.capture(task_id) now returns a CapturedCDP pinned to the
supervisor's current socket and default page session:
call(method, params=None, *, session_id=None, timeout=10.0) -> raw reply
is_valid() / page_session_id / task_id / cdp_url
The validity check and the send run in one supervisor-loop step, so a call
never reaches a newer connection: after a reconnect, stop or registry
replacement every call raises CapturedCDPInvalid. Calls in flight when the
socket drops now fail at once (the supervisor fails pending replies on
teardown) instead of sitting out their timeout.
No model tool, config default or new capability: in-process Python could
already reach this. The handle enforces no origin/consent/ownership policy.
Reduced-scope salvage of #132752 (API shape and names from that PR); its
attach-reply handoff, old-wire cleanup and dispatch fences are not needed
because the supervisor closes the old socket on every reconnect, which drops
its sessions with it.
Co-authored-by: web3blind <264741654+web3blind@users.noreply.github.com>
* refactor(gateway): move plugin message injection into its own runner mixin
run_inbound.py was past the 2,000-line gate; the injector install/clear,
scheduler and dispatch move verbatim to gateway/run_plugin_injection.py
(GatewayPluginInjectionMixin). No behaviour change.
* feat(plugins): inject_message(origin=...) starts a gateway session in the plugin's own profile
`ctx.inject_message` could only reach an existing gateway session (`session_key`), so an
orchestrator plugin that opens a chat/thread itself (a bot-authored Discord forum post never
comes back as an inbound message) had no way to get an agent turn there.
`origin` takes the SessionSource.to_dict() shape. The gateway creates (or continues) that chat's
session through the adapter's normal message path, in the profile whose plugin manager loaded the
plugin, and the reply goes back to the origin chat:
- Profile: resolved from the manager's immutable home, never an ambient one. The origin may not
name another profile, the gateway's own routing for that chat through that profile's adapter
must land in the plugin's profile, and the profile needs its own connected adapter for the
platform (another profile's bot is never borrowed). Refusals are synchronous False + one log line.
- Authorization: same current-allowlist check as the session_key path (no bypass for host code).
- Multiplex: handle_message runs inside the profile's runtime scope so the spawned turn AND the
reply send after it inherit that profile's home and secrets (applies to the session_key path too,
which used to send from the default loop context).
- allow_gateway_injection is read from the plugin's own profile config.
- Plugin/system turns (event.internal) skip the human first-contact notes (onboarding intro and
the "no home channel" nudge), which were posted into the plugin's fresh thread.
Requested by canonicle-desk in #132609.
Closes #132609
* feat(computer-use): pluggable single-select computer-use providers
Third-party drivers (e.g. a Codex-style computer-use reimplementation) could
only replace cua-driver by patching core. computer_use now follows the
single-select provider model Hermes uses for memory providers, context
engines and cron schedulers:
- plugins/computer_use/__init__.py discovers bundled
plugins/computer_use/<name>/ then user $HERMES_HOME/plugins/<name>/
(marker: register_computer_use_provider / ComputerUseProvider) and loads
ONLY the provider computer_use.backend names (default "cua"), read per
call so each profile gets its own. Unselected providers are never
imported; listing reads plugin.yaml only. An unknown/unloadable name
returns an error naming it and the installed providers; no fallback.
- The built-in cua-driver is an ordinary bundled provider at
plugins/computer_use/cua/ (driver implementation stays in
tools/computer_use). tools/computer_use/backend.py gains
ComputerUseProvider (create_backend(permission_mode=...), is_available(),
optional doctor()).
- The tool's check_fn and backend construction go through the selected
provider; the model-facing schema is unchanged for every provider
(prompt-cache invariant). The test-only HERMES_COMPUTER_USE_BACKEND env
switch is gone (tests patch _new_backend).
- General plugin discovery treats computer-use providers as exclusive
(not eagerly imported, not listed as toggleable) and skips the bundled
computer_use/ category, same as memory/, context_engine/, cron_providers/.
- hermes tools and the Desktop toolset panel list installed providers next
to cua; computer-use doctor/status/permissions and the Desktop status
payload skip cua-driver/TCC probes when another provider is selected.
* feat(plugins): let auxiliary tasks inherit a base slot at read time
register_auxiliary_task's defaults start from a fixed shape and every key
is a full replace, so a plugin whose side model is a near-copy of a
built-in has to restate the whole block and silently drifts when that
built-in's shape changes.
inherit_from names a built-in or already-registered task whose effective
config becomes the base, resolved at read time so the plugin tracks the
base's current values instead of snapshotting them. Precedence is base,
then plugin defaults, then user config.
Self-referential inherit_from is rejected at registration rather than
left to the read-time guard: same-owner re-registration is allowed, so
the bad value would otherwise surface as unbounded recursion in a config
read. The guard stays for a hand-edited registry.
Closes #129189
* fix(plugins): keep an inherited aux base from being shadowed by write-only empties
_get_auxiliary_task_config merged the raw auxiliary.<key> block over the
inherited base, and Hermes's own config writers persist model, base_url,
api_key and reasoning_effort as "" when the operator expresses no preference
for them. _save_aux_choice writes those four fields unconditionally, and
_reset_aux_to_auto clears them for every known task including plugin-registered
ones via _all_aux_tasks(). So picking a provider for one task, or hitting
"Reset all to auto" once, left every routing key resolving to the empty string,
which maps to None and falls back to the main agent model. That discarded the
inherited base for exactly the keys the operator UI writes, which is the whole
point of inherit_from.
Filter "" out of the user layer when a task actually inherits. Only "" is
dropped, never any falsy value: provider "auto" is a deliberate operator choice
and reasoning_effort false is an explicit request for no reasoning. Tasks
without inherit_from keep today's behaviour, including their empty placeholders.
The picker also rendered the raw stored block, so a task inheriting a base's
provider reported "auto" while actually running on that provider. Display now
resolves the inherited view for inheriting tasks only, and falls back to the
stored block whenever the runtime resolver is unavailable.
Reported-by: Enough1122 (AI code review on this PR).
Refs #129189
* fix(dashboard,desktop): surface plugin-registered auxiliary tasks in Models settings
`PluginContext.register_auxiliary_task` already lists a plugin task in the `hermes model`
picker and layers its defaults into `auxiliary.<key>`, but every REST/UI surface behind the
Models page enumerated only the built-in `_AUX_TASK_SLOTS`:
- `GET /api/model/auxiliary` never listed the task
- `POST /api/model/set` (scope=auxiliary) rejected it: `400 unknown auxiliary task`
- `__reset__` and the empty-task broadcast skipped it; `_stale_aux_pins` ignored it
- Desktop Settings → Models and the dashboard ModelsPage hardcoded the built-in list
So a plugin's side model could only be pinned by hand-editing config.yaml per profile.
Fix, at the existing seams:
- `web_server_config._aux_task_slots()` = built-ins + plugin keys from
`get_plugin_auxiliary_tasks()`, fail-soft; every consumer in that module uses it.
- `GET /api/model/auxiliary` reads plugin tasks INSIDE `_profile_scope`, so a request for
profile B enumerates B's plugins (the plugin manager keys on the same context-local home) —
the profile-scope gap flagged on #40922. Built-in rows are byte-identical; plugin rows are
appended with `label`/`hint`/`plugin` from the registration.
- Desktop `model-settings.tsx` and dashboard `ModelsPage.tsx` render served plugin rows after
the built-ins using the server label/hint (pure `auxTaskRows`; no new i18n strings, no-op on
older backends).
Tests: real plugin loaded from a temp HERMES_HOME through PluginManager discovery, two
profiles in one process (listing scoped per profile; foreign task still 400), assignment /
broadcast / reset / stale-pin over a plugin slot, discovery failure leaves built-ins intact —
all red on main. Vitest covers the pure row merge and the rendered plugin row.
Fixes #40880. Salvages #40922.
Co-authored-by: Tranquil-Flow <tranquil_flow@protonmail.com>
* fix(plugins): inherited aux slots follow their base until the user pins one
Salvage follow-up for #129189 on top of #129209:
- An unknown or self-referential `inherit_from` now logs a warning and
registers the task without inheritance instead of raising, so one bad
base no longer fails the whole plugin load (the #129191 approach).
- `provider: auto` on an inheriting task means "no preference", like the
empty strings: "Reset all to auto" (CLI + dashboard) writes exactly that,
and before this it pinned the slot to the main provider while the model
was still inherited from the base (mismatched route).
- Once the user pins a route on the task (non-auto provider, model or
base_url), provider/model/base_url/api_key/api_mode/key_env/effort all
come from the task's own block. Mixing per key let a pinned provider pick
up the base's base_url and api_key.
- The CLI picker resolves the inherited view only for inheriting tasks.
Tests collapse to behavior contracts: base followed and changes
propagate, picker placeholders keep following, a pin wins whole, bad
bases warn and are ignored, re-registration cycles are cut.
Co-authored-by: Yuan Li <dskwelmcy@163.com>
* feat(dashboard,desktop): show which slot a plugin aux task inherits
Plugin aux rows (#40880) now carry `inherit_from` and, for inheriting
tasks, an `effective` {provider, model, base_url} resolved inside the
request's profile scope. Desktop Settings → Models and the dashboard
Auxiliary Tasks modal render an unpinned inheriting row as
"inherits Compression · <provider · model>" and offer "Follow <base>"
on a pinned one (writes provider auto, which resumes inheritance).
The dashboard test now writes plugin manifests/config as JSON (no yaml
dependency in the test env) and covers profile A/B isolation of the
same plugin slot: listing, effective route, pin persistence, follow and
base-change propagation each stay in their own profile.
* refactor: move aux-slot growth into topical siblings (code-health caps)
No behavior change. Keeps every touched file and function at or below
its value on main:
- agent/auxiliary_task_config.py: read-time `auxiliary.<task>`
resolution (plugin defaults, inherit_from, whole-route pin); still
re-exported as agent.auxiliary_client._get_auxiliary_task_config.
- hermes_cli/plugins_aux_tasks.py: register_auxiliary_task validation
and entry shape.
- web/src/lib/api-aux.ts: /api/model/auxiliary wire types.
- Desktop: aux row list, label resolver, deep-link highlight, route
summary, Follow button and the set-to-main route in
settings/aux-task-rows.tsx; one assignAuxiliary callback replaces
setAuxiliaryToMain. Built-in aux slot labels move from each locale
into <locale>_aux_tasks.ts (types_aux_tasks.ts holds the shape).
* test(install): cover treeless checkout migration
* fix(update): convert a treeless checkout before the dependency work, and on installer re-runs
A Desktop built before the blobless fix keeps walking history during
`hermes update`. On a treeless (tree:0) checkout every walk downloads trees
again, and #129514 filled a 434 GB disk during the minutes of dependency work.
The one-time conversion (#133016) ran at the very end of the update, after that
window. Both update hand-offs (update_completion._prepare and an older
updater's _update_takeover.prepare) now run it first, in the bootstrap
interpreter, and the late call is gone.
Re-running install.sh / install.ps1 over a treeless checkout now converts it
too. The filter is recorded only after the refetch succeeds, so a failure is
retried by the next update instead of leaving a blob:none filter over a
history that has no trees.
The pack cleanup also sweeps tmp_* transfer files that killed fetches left
behind. They are more than an hour old, a storm leaves gigabytes of them,
and git itself only prunes them after two weeks. The docs gain the recovery
steps for a disk that the storm already filled.
Co-authored-by: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
Co-authored-by: Yuan Li <dskwelmcy@163.com>
* test(install): start the treeless re-run fixture from a real treeless clone
The bare origin had no main HEAD, so the clone had no commits and stage_repository
re-cloned it as an interrupted install; a plain-path clone also ignores --filter.
The test passed on the unfixed installer. Use -b main and a file:// URL, and assert
the checkout was converted in place.
* test(install): run the treeless re-run case through install.ps1 too
* refactor(update): move the no-prompt git kwargs helper where the bootstrap can reach it
The convert-first step runs in the bootstrap interpreter, which cannot import
update_cmd (ruamel), so it carried its own raw os.environ copy (the spawn-env
guard flagged it) and lacked the Windows console-hide flags (#117781).
Move the body unchanged to _subprocess_compat.no_prompt_git_kwargs: prompts
off, credential helpers kept, consoles hidden. update_cmd keeps the
_no_prompt_git_kwargs name as an import because old updaters load it after
the checkout swap (tests/compat/old_updater_surface.json); existing callers
are untouched.
* feat(plugins): let plugins register Automation Blueprints via ctx
Plugins can now call ctx.register_automation_blueprint(key, *, title,
description, schedule_template, prompt_template, category, slots,
deliver_default, skills, tags). The fields mirror the built-in
AutomationBlueprint, and slots are BlueprintSlot field dicts, so the call
also crosses the plugin-host boundary.
- One lookup, cron.blueprint_catalog.list_blueprints() / get_blueprint(),
merges the built-ins with the active profile's plugin blueprints at call
time. /blueprint (CLI/TUI/messengers), /api/cron/blueprints and the
instantiate route all use it. Each dashboard route resolves inside the
?profile= scope, off the event loop.
- Keys are namespaced <plugin>:<key>, so a plugin cannot shadow a built-in.
A ':' in the key, a duplicate key, an unknown placeholder, a bad slot type
or a schedule that does not parse is warned and ignored at registration.
- Catalog entries carry source/plugin. Desktop fetches the catalog for the
profile it creates jobs in, keyed per profile, and shows the plugin name
on the blueprint row and in the "Start from" select. The dashboard shows
a plugin badge.
- A created job is an ordinary cron job with the prompt and schedule already
filled in, so it keeps running if the plugin is later disabled.
- The ctx content registrars (register_skill, now joined by
register_automation_blueprint) move to hermes_cli/plugins_content.py.
The dashboard's blueprint types move to web/src/lib/automation-blueprints.ts.
Closes #132373. Requested by @enckequity.
* fix(discord): a role-authorized member's speech in a voice channel reaches the agent
With DISCORD_ALLOWED_ROLES and no user allowlist, the voice listen loop
admits a role member through the adapter's guild-scoped role check, but
_handle_voice_channel_input rebuilds the turn's source from the
`/voice join` copy. SessionSource.to_dict() deliberately drops the
per-event role_authorized grant, so the gateway's own check saw a
principal with no grant and dropped every spoken turn from a role-only
member, while the same member's typed messages were answered.
The grant is now recomputed for the current speaker against the guild's
current member, with the meaning the message path gives it (the adapter
has a role allowlist and this user passes its gate). It is never
serialized and never inherited from whoever ran `/voice join`: a speaker
without the role, or no longer in the guild, is still refused.
Reported by andrexibiza (F4, voice half) on #128788.
(cherry picked from commit 6985ea6300671466b8cfa8e7948ee4c402bcb9e2)
* docs(voice): voice-channel access follows DISCORD_ALLOWED_ROLES too
(cherry picked from commit 7976b4aae1826a4e234f3f2a7cf796cb5b7ae98e)
* fix(discord): carry the role verdict into slash command events
Fixes #118958
(cherry picked from commit f5f538ef4b86a1d179e76292894372cfece8d2f3)
* docs(discord): state what the slash role flag means
Review feedback: the comment read as if the stamped role_authorized
value were a per-user role match. It is the same adapter-level verdict
the message path stamps: a role allowlist exists and the user passed
the slash authorization gate.
(cherry picked from commit e4165fc2ed7f65b9356ac75f90cab02aec85d70c)
* fix(discord): a native slash or /thread starter stamps the role grant for its own actor only
The salvaged #125620 commits stamp role_authorized on _build_slash_event
and _dispatch_thread_session from bool(_allowed_role_ids), which is only
right because today's two callers run _check_slash_authorization first.
Neither builder checks its actor itself, so any later caller that builds
an event before (or without) the gate would mint a grant for a member the
gate refuses.
The grant is now re-evaluated from the interaction in both builders
(_slash_role_grant): a role allowlist exists AND this actor passes the
slash gate, the meaning _discord_message_admission gives the flag. A
role-only member still reaches the agent, and an unrelated-role actor's
event is refused by the gateway even when built directly. The two
salvaged tests fold into one parametrized invariant over both builders
with the refusal row.
Recomputing the verdict from the actor follows #118966 by KoNit-K.
Reported by andrexibiza (F4, native half) on #128788 and #130303.
Co-authored-by: KoNit-K <konit.block@protonmail.com>
(cherry picked from commit 357e77d09263d25d7779d42f0eda3d352b3032a9)
* refactor(discord): move the slash gate verdict to a sibling mixin; adapt the thread-starter test
_evaluate_slash_authorization moves unchanged into adapter_slash_auth.py beside the new _slash_role_grant, so adapter.py stays within its code-health file cap. The thread-starter test passes a thread, as _dispatch_thread_session now takes one, and the voice role-grant test gains type hints.
* fix(gateway): voice turns drop the bound /voice join message id
A typed `/voice join` binds that message's source, message_id included, and
`_voice_input_source` rebuilt every voice turn from it. The join message is
not the voice turn's trigger: run.py exports source.message_id as
HERMES_SESSION_MESSAGE_ID (reply anchoring), and on main it also set the
presence bit in the prompt-pin key, so the note-gating cache miss this PR
fixes only surfaced for native slash-command joins. Clear it on the way out.
Review-fix for #127944.
[adapted onto main: #133463 resolves the speaker name inline in _voice_input_source, so the
clear lands in main's block; the test moved to test_discord_turn_prompt_parity.py, a typed
`/voice join` id on the bound source]
(cherry picked from commit fe70c11bdabdb1ea0b9494880146f4cf16b19ad0)
* fix(discord): a voice speaker outside the member cache still gets a name
guild.get_member() is cache-only, so a speaker who had never typed in the
guild (no member intent, or joined voice straight away) rendered as the bare
user id while their text turns render a display name: the session-context
prompt differed again on every switch. Fall back to the cached global user
(global name / username) before giving up; a guild nick still wins when the
member is cached. Suggested in the first review, reproduced independently
(F3 on the thread).
[adapted onto main: #133463 resolves the speaker name inline in _voice_input_source, not
through DiscordAdapter.voice_member_display_name, so the fallback is a small helper there. It
runs after the /voice join invoker's bound name, which is the nickname their typed turns
carry; a cached global name would differ from it. Tests moved to
test_discord_turn_prompt_parity.py]
(cherry picked from commit 295a01f3ab88f07f3bac22f8939c42bca928c398)
* fix(auxiliary): support MiniMax OAuth side tasks
_resolve_registry_branch now has an oauth_minimax arm that resolves a
callable bearer via resolve_minimax_oauth_runtime_credentials and wraps
the resulting Anthropic SDK client in AnthropicAuxiliaryClient(..., is_oauth=True).
Without it, minimax-oauth falls through to the oauth_device_code/oauth_external
branch and silently returns (None, None), re-routing every aux task pinned
to that provider to the Step-2 fallback chain.
The callable bearer mints a fresh access token per outbound request because
MiniMax's tokens live ~15 minutes and a static string would 401 mid-session.
* fix(auxiliary): MiniMax OAuth route keeps third-party wire contract
Review fix for the oauth_minimax arm (ehz0ah, #109316): is_oauth on
AnthropicAuxiliaryClient is not 'credential came from OAuth' — it enables
Claude Code-native request behavior (mcp__ tool-name wire prefixing,
identity/system rewrites, response prefix stripping) that only
api.anthropic.com routes qualify for. api.minimax.io is a third-party
Anthropic-protocol endpoint and anthropic_route_is_oauth(base_url,
token_provider) is False for it by contract
(tests/agent/test_anthropic_route_oauth_identity.py), so the hardcoded
True sent mcp__-prefixed tool names to a host that never round-trips
them.
Derive the flag via anthropic_route_is_oauth(base_url, token_provider)
instead — same rule the custom anthropic_messages arms already use —
keeping the callable bearer (per-request token refresh) intact.
The regression test asserted the wrong invariant; it now pins
_is_oauth False, and a new witness test proves tool names reach the
wire unprefixed (read_file stays read_file, not mcp__read_file;
session_search is not aliased).
* refactor(auxiliary): registry auth-type table; MiniMax OAuth arm in a topical sibling
Clears the code health ratchet on the branch:
- FILE_LINES: agent/auxiliary_client.py 8469 > 8434 (main's cap, may only go
down). The PR's new MiniMax OAuth arm moved out of the facade into
agent/auxiliary_client_minimax.py, following the <stem>_<topic>.py sibling
layout; the arm late-imports the facade so the seams tests patch
(hermes_cli.auth.resolve_minimax_oauth_runtime_credentials,
agent.anthropic_adapter.build_anthropic_client) are exactly where
production reads. Facade now 8432 lines.
- HX011: the auth_type if/elif ladder in _resolve_registry_branch became a
dict -> handler table (REGISTRY_AUTHTYPE_ARMS in
agent/auxiliary_client_registry.py, the registry twin of
_EXPLICIT_PROVIDER_BRANCHES). Behavior identical: same arms, same
log-once fall-throughs on the facade.
- BLE001: the arm's blind except Exception around the runtime credential
resolution is a deliberate absent-credentials boundary; it now logs with
exc_info=True so the traceback surfaces (verified clean under the pinned
ruff with --ignore-noqa; no waiver comments).
No behavior change: the 3 PR tests, test_auxiliary_client.py (240),
test_anthropic_route_oauth_identity.py, test_auxiliary_client_resolve_dedup.py,
test_missing_provider_credentials_hint.py, bedrock/vertex/minimax suites all
green; scripts/check 11 checks ok.
* fix(auth): serialize MiniMax OAuth refresh and keep it a credential-only write
teknium1's request-changes review of #133534, all four findings plus nits:
1. Concurrent refresh logged the user out (major): _minimax_fresh_state now
holds _auth_store_lock across read -> refresh -> save and re-reads state
inside the lock, so a second concurrent consumer (main agent + aux task)
sees the first caller's new expiry instead of replaying the single-use
refresh token into refresh_token_reused and quarantining the winner's
fresh tokens. Lock timeout outlives the refresh POST (codex pattern).
2. Aux refresh overwrote active_provider (medium): _minimax_save_auth_state
gains set_active; the refresh and quarantine paths persist with
set_active=False (the _save_provider_state_to_source rule), only
_minimax_oauth_login sets active.
3. Warning spam when logged out (medium, #21521's complaint): the arm's
expected absent/not-logged-in/AuthError fall-throughs are debug-once
(contract test_auxiliary_client_resolve_dedup.py); only a genuinely
unexpected exception keeps exc_info, logged once.
4. Probe mode made a network call (minor): under _aux_probe_active() the arm
reads get_provider_auth_state("minimax-oauth") raw and stubs when
access_token + inference_base_url are present, skipping the refresh
(mirrors credential_pool.py::_seed_minimax_singleton).
Nits: the arm body lives directly in auxiliary_client_registry.py (one new
module, agent/auxiliary_client_minimax.py deleted), the in-tree
try/except ImportError wrappers are gone (only the build_anthropic_client
SDK-absent boundary remains, matching _try_anthropic), and the misnamed
..._with_oauth_semantics test is now ..._third_party_wire.
Tests: concurrent loopback-portal refresh race (1 POST, 0 errors, tokens
intact), aux refresh preserves active_provider, login still sets it,
logged-out resolutions are debug-once without traceback, probe mode with an
expired token makes 0 HTTP calls.
Co-authored-by: Ubxty <ubxty@users.noreply.github.com>
* fix(process): a kill's result stands on every backend, and a failed scope stop never counts as stopped (#133648)
* fix(process): a kill's result stands on every backend, and a failed scope stop never counts as stopped
Follow-up to #133434, which fixed these races for local processes only.
Non-local backends (spawn_via_env) settle through _env_poller_loop, which
still wrote the session's final state without the session lock:
- It set exit_code before _finish_exited, so a kill that landed first ended
as killed with the sandbox's exit code. The exit now goes through
_finish_exited alone, and a committed kill keeps its code.
- Its backend-lost path set exited/-1/lost directly, replacing a recorded
kill. It now goes through _finish_exited(reason="lost"), so the kill stands
and the completion is still published by its owner.
- A log delta (or a log-rotation reset) fetched before a kill was applied
after it, so the session's output no longer matched the kill's snapshot.
Both are now dropped once the session has exited, decided under the lock.
A failed systemd scope stop could still count as a successful stop:
- A recovered PID that died between get() and _signal_kill ignored the stop
result and returned a plain already_exited. It now reports
scope_stop_failed.
- kill_all counted already_exited as stopped even when the scope stop
failed, and only looked at _running, so a session that exited after being
picked kept a live scope that no later sweep retried. Every kill path now
stops the scope through _stop_scope, which marks a failed stop pending;
kill_all does not count a result with scope_stop_failed and also retries
finished sessions whose stop is pending.
_log_delta_command moves unchanged into tools/process_registry_env_log.py
(ProcessRegistry._log_delta_command stays as an alias) to keep
process_registry.py under its size cap.
Refs #132358. Both gaps were found by @andrexibiza in the review of #133434.
* fix(process): pruning keeps a session whose scope stop is pending
kill_all retries a failed scope stop through the finished session, but
_prune_if_needed still evicted that session by TTL (measured from process
start, so a long-running process was prunable the moment it finished) or
by capacity, leaving a possibly live scope with no retry target. Sessions
with _scope_stop_pending are now exempt from both until the stop succeeds;
capacity pruning drops the oldest prunable session instead.
Found by the independent review of #133648.
* fix(discord): buttons stop accepting a user revoked after connect
Component views authorized clicks against the adapter's connect-time
_allowed_user_ids, so a user removed by `hermes pairing revoke` (CLI or
dashboard, another process) or a .env edit kept approving exec commands
until a gateway restart. A user found only in that snapshot is now
confirmed with the gateway's per-call authorization check, as Matrix
approval reactions are since #130920; role, allow-all and pairing grants
are unchanged, and an unwired adapter keeps the snapshot.
(cherry picked from commit a6ced15a6000247b9736cede3cbf4c6ed0c11265)
* fix(discord): confirm a snapshot wildcard with the live check too
A '*' in the connect-time DISCORD_ALLOWED_USERS snapshot still
short-circuited component auth, so removing the wildcard from .env after
connect left every button open until a restart. The gateway's live check
already honours '*' itself, so route the wildcard through it like a
listed ID. Also drop the redundant _authorization_check guard
(_is_sender_authorized already returns None when no check is wired) and
trim the docstring.
* test(discord): trim component live-auth tests to two invariants
Keep the parametrized helper test (now also covering a '*' snapshot) and
the end-to-end revoked-user deny via send_exec_approval. Drop the
role-grant case: live_auth only gates the user-set branch, and the role
OR path is already covered by the existing *_accepts_role_allowlist tests.
* refactor(discord): move _component_check_auth into adapter_component_auth to keep adapter.py under its size cap
Code-health ratchet: plugins/platforms/discord/adapter.py was 7389 lines
against a cap of 7370 after this stack added the live-auth confirmation to
the component click check. Move the shared component authorization helper
(byte-identical body) into a topical sibling; the view's _check_auth
late-imports it, and the direct-helper tests import it from its new home.
* fix(discord): a programmatic voice join binds the requested text channel on every path
join_voice_channel(channel, text_channel_id=..., source=...) is the programmatic join API, and it
kept the requested binding only on a cold join: the already-connected paths (same voice channel, or a
move) returned before writing it, so a successful join to another text channel kept transcribing
into the old one. All three paths now apply the binding through one helper. Moving to another text
channel drops speech still buffered for the old one, as /voice join does, and the old channel's bound
source unless a new one is given: its chat would keep routing turns into the old conversation.
With no bound source, a voice turn built a synthetic one with chat_type "channel". A typed message in
a guild text channel is "group" (or "thread" under its parent), so the voice turn keyed another
session. It now goes through build_source as a typed message does, with the guild, thread and parent.
/voice join, the only in-repo caller, already writes its binding itself after joining, so this is
latent until something calls join_voice_channel with a text channel (the open auto-join PRs would).
Reported by @andrexibiza as F7 in #132474.
* fix(update): drop the duplicate tmp-pack sweep from pack cleanup
#133567 taught git_pack_tidy to delete tmp_* transfer files, but
gitlock.clear_stale_tmp_packs already does that a few lines earlier in the
same update step (since Sep 6), with a stricter guard: it skips while any
git process runs. Remove the copy, keep the shared _unlink helper, and
correct the docs line about when the files go away.
* feat(telemetry): memory and compression rows say why they failed or were skipped
Fleet telemetry shows built-in memory writes failing 34-47% and automatic
compression failing ~15%, and several merged fixes each target one cause
(#133038 for memory, #133054/#133040/#133051/#133032 for compression). Both
metrics only carried outcome, so the fleet could not tell which cause a fix
moved.
- hermes.memory.op.count and hermes.compression.count gain one closed-enum
dimension, failure_class; `none` on success.
- Memory: every refusal/failure return in tools/memory_tool.py and
tools/memory_tool_store.py names its class (no_match, ambiguous,
over_budget, would_empty, drift, read_failed, retry_cap, scan_blocked,
missing_content, missing_old_text, invalid_args, stale_entry, disabled,
staged, gate_refused) through a context variable; the result the model
sees is unchanged. Plugin providers report provider_error / exception.
- Compression: the attempt log's own failure_class, collapsed onto the
compressor's closed names (exception:<Type> / rollback:<Type> keep only the
prefix, anything else reads other, a classless failure unknown). Skipped
rows carry the skip reason, and the skip set is now defined once in the
contract.
- Package schema v4 / event schema v4; v3 field sets of both metrics stay
valid so pending rows drain. Doc table rows and the consent copy (CLI
setup and every Desktop locale's collectedActivity) name the new reason.
* fix(telemetry): compression verdict reads the stored class; consent offer names the reason
Review follow-ups for the failure_class split: finish_compression_attempt now
decides skipped/failed from the normalized class it records (a padded or
mixed-case skip name no longer reads failed with a skip reason, and a
non-string class no longer raises); the pre-chat consent offer names the new
reason like the setup copy; the setup copy keeps 80 columns; the doc says a
refused batch's op rows carry the class of the op that stopped it.
* fix(build): refuse a completed-install receipt whose packages lost their manifests
The reuse receipt check in node-deps.mjs accepted a tree where every locked
directory existed but package contents had been stripped (partial cleanup or
snapshot restore). npm trusts the hidden lockfile and never repairs such
entries, so every 'hermes update' skipped npm ci, the TUI build died on the
missing esbuild manifest, and the stale receipt kept the loop closed (#128935).
Probe each locked entry's package.json — the same file product builders
resolve tools through — so a gutted tree falls back to a real npm ci.
* fix(gateway): return a failed reconnect candidate's held inbound to its predecessor
7ef5aca075 (#133228) handed held Telegram inbound over only when a
replacement was published, on the premise that a candidate that fails to
connect never owns the hold queue. That is false: TelegramAdapter.connect
failure sets a retryable fatal error, so updates PTB already acked are held
on the candidate, and disconnect() salvages pending text batches into the
same queue. The runner then disposed the candidate and those events were
lost for good.
After every candidate dispose path (primary retryable and exception
branches, secondary connect-raised teardown, secondary not-installed
teardown and the outer secondary except), hand the candidate's held events
back to the retained predecessor with the existing hand_over_held_inbound.
The predecessor's queue then moves to the eventual replacement at publish,
and the candidate's successor weakref forwards any late hold.
Fixes #133399
Reported-by: j2h4u (Maxim Brashenko)
* test(gateway): cover held inbound on failed Telegram reconnect candidates
Parametrize the existing hand-over test over a candidate whose connect
returns False or raises, holding one event in connect and salvaging one in
disconnect; the published replacement must deliver all three once. Add one
secondary-path case: a not-installed candidate returns its holds to the
retained predecessor (#133399).
* fix(gateway): do not hand an installed reconnect adapter's hold queue back to its predecessor
Gate finding C01.r1.2c [High]: the hand-back added for failed candidates
also ran in the post-install exception/cancel branches. After a successful
secondary install (or a primary publish) the predecessor already forwards
to the new adapter; if the task was cancelled during redelivery (gateway
teardown cancels secondary reconnect tasks) the hand-back linked the new
adapter back to the predecessor, and any hold - e.g. the disconnect()
teardown salvage - ping-ponged between the two until RecursionError.
Break the cycle at both ends: adopt_held_inbound clears the adopter's own
successor link (it owns the queue again), and the runner skips the
hand-back when the adapter is the one installed in its slot. Covered by a
cancel-after-install row on the secondary hand-back test.
* refactor(gateway): name hand_over_held_inbound's params source/target
Gate finding C01 quality [Low]: since #133399 the helper also runs in the
reverse direction (failed candidate -> retained predecessor), so the
hand-back calls read hand_over_held_inbound(adapter, predecessor) against a
(predecessor, adapter) signature. Neutral source/target names match both
directions. Every caller passes positionally, so no call site changes.
* test(gateway): cover the hand-back when a secondary attempt's connect raises
Gate finding C01 quality [Low]: the secondary hand-back test stubbed
_secondary_reconnect_attempt, so only the not-installed branch ran and the
hand-back in the attempt's own configure/connect teardown could be deleted
with every test green. Add a connect-raises row that runs the real attempt
(profile/config seams patched) with a candidate whose connect raises after
holding; it is red without that hand-back. Same test function, no new one.
* test(gateway): pin where an installed secondary's held inbound lands on cancel
The cancel-after-install row asserted only that the salvaged event sat in
predecessor + failed combined, which passes wherever it lands, so it pinned
neither half of the r1 High fix (fold2 finding: mutating the secondary
installed-guard to `if True:` stayed green). Assert the installed adapter
keeps its queue, the predecessor stays empty, and the forward link points
predecessor -> installed adapter with no reverse link.
* feat(providers): add Solstice as a secret-launch OAuth inference provider
Per-user-quota OAuth inference for Hermes Agent, hidden by default until
public launch is cleared. Auth (OAuth2 + PKCE with the user's account) is
brokered by nous-account-service, which holds the confidential client secret;
inference goes direct to the per-user-quota endpoint.
Built fresh on current main; commit history carries only the codename.
Provider surface:
- plugins/model-providers/solstice/ — profile + hidden enable gate
- hermes_cli/solstice.py — PKCE loopback login, NAS-brokered config/exchange/
refresh, token store, quarantine; callback path unchanged
- agent/solstice_adapter.py — per-user-quota transport over the native
adapter's OpenAI-compatible facade
- providers/: _HIDDEN_GATES + register_hidden_provider_gate + a hidden filter
in list_providers (kept out of the picker/registry/doctor until enabled)
- wired into main.py (_PROVIDER_MODEL_FLOWS), auth_commands.py (_PROVIDER_ALIASES,
_OAUTH_ADD_SPECS, _is_known_provider allowlist), models.py (catalog fetcher),
runtime_provider.py (resolution rung), model_setup_flows.py, agent_runtime_helpers.py
Kept verbatim (the live wire contract, until the broker is renamed): the
inference host, the OAuth scope, the deployed broker routes, and the
registered redirect callback path.
The enable gate is unchanged (open item: gate on stored credentials).
Verified: 226 focused tests pass; runtime resolution, client construction
and auth-add all exercise the real paths.
* feat(providers): pre-release profiles stay off discovery until signed in
ProviderProfile gains `hidden` and `listed()`. A hidden profile is left
out of every surface that offers providers (the hermes model / setup
lists, /model and the TUI inventory, the provider catalog behind the
dashboard accounts tab, the hermes auth add picker) until the user signs
in by name and its credential pool holds a row. Resolution by name is
never gated, so --provider, model.provider and `hermes auth add <name>`
keep working.
This replaces the PR's global _HIDDEN_GATES registry plus a per-provider
enable function, which returned True unconditionally and showed the
provider to every user.
* feat(auth): brokered PKCE, expiry-driven rotation and aux on plugin OAuth rows
Generic plugin-OAuth support that the Solstice provider needs. None of it
names a provider:
- OAuthPKCEConfig.token_request: a confidential client whose secret lives
behind a broker hands the grant to the plugin in place of a token_url
POST. The old refresh_token is kept when the response omits one, which
Google does on refresh.
- Plugin OAuth rows rotate 2 minutes ahead of their stored expires_at_ms
(shared with the anthropic rule). Opaque bearers such as Google's ya29.*
carry no JWT exp, so this stored expiry is the only clock they have.
- A row whose refresh grant just went DEAD stays DEAD on the inference 401
that follows; before this it was re-benched EXHAUSTED and the dead token
was replayed every cooldown.
- The auxiliary client serves an OAuth plugin that ships create_client from
its pooled row on the main runtime's endpoint (compression, titles). It
used to return "not directly supported".
- A 401 on a plugin OAuth row prints a re-sign-in hint, not the API-key
advice.
- The Gemini native adapter exposes its method names, auth headers and
error builder so a Bearer-auth subclass can reuse it without the API-key
remedies.
* refactor(solstice): rebuild the provider as a standard model-provider plugin
The whole provider now lives in plugins/model-providers/solstice/ on the
generic seams:
- auth.py: NAS config discovery plus the brokered exchange/refresh, given
to the generic PKCE flow as token_request. A refresh that NAS rejects
(it folds every Google failure into 502) is treated as a dead grant
needing re-login.
- transport.py: a GeminiNativeClient subclass on the
generateContentPerUserQuota methods with Bearer auth (~30 lines, where
the PR duplicated a 257-line adapter).
- __init__.py: a hidden profile (create_client, fallback models,
default_aux_model).
Removed from core: hermes_cli/solstice.py, agent/solstice_adapter.py, and
the provider's branches in main.py, auth_commands.py, models.py,
runtime_provider.py, model_setup_flows.py and agent_runtime_helpers.py.
The unused SOLSTICE_CLIENT_ID env read is gone too.
* fix(providers): listed filter reads the live provider table; parity covers the hidden case
listed_canonical_providers() read the CANONICAL_PROVIDERS binding
captured in models_catalog_static at import. Every list consumer used to
read hermes_cli.models.CANONICAL_PROVIDERS, so a provider appended there
at runtime vanished from list_available_providers(). It now reads the
models binding again.
The desktop parity contract now covers the listed universe in both
directions: every provider `hermes model` offers is configurable, and an
unlisted pre-release provider appears on neither Providers tab.
* chore: map pberard@nvidia.com to nv-pberard
Co-author of the #129431 salvage (Co-authored-by trailer on the
contributor fix commit).
* fix(compaction): bench a failing summary model instead of blocking compaction
Two fallback summaries in a row tripped the anti-thrash breaker even when
each fallback compaction was effective, so automatic compaction was blocked
and the session rode into the provider's hard context limit. Only
ineffective compactions now trip the breaker; a fallback streak >= 2 instead
benches the summary model (deterministic drop, one probe per recovery
window), which still stops the paid fallback loop from #63008. force=True
(/compress) always calls the model.
Salvaged from #129431.
Co-authored-by: Pascal Bérard <pberard@nvidia.com>
* fix(compaction): persist the summary-model bench probe deadline
The bench probe clock was in-memory monotonic time, so every rebuilt
compressor (the gateway rebuilds the AIAgent on cache eviction) came up
with a durable fallback streak but no clock, armed a fresh 300s window and
benched again. If rebuilds arrive faster than the window, the summary model
is never re-probed and the session stays on deterministic summaries until
a manual /compress.
Store the deadline as wall-clock time on the session row (model_config
key, the same durable path the proactive-prune runway uses) and load it in
bind_session_state, mirroring the anti-thrash recovery deadline (#100185).
A deadline more than one window in the future (clock jumped back) is
re-armed from now.
* test(compaction): trim bench tests to the two invariants
Keep one test per invariant: effective fallback compactions no longer block
auto-compaction (test_fallback_boundaries_wait_for_real_effectiveness_verdict)
and the bench probe deadline survives a compressor rebuild (which also pins
that a streak >= 2 benches the summary model). Drop the window-arithmetic
tests that pinned the old in-memory clock and the unrelated session-reset
counter test.
* refactor(compaction): move pre-LLM summary skips into context_compressor_prellm
Code-health ratchet: agent/context_compressor.py grew past its cap on main.
Move _feasibility_skip, _fallback_streak_skip and _set_fallback_probe_at
(plus their constants) byte-identical into a PreLlmSkipMixin in the new
agent/context_compressor_prellm.py sibling, matching the existing
SummaryDispatchMixin layout, to keep context_compressor.py under its size
cap. No behaviour change; no test patches the moved names.
* test(compaction): move the bench probe rebuild test into its own file
Code-health ratchet: tests/agent/test_context_compressor.py grew to
3847 > 3829. New tests go in a topical test_<stem>_<topic>.py.
* fix(compaction): carry the bench probe deadline across rotating compaction
Rotating compaction (compression.in_place: false) calls on_session_start
with boundary_reason="compression"; bind_session_state zeroes
_fallback_probe_at and reloads it from the fresh child row, which has no
model-config key. The fallback streak was carried but the deadline was
not, so the benched summary model was re-benched with a new window on
every rotation and never probed again. Capture the deadline before the
rebind and persist it onto the child row alongside the streak.
* refactor(compaction): own the bench probe deadline load in PreLlmSkipMixin
The mixin already owns _set_fallback_probe_at and the model-config key;
the matching load lived inline in bind_session_state, which forced
context_compressor.py to import the key. Move it to
_load_fallback_probe_at() and make the key private to the mixin module.
* refactor(compaction): refresh stale skip/anti-thrash wording
record_completed_compaction carried two overlapping comments and logged
"via pre-LLM feasibility skip" although the path now also covers the
benched summary model; the anti-thrash comment still said it drops
"counters" when only the ineffective count is reset. Split the long
streak/skip tuple assignment in the stall-retry path and drop the
`is True` hedge (the flag is always a bool on the built-in compressor).
* test(compaction): trim the redundant fallback-streak gate tests
The rotation-state gate test now pins only that a durable fallback
streak never blocks should_compress (renamed to say so); its
clear-and-reload tail re-tested the loader, not the gate.
test_fallback_streak_alone_does_not_block_compaction duplicated
test_fallback_boundaries_wait_for_real_effectiveness_verdict, which
already asserts should_compress stays open with a streak of 2.
* refactor(compaction): name the pre-LLM summary skip in the fallback log
_fallback_summary_for_window logged "Feasibility skip" and the compress()
comment said "feasibility skip", but the flag is now also set when the
summary model is benched. Say "Pre-LLM summary skip" so the log matches
both causes. Wording only; no test asserts the old text.
* fix(desktop): a resume keeps the owner hint of a session with no row
#132017 started dropping a remembered owner hint unless a connection-tagged
cached row agreed with it. Hidden sessions (Bot Chat, plugin opens) never
get a sidebar row, and for them the hint is the only owner record, so a
remote bot's pathname-driven resume lost its route and went to the local
primary: Desktop core E2E remote-secondary has been red on main since.
Only an untagged or disagreeing row now makes the hint stale; no row
keeps it.
* ci: Desktop session open/resume changes run the core E2E
Every core spec drives open, resume and switch, yet changes under
apps/desktop/src/app/session/ skipped the suite on the PR; #132017
changed resume that way and main went red on remote-secondary.
* fix(pm): cold updates on Python < 3.13 trust a stored intermediate CA
An install still on its legacy 3.11 venv runs its first `hermes update`
from that interpreter, which downloads uv and Python 3.14 with plain
urllib before PM's worker (and truststore) exists. Windows can cache Let's
Encrypt "Root YR" in its intermediate store without ISRG Root X1; GitHub's
asset CDN now chains through Root YR, so 3.11 rejected every download with
"unable to get issuer certificate" while PowerShell and 3.14 accepted it.
Python 3.13+ sets VERIFY_X509_PARTIAL_CHAIN by default. Set it on the PM
downloader's opener so the legacy interpreter applies the same rule;
hostname, expiry and pinned sha256 checks are unchanged.
Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
* chore: map LuwenOosten for salvage of #133340
* fix(telegram): reuse the shared platform TLS context for every Telegram client
Every Telegram httpx client (DoH fallback discovery, both PTB HTTPXRequest
pools, and each lazily rebuilt TelegramFallbackTransport pool) was built with
verify=True, so httpx ran ssl.create_default_context() and a full CA load on
the gateway event loop per client, per reconnect. A slow trust store stalled
the loop for most of the connect deadline on every attempt.
agent.ssl_verify already caches one platform-trust client context
(_shared_context(None)); expose it as platform_ssl_context() and pass it as
verify= to every Telegram client and transport. Verification is unchanged
(CERT_REQUIRED + check_hostname via the truststore-injected context), proxies
still flow through httpx's proxy/env handling, and the first build is ~3 ms
because the platform verifier loads no CA file.
This is the minimal slice of #133340: the PR's single-flight Future, worker
thread and to_thread client construction are unnecessary once the context is
cached and shared.
Fixes #133339
Salvaged from #133340.
* refactor(telegram): import platform_ssl_context at module top in the adapter
adapter.py is over the 2000-line target and frozen at its main size by the
code-health ratchet; the local import + named context variable grew it by 3
lines. A module-level import and calling the cached accessor at each use site
(it returns the same identity) keeps the change net-zero, and the test now
patches the name where production reads it.
Co-authored-by: luwen <142317085+LuwenOosten@users.noreply.github.com>
* fix(ssl): honour SSL_CERT_FILE in the shared Telegram TLS context
Gate finding C02-1 (High regression): before this stack Telegram passed
verify=True, and httpx built create_default_context(cafile=$SSL_CERT_FILE),
so a private CA supplied only through SSL_CERT_FILE was trusted.
platform_ssl_context() always returned the truststore platform context,
so on macOS that CA was rejected (probed on a real local TLS server).
When SSL_CERT_FILE names an existing file, return the cached bundle
context from _shared_context(path) — still one load per process, never
per reconnect. A missing or stale path keeps the platform context.
SSL_CERT_DIR (BASE used capath) stays uncovered.
* refactor(telegram): import platform_ssl_context at module top in telegram_network
Gate finding C02-2 (Low): the function-local import in
discover_fallback_ips did not match the adapter, which already imports it
at module top. agent.ssl_verify imports only stdlib at module level, so
there is no cycle or import cost.
* test(telegram): cover the DoH discovery client in the shared TLS context test
Gate finding C02-3 (Low): the kept test only checked the PTB clients, so
dropping verify= from the discover_fallback_ips AsyncClient still passed
(mutation probe). Record the AsyncClient kwargs inside the same test.
* test(telegram): drop raising=False from the platform_ssl_context patch
Gate quality Low (C02 r1): platform_ssl_context is a real module-level
import in the adapter, so raising=False only hides a rename by planting
an unused attribute instead of failing at setup.
* fix(ssl): trust SSL_CERT_FILE on top of the platform store, not instead of it
Gate finding (C02 fold2 #1, High regression): platform_ssl_context()
built the SSL_CERT_FILE context from the un-injected stdlib class with
only that bundle loaded. Before this stack the Telegram clients used
httpx verify=True, i.e. ssl.create_default_context(cafile=SSL_CERT_FILE)
on the truststore-injected class, which trusts the bundle PLUS the OS
store. An operator exporting a private-CA-only SSL_CERT_FILE therefore
lost api.telegram.org (SSLCertVerificationError, live-probed).
Give the env bundle union semantics under its own cache key: the context
is the injected ssl.SSLContext (PROTOCOL_TLS_CLIENT: CERT_REQUIRED +
hostname checking) with the bundle loaded on top. A provider ssl_ca_cert
keeps its replace semantics through resolve_httpx_verify unchanged.
* test(ssl): assert the platform_ssl_context contract in the ssl_verify tests
Gate finding (C02 fold2 #2, quality): the SSL_CERT_FILE contract of
platform_ssl_context (bundle honoured on top of the platform store,
cached once, distinct from the plain platform context, CAs loaded) is an
agent/ssl_verify.py behaviour, but it was asserted inside the Telegram
close-wait test. Move it into the existing SSL_CERT_FILE test in
tests/agent/test_ssl_verify.py (no new test function); the Telegram test
keeps only its every-client-shares-the-context checks.
* refactor(ssl): collapse the duplicated union arm in _shared_context
The SSL_CERT_FILE (union) arm repeated the platform arm line for line
plus a load_verify_locations call. Build the context from one of two
classes (stdlib for a replacing provider bundle, injected for everything
else) and load the bundle once when there is one.
Equivalence probed on 44a807c2c7 vs this head for (path, False),
(path, True), (None, False): identical type, verify_mode, check_hostname
and cert_store_stats().
Fold3 finding 1 (reuse/quality Low).
* test(ssl): pin the SSL_CERT_FILE context to its union cache slot
The getattr(bundle_ctx, '_ctx', bundle_ctx).cert_store_stats() hedge
asserted nothing a non-empty certifi bundle would not satisfy either way.
Assert instead that platform_ssl_context() returned the (bundle, union=True)
cached context, which goes red if the platform path ever builds a
replacing (union=False) bundle context.
Fold3 finding 2 (quality Low).
* docs(ssl): note SSL_CERT_FILE sits on top of the platform store
The module docstring still said only explicit per-provider config sits
above the platform store; platform_ssl_context() now honours an exported
SSL_CERT_FILE bundle in addition to it.
Fold3 finding 3 (quality Low).
* docs(user-stories): add 45 business and professional stories (#133708)
Prepends 45 verified first-person stories of Hermes Agent used at work
(companies, agencies, practices, shops and solo operators) to
userStories.json. Every quote is verbatim from the linked public source.
No existing entries are changed or removed.
Co-authored-by: witcheer <208820874+notwitcheer@users.noreply.github.com>
* feat(plugin-catalog): add fxmacrodata
* chore: map contributor email for @roberttidball
* chore(plugin-catalog): fxmacrodata disclosure at merge
Normalizes 'Disclosure:' to 'Disclosure —' and appends the reviewed note on the system-prompt section, tool count and remote MCP behaviour (salvage of #133564).
* catalog: bump honcho (pin 3dd3b7a)
Adopts plastic-labs/honcho#1203: the plugin sends X-Honcho-Host and
X-Honcho-Plugin on its Honcho client. No capability changes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* chore(plugin-catalog): honcho disclosure at merge
* chore: map contributor email for honcho (#133486)
* fix(update): verify a parked branch from the commit graph before git cherry
git cherry's patch-id walk needs upstream-side trees, which a tree:0
partial clone lazy-fetches in many promisor batches; one failed batch
made the guard call a clean checkout unverifiable and skip the update
(#124767). Count parked-only commits with rev-list (commit graph only)
first, keep cherry as a refinement, and degrade a cherry failure to the
conservative commit count instead of unverifiable.
* fix(update): the parked-branch cherry refinement never lazy-fetches
`_assess_parked_branch_switch` still ran `git cherry origin/<target>` whenever the
commit-graph count found parked-only commits. On a tree:0 partial clone that
patch-id walk lazy-fetches a tree batch per upstream commit, and nothing bounded
it: the #131444 trace identifies exactly this child (`git cherry origin/main`,
6 h 45 m, 332 packs / 180 GiB) as the writer behind the silent disk runaway.
Run the refinement with GIT_NO_LAZY_FETCH=1 (the NO_LAZY_FETCH_ENV every other
read-only updater probe already uses). A missing object now fails `git cherry`
fast and the conservative rev-list count stands in, so the guard can never turn
into a fetch loop; full clones keep the patch-equivalence refinement.
Test: a reachable promisor remote, a parked branch one commit ahead — the guard
must not add a single pack. Red on origin/main and on #124777 alone (4 packs
after vs 2 before), green here.
Refs #131444. Supersedes the bound/offline variants in #125529, #126693, #128335.
* chore(tests): explicit encodings in the parked-branch guard tests
The Windows footgun ratchet flags every bare read_text()/write_text() in a
touched file; this file predates the rule.
* test(update): the parked-branch fetch guard is proven by a Trace2 fetch count
Review on #132378 (@andrexibiza) showed the regression oracle was the net pack
count: with foreground auto-GC (gc.auto=1, autoPackLimit=1, autoDetach=false)
a guard with the override removed still made two lazy fetches and left the
count at 2 -> 2. The test now captures Trace2 around the assessment and
requires zero fetch child starts, pins global/system git config, and runs
plain and foreground-GC variants; removing the override fails both.
The cherry child's environment now comes from noninteractive_git_env() like
the other no-lazy-fetch callers, instead of a raw os.environ copy that the
spawn-env guard rejects.
* plugin-catalog: add session-token-counter
* chore: map vsemagush@gmail.com to @Vsemagush for attribution (catalog PR #133144)
* feat(plugin-catalog): add hermes-teammates plugin
* chore(catalog): pin pass-secrets @ 485f105 — publish-identity scrub (replaces 55189d3, same tree)
* feat(plugin-catalog): add error-ledger
* feat(plugin-catalog): add the error-ledger catalog card
* feat(plugin-catalog): add stalkchain
* chore(plugin-catalog): stalkchain disclosure at merge
Applies the reviewed Disclosure line from the catalog sweep review (catalog PR #133356).
* feat(plugin-catalog): add bento plugin
* chore(plugin-catalog): bento disclosure at merge
Applies the reviewed Disclosure line from the catalog sweep review (catalog PR #133524).
* fix(catalog): refresh Parallel Search listing status
* feat(catalog): add Tabro local browser automation plugin
* fix(catalog): pin Tabro browser compatibility and host gate update
* fix(catalog): pin Tabro relay and installation review fixes
* chore(plugin-catalog): tabro disclosure at merge
* chore: map contributor email(s) for PR #132619
* Re-pin agora to v2.0.7 (fix #3: dashboard 500)
* Re-pin agora to v2.0.7 (7e7e270 — includes test fixture fix)
* Re-pin agora to v2.0.7 (auto-patch finder + test fixes)
* Re-pin agora to v2.0.8 (security-clean shim per review)
* Re-pin agora to v2.0.8 (b78b42a — cold-load fix per review)
* Re-pin agora to v2.0.8 (5e90c30 — security-clean, no core override)
* Re-pin agora to v2.0.9 (fix #5: completion gate missed todo/triage/review)
* fix(agent): gate reasoning-only answer promotion by route
A clean-stop response with empty content and a reasoning payload was
promoted to the visible answer on every route. A field name is not
provenance: OpenRouter, DeepSeek and other adapters put private
chain-of-thought in `reasoning`, so users saw the model's private
thoughts as the reply (#111761, #114082). Exhausted recovery also echoed
a 500-char reasoning preview.
Promotion now requires a trusted route: an explicit
`answer_in_reasoning` runtime capability, a custom_providers per-model
opt-in, or the local Nemotron-3.5-Lightning parser route from #109205.
The decision lives in runtime_capabilities and is re-derived at startup,
fallback activation, primary restore and model switch. Exhausted
recovery returns the empty sentinel without a reasoning preview.
Rebuilt by hand on current main (keeps the Anthropic signed-thinking
skip and the _refund_api_call import added since the PR's base).
Salvaged from #112833 (a339569d77, 15a19b2fc0, 1d240e7b24).
* refactor(agent): drop defensive capability try/except and dead reasoning_only copy
get_custom_provider_model_capability already returns None when the
config cannot be read, so the try/except around it in
answer_in_reasoning_capability only hid real bugs.
_terminal_empty no longer returns site_copy("reasoning_only") with a
reasoning preview, and nothing else references that entry, so remove it
instead of leaving dead copy behind.
* test(agent): trim reasoning-promotion tests to two invariants
Keep two behavioural invariants for the route gate: an untrusted
OpenRouter/DeepSeek route retries past private reasoning to the visible
answer while the local Nemotron parser route still promotes in one call
(one parametrized test), and exhausted recovery never echoes private
reasoning. The capability-helper, constructor-plumbing and fallback-
restore unit tests duplicate what those end-to-end paths already cover.
* refactor(agent): derive reasoning-promotion trust live in agent/reasoning_promotion
The code-health ratchet blocks growth of agent_runtime_helpers.py
(3909), agent_init.py (2527), chat_completion_helpers.py (4122) and
finish_text_response (309 lines); the gate added 60+ lines to those
facades.
Move answer_in_reasoning_capability into a topical sibling and evaluate
it from the agent's live route at the promotion site instead of caching
the decision in runtime_capabilities. That removes the plumbing at
startup, primary snapshot/restore, switch_model and fallback activation:
a live check…
… not posted to the new channel _process_voice_input awaits WAV conversion and STT, then calls the gateway callback with only guild, user and transcript, and the callback routes by the guild's binding as it is at that moment. A `/voice join` from another text channel while the bot is already connected rewrites _voice_text_channels and _voice_sources, so an utterance captured for the old conversation became a turn in the new one, with its channel prompt, bound skills, transcript echo and replies. The bound text channel is captured when the utterance is accepted and compared after STT; if the binding moved, the stale work is dropped and logged. Rejoining from the same channel keeps the conversation and is dispatched as before. Nothing awaits between that check and the callback's own read of the binding. Reported by andrexibiza (F6) on NousResearch#128788. (cherry picked from commit 47d515e) Backport the required captured-binding check to the frozen adapter. Keep the existing callback, hallucination filter and resource cleanup. The two upstream regression cases are included unchanged.
What does this PR do?
_build_slash_event: DMchat_nameis the user's name; threadchat_namecomes from_format_thread_chat_name;auto_skillfrom_resolve_channel_skills(channel_id, parent_id)._dispatch_thread_sessiontakes the created thread (returned by_thread_created) and derives name, topic and parent through_format_thread_chat_name,_get_effective_topic(thread, is_thread=True)and_get_parent_channel_id, as a message posted in the thread does.user_nameis the guild member's display name, and the event carriesauto_skillfrom the bound text channel.b993d20b24): the channel prompt and skill lookups get the bound thread's parent, as a typed message in that thread does; chat name and topic come from the bound channel's current state through the same formatters (the/voice joincopy is the fallback when the channel is not cached); on a member-cache miss the/voice joininvoker keeps the name bound at join, any other speaker gets their id.Found while checking the gaps of #128722 (ours, Slack) across every adapter: Telegram, Mattermost, Teams, Matrix, WeCom, LINE, DingTalk, Google Chat, WhatsApp and Feishu each build turns through one path, and TUI / api_server build no
MessageEvent. Not changed: with the opt-indiscordtoolset, the context prompt's "Triggering message" line appears only when a turn has a message id, which a slash or voice turn does not (c0c76a4715renders it on purpose; fixing it is a renderer change). Expect a small textual conflict with #125620 in_build_slash_event.Related Issue
Fixes #128787
Type of Change
Changes Made
plugins/platforms/discord/adapter.py: slash and/threadevents built from the same helpers as message events.gateway/run_voice.py: voice turns carry the display name, the channel skill with parent inheritance, and the bound channel's current name and topic.plugins/platforms/discord/adapter.py:_guild_channel_labels(channel)returns(chat_name, chat_topic)as the message path computes them; voice uses it.tests/gateway/test_discord_turn_prompt_parity.py: two invariant tests: message → slash //thread→ message through the real pins (rows: dm, channel, thread, thread starter) keeps one context prompt, one channel prompt and the sameauto_skill; and the same for a voice turn (rows: channel, thread under a bound parent, channel renamed and re-topiced after the join, speaker not in the member cache).tests/gateway/test_voice_command.py: one call assertion updated for the parent argument.tests/gateway/test_discord_slash_commands.py: one call updated for_dispatch_thread_session's new argument.user-guide/messaging/discord.mdalready says channel prompts are injected on every turn in the matching channel or thread; channel skill bindings are not documented for Discord.How to Test
scripts/run_tests.sh tests/gateway/test_discord_turn_prompt_parity.pymain: 5 failed (fourassert 2 == 1distinct pinned prompts; the channel rowassert None == ['triage']).auto_skillfails the voice test.c2727ba500(the previously approved head) and green here. Sabotaged one at a time, each fails only its own row: no parent to the resolvers (thread row), no label refresh (renamed row), no joiner fallback or a fallback for every speaker (uncached row).b993d20b24, related files (test_discord_turn_prompt_parity,test_voice_command,test_discord_slash_commands,test_discord_channel_skills,test_discord_channel_prompts,test_discord_slash_auth,test_voice_mode_platform_isolation,test_discord_race_polish,test_discord_voice_mixer,test_platform_reconnect,test_discord_free_response,test_discord_thread_slash_expired_defer,test_thread_tracker_mark_off_loop): all pass. Ruff clean. Merges cleanly into origin/main233434b414.tests/gateway/test_discord*.pyand voice files: 434 passed, 6 skipped. Ruff clean. Python 3.14.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass. I ran only the related files listed above viascripts/run_tests.sh, not the full suite.Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/ACompared with #127944
#127944 (@matttmcfarland, opened 2026-09-29) fixes the voice-to-text prompt-cache miss from the other side and overlaps this PR in
gateway/run_voice.py::_voice_input_source. Read from both diffs at #127944 dce6efedfc and this head:/voice join, then the id)b993d20b24(credited:Co-authored-byMatt McFarland)/voice joinmessage's id as its own triggersource.message_id = None)auto_skill/voice joinauto_skill, DM and threadchat_nameas the message path builds them/threadstarter: name, topic and parent from the created threaddiscordtoolset)c0c76a4715)message_idpresence dropped from_ephemeral_change_key(the pin key)Merge check (
git merge-tree --write-tree, origin/mainbddd22be7c): each head merges cleanly into main on its own. Merged together,plugins/platforms/discord/adapter.pyauto-merges and one hunk conflicts:GatewayVoiceMixin._voice_input_sourceingateway/run_voice.py, where both compute the speaker's name (#127944 throughDiscordAdapter.voice_member_display_name, this PR inline from the guild member).gateway/session.py,gateway/run_agent_cache.pyand theauto_skill, slash and/threadchanges do not overlap.The two are complementary. #127944 (with @teknium1's
c28ab03fe6on it) owns the message-id and renderer half; this PR does not touch it. Its tests on this head: the two uncached-name rows pass; the other five fail on that renderer/message-id half or on itsvoice_member_display_namehelper, which this PR does not add. Whichever lands first, the_voice_input_sourceconflict resolves by keeping one name lookup (both apply the same joiner fallback) plus this PR's label refresh. Ported: the joiner fallback rule only.Compared with #129317
#129317 (@Sahilvishnaliya, opened 2026-09-30 13:19Z, after this PR) changes the same
_build_slash_event,_dispatch_thread_session,_thread_createdand_voice_input_sourcelines, plus unrelatedagent/jiter_preload.py,hermes_cli/venv_sync.pyandhermes_cli/web_routers/config_env.pychanges; it adds no tests. This PR's test file run against its code (f45457a364): 4 passed, 4 failed. The DM slash row fails because it names a DM bydisplay_namewhile a typed DM message usesauthor.name, so the pin still flips; the three voice rows fail (no parent bindings, join-time labels, uncached speaker by id). It covers no case this PR does not. Nothing ported.Related review follow-ups
From @andrexibiza's review of this PR, each filed and fixed on its own branch from
main(they are admission/routing defects that predate this PR, not prompt-input parity):/threadstarter are [Bug]: Discord slash commands from role-authorized users are rejected by the gateway (slash event never carries role_authorized) #118958 (fix(discord): carry the role verdict into slash command events #125620, fix(discord): preserve slash role authorization #118966); the voice half is [Bug]: Discord voice-channel speech from a role-authorized member never reaches the agent (voice source drops role_authorized) #130302 / fix(discord): a role-authorized member's speech, native slash commands and /thread starters reach the agent (#130302, #118958) #130303./voice joinrebind: [Bug]: Discord voice utterance in transcription is answered in another text channel after a /voice join rebind #130311 / fix(discord): an utterance transcribed across a /voice join rebind is not posted to the new channel (#130311) #130312.