Skip to content

fix(slack): unify slash and message prompt inputs - #129046

Open
JoaoMarcos44 wants to merge 5 commits into
NousResearch:mainfrom
JoaoMarcos44:fix/slack-slash-turn-inputs
Open

JoaoMarcos44 wants to merge 5 commits into
NousResearch:mainfrom
JoaoMarcos44:fix/slack-slash-turn-inputs

Conversation

@JoaoMarcos44

@JoaoMarcos44 JoaoMarcos44 commented Sep 30, 2026 •

Copy link
Copy Markdown

What does this PR do?

Fixes #128720 by keeping Slack native slash turns on the same prompt/session inputs and admission policy as ordinary Slack messages.

The original change shared the prompt-sensitive producer inputs:

  • chat_name
  • user_name
  • channel_prompt
  • auto_skill

The review pass then traced the downstream boundaries and closes the remaining integration gaps rather than treating producer parity as sufficient.

Review blockers resolved

  • Unauthorized DMs: a negative pre-enrichment auth result now skips Slack metadata I/O but still forwards a minimal DM event to gateway admission, so unauthorized_dm_behavior: pair|decline|ignore remains authoritative. Rejected channels still drop before enrichment.
  • Urgent and unblock controls: interrupt_then_dispatch commands such as /stop, /new and /reset, plus /approve and /deny, use IDs/cached labels and never await cosmetic name resolution before BasePlatformAdapter dispatch. Ambiguous G... controls recover DM/group class from the trusted active session key instead of waiting on Slack metadata.
  • Conversation policy: slash commands and Block Kit interactions share workspace-scoped conversation classification. D... is a 1:1 DM; G... is resolved with conversations.info.is_mpim; unresolved/underspecified G... classification fails closed.
  • Channel policy: ignored_channels and allowed_channels are enforced before enrichment/dispatch. The documented 1:1-DM channel-allowlist exemption remains; MPIMs are still subject to the channel allowlist.
  • Live interaction policy: approval/model-picker interaction authorization uses the resolved DM/group class and reads disable_dms at click time. Approval buttons first use the exact gateway-owned session key embedded in the button; this keeps legacy G... approval responsive during conversations.info failures without trusting the clicker's identity to reconstruct the key.
  • Goal kickoff: the first synthetic /goal <text> turn now preserves auto_skill; continuations intentionally do not reload it.
  • Prompt pins: committed message → slash → message coverage exercises the actual session-context and channel-prompt pin consumers and asserts A → A → A.

Regression coverage added

The review tests now cover:

  • denied DM × pair/decline/ignore through real GatewayRunner._hm_admit_event and a real temporary PairingStore, including pending-code / decline-stamp / ignored-owner diagnostic behavior and zero Slack name lookups;
  • denied channel × the same policy matrix, with zero dispatch and zero enrichment;
  • real active-session /stop cancellation through BasePlatformAdapter, with metadata APIs forbidden;
  • active legacy-G... /approve and MPIM /stop barrier cases with users.info / conversations.info forbidden;
  • approval-button resolution from the prompt's session key during metadata failure, including a different authorized clicker and a DM-vs-group negative control;
  • authorized excluded-channel slash calls, including lookup-free drop;
  • MPIM vs legacy private-channel classification for slash and the real approval-button consumer;
  • one-to-one DM control plus live disable_dms: false -> true with an outstanding approval;
  • fail-closed behavior for an ambiguous G... conversation;
  • /goal kickoff metadata plus the real auto-skill loader consumer and a direct-turn positive sibling;
  • same-session prompt/context pin stability across message → slash → message.

Non-duplication / related work

Commit structure

The original producer-parity commit is retained. Review fixes are kept as focused follow-ups; the latest two are:

  • 315b292 — fix(slack): keep active controls lookup-free;
  • f3ec460 — fix(slack): classify controls from session keys.

Current head: f3ec460def446b913b4995e40f8d335af955c78d (5 commits total in the PR).

Main/drift check

Upstream main is currently 9aae3c23e2c4f39f77d8d54b5e72ba4b90f4ba8f. This PR has a long-lived historical base (the head is 1242 commits behind current main), so branch-only validation is not treated as sufficient. GitHub still reports the PR mergeable; the current generated merge candidate is 6b35ce738d0c86567968afe4508f3e53d51cb44d against 46904a3b, and main has since advanced by one unrelated plugin-catalog lint commit.

Validation status

The earlier producer-only head was independently reported as 212 passed, 1 skipped for tests/gateway/test_slack.py, plus Ruff and git diff --check.

The current reviewed head has additional runtime and regression changes. A 10-pass post-fix audit found no additional blocker in provenance/scope, canonical session-key parsing, control call sites, authorization gates, active-session classification, ordinary-ingress fail-closed behavior, competitor overlap, current-main merge shape, review state, or CI state. Upstream CI, Docker and Nix workflows for this fork head are still held at action_required; there is no green execution receipt for f3ec460. Therefore I am not claiming the new head is CI-green.

Type of change

  • Bug fix
  • Tests
  • Feature
  • Documentation

Route Slack turn prompt inputs through one resolver so ordinary messages
and slash commands cannot drift in source names, channel prompt, or
new-session skill bindings.

Reject unauthorized slash callers before name enrichment, matching the
ordinary message path and avoiding pre-auth Slack API lookups.
@JoaoMarcos44
JoaoMarcos44 force-pushed the fix/slack-slash-turn-inputs branch from eeb7294 to 62ca64f Compare September 30, 2026 07:10
@alt-glitch alt-glitch added type/bug Something isn't working P0 Critical — data loss, security, crash loop comp/plugins Plugin system and bundled plugins platform/slack Slack app adapter sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) labels Sep 30, 2026
@JoaoMarcos44
JoaoMarcos44 marked this pull request as ready for review September 30, 2026 07:14

@ehz0ah ehz0ah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Request changes conclusion. GitHub does not allow this account to set the formal review state on this repository.

Reviewed exact head 62ca64f6fb4af66abf34da1346b71e3f132a1109.

Motivation

This PR fixes a real Slack inconsistency. Native slash turns currently omit the configured channel prompt, bot identity, source names, and channel-bound skill that ordinary message turns carry. That changes prompt inputs and can rebuild the cached agent twice around one slash turn.

Approach

The shared _resolve_turn_prompt_inputs boundary is a clear way to keep ordinary messages and slash commands aligned. It resolves the channel name, user name, channel prompt, and bound skills once for both producers. The slash path also tries to avoid Slack API lookups for rejected senders.

Concrete changes

_build_message_event now uses the shared resolver without changing its emitted fields. _handle_slash_command adds the same four inputs before it builds the event. The two tests cover positive prompt parity and the zero-lookup path for a rejected channel sender. Reversing the production diff makes both new tests fail. Removing only the early authorization branch makes the zero-lookup test fail.

Risk to main

The authorized path is correct. The full Slack test file passes with 212 tests passed and 1 skipped. Ruff and git diff --check also pass. Current upstream main is the PR base. Hosted CI has no jobs or commit statuses yet.

One blocking behavior gap remains. The early return also catches unauthorized DMs. That prevents the gateway from applying unauthorized_dm_behavior, so a DM sender cannot receive a pairing code or decline response. It also returns before the slash response context is stored. Slack has already shown the ephemeral Running /... acknowledgement at that point, so the user can be left with a response that never completes. The inline comment has the focused reproduction.

PR #128722 now handles this distinction at head 11d7f36c93e48f0a6a7c6c32cd1b10e123bff266: rejected channel senders stop before enrichment, while rejected DM senders reach the gateway without name lookups.

Overall assessment

Request changes. The core prompt-parity fix is focused and well tested, but the DM authorization path must preserve the gateway's pairing and decline behavior before this can merge.

Comment thread plugins/platforms/slack/adapter.py Outdated

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed head 62ca64f6fb4af66abf34da1346b71e3f132a1109 against base f42f579cf8bac4918ac9599bece71618afadd846.

The authorized free-form slash path now supplies the configured prompt, bot identity, source names and bound skill. My message → slash → message probe also keeps the session-context change key stable. The remaining problems are at admission, control dispatch and the downstream synthetic-turn boundary: two introduced regressions and one pre-existing gap left open by the skill-parity change.

[P1] Rejected DMs lose the gateway's pairing, decline and owner-diagnostic policies

The new early return treats a negative authorization verdict as a terminal drop for every conversation. For DMs, that verdict is the input to the gateway's unauthorized-sender policy, not the end of processing.

I connected the real adapter authorization callback to the real admission method and a temporary PairingStore, then sent an unapproved user's native /help in D123. On the base, pair creates one code and attempts one reply; decline persists its deduplication stamp and attempts one reply; ignore runs the owner warning/notifier path without replying to the stranger. On this head, all three stop before gateway admission. Both name-lookup counts are zero on both revisions. The return also precedes slash response-context storage, after Bolt has already acknowledged the command.

The earlier DM finding remains present; the composed repro additionally verifies the actual policy effects. Please preserve a minimal, lookup-free rejected-DM path into the gateway, including its response context. Continue dropping rejected channel senders. I reproduced the same early-drop pattern on ordinary DM messages at both revisions; that sibling is pre-existing, and copying its gate does not establish correct DM behavior. Cover both producers without allowing rejected senders into expensive enrichment.

[P2] Cold metadata lookup delays an authorized /stop

The new resolver await executes for control commands as well as prompt-consuming turns. Its users.info and conversations.info waits happen before the active-session command dispatch.

I started a task through native /hermes in a shared channel session, then sent native /stop from a second authorized operator whose name was uncached. An event barrier held that operator's metadata response. On the base, the real base-adapter cancellation path stopped the task without querying metadata. On this head, the stop command did not reach the handler and the task remained alive until I released the metadata response. Separate cold-user and cold-channel probes gave the same dependency.

Caching limits when this occurs; it does not remove the first-use case for another operator. Please give control commands a path using authenticated IDs and cached display data that does not await live name enrichment. Retain full prompt resolution for turns that consume it. The regression should hold metadata unresolved while asserting that the real running task is canceled, then verify normal prompt parity independently.

[P2] Goal kickoff still discards the newly supplied bound skill

The new auto_skill assignment fixes the direct free-form slash path, but _enqueue_goal_turn copies only the kickoff's channel prompt and omits its skill. Turn preparation then consumes the fresh-session flag and has no skill to load.

With a real temporary session store, a fresh reset and a real local triage skill containing a sentinel, I ran the actual /goal command handler and kickoff producer into the actual turn-preparation/skill-loader path. The incoming slash event carried ["triage"]; the kickoff carried None; the sentinel was absent; and the fresh-reset flag was consumed. A direct /hermes control through the same loader included the sentinel. The kickoff defect exists on the base too, so this is an incomplete repair of the skill-propagation class, not a newly introduced regression.

Please preserve auto_skill on goal kickoff copies and assert the loaded payload at the consuming first turn after /new or /reset. Keep ongoing goal continuations from reloading it. Producer-field equality alone cannot establish that the bound skill reaches the agent.

Verification

Canonical runner: 272 passed, 1 skipped across the Slack suite and six related files covering model picking, skills, bot authorization, mention handling, DM policy and prompt freezing. git diff --check passes.

The differential probes used Python 3.14.7, separate pinned worktrees/processes and temporary Hermes homes. Slack transport and the long-running agent were synthetic; cancellation, authorization/admission, pairing persistence, goal dispatch, session reset and the skill loader were production code. The goal probe stopped before turn-lease acquisition and model execution.

Hosted CI is not green: the CI run, Docker and Nix runs report action_required; the CI run has zero jobs. The PR has one commit, and the P1 admission regression still needs correction before merge.

Comment thread plugins/platforms/slack/adapter.py
Comment thread plugins/platforms/slack/adapter.py Outdated
Comment thread plugins/platforms/slack/adapter.py

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Follow-up review of unchanged head 62ca64f6fb4af66abf34da1346b71e3f132a1109 against base f42f579cf8bac4918ac9599bece71618afadd846.

Two additional P1 policy gaps survive the new early-admission boundary. Both are pre-existing at the base, rather than regressions introduced by this commit. They matter to the claimed ingress parity: matching prompt fields and rejecting a disallowed principal does not establish that the conversation is authorized.

The earlier review remains applicable at this exact head: rejected-DM policy handling, cold-metadata control dispatch, and goal-kickoff skill propagation are still unresolved. This follow-up adds the two admission gaps below without duplicating those findings.

[P1] MPIM commands and approval clicks use channel privileges instead of DM privileges

The new early gate consumes a conversation type derived only from a D prefix. An MPIM with a G id is therefore checked as a channel, although the ordinary producer checks it as a DM. The same prefix assumption controls interactive authorization.

With allow_from=["U_DM"], group_allow_from=["U_GROUP"], and an MPIM G123, I exercised both producers through the real profile-bound authorization callback and gateway admission. U_DM could send an ordinary message but could not run native /help; U_GROUP was denied the ordinary message but its native /help was admitted.

I then generated the actual Block Kit approval button for the DM owner's session and passed that button to the actual click handler. The channel-only user's click resolved the real blocking approval waiter to {"resolved": true, "choice": "once"}; the DM-authorized user's click was refused. This verifies an approval-boundary bypass, not merely a different boolean in a mocked policy.

The same classification lets native MPIM commands pass with disable_dms=true. There is also a direct-DM sibling: post a pending approval while DMs are enabled, then set disable_dms=true; approval clicks still resolve the waiter in both G123 and D123. No command was executed in these probes.

PR #128767, inspected at 24f07b981da79e77db7651566cb36cc7efcf8f71, addresses the classification/click area. It is not present in this head. The repair must reconcile message, slash and interaction entrypoints under one workspace-scoped conversation classification and apply the live DM-disable policy to clicks too. Preserve legacy private-channel behavior; a G prefix alone cannot prove that a conversation is an MPIM. An unresolved conversation kind must not select a broader grant when the DM and channel policies differ.

[P1] Native free-form slash turns bypass the configured channel allowlist

The new sender check does not enforce allowed_channels. The ordinary channel gate does, but the native slash producer never invokes that restriction. Gateway admission and outbound delivery enforce ignored_channels, which is a different policy.

I configured allowed_channels=["C_ALLOWED"] and globally authorized the caller. An ordinary @Hermes what broke? in C_BLOCKED was dropped. Native /hermes what broke? in that same channel reached the real admission sink, and the production send() path issued chat.postMessage(channel="C_BLOCKED", ...) for the synthetic reply. The MPIM allowlist case has the same bypass.

Positive and negative controls distinguish the policies: both producers admitted and replied in C_ALLOWED; both were dropped with no public reply in C_IGNORED; an authorized one-to-one DM retained the documented allowlist exemption. The blocked-channel scenario assumes the app is a member with permission to post there, as in the documented use case of a bot invited to many channels but restricted to a few.

Enforce the conversation allowlist independently of mention/wake rules before native turns reach the agent. Preserve the one-to-one DM exemption, retain MPIM restrictions, and keep ignored_channels as an unconditional exclusion. The regression must assert both no admission and no public delivery for excluded channels; testing only a denied user misses an authorized caller on a denied surface.

Verification and boundaries

The expanded canonical runner covered 37 files: 481 passed, 1 skipped, including all selected Slack files and multiplex interaction authorization. The independent policy probes fail their safety assertions at both pinned head and base under Python 3.14.7, in separate processes with temporary Hermes homes.

Authorization, gateway admission, session-key generation, Block Kit prompt generation, Slack delivery code, and the blocking approval waiter/resolver were production code. Slack transport, the agent reply, and the interaction delivery were synthetic. No live Slack request, model call, or terminal command was executed.

Hosted CI, Docker and Nix still report action_required for this commit. The passing local suites do not cover the newly reproduced policy bypasses. The first pass mentioned MPIM classification as adjacent work but did not establish its admission and approval effects; this follow-up closes that evidence gap.

Comment thread plugins/platforms/slack/adapter.py Outdated
Comment thread plugins/platforms/slack/adapter.py Outdated

@andrexibiza andrexibiza left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR #129046 —

Head: 62ca64f6fb4af66abf34da1346b71e3f132a1109 (unchanged since ehz0ah 7h ago, andrexibiza 3h/2h ago). Base: f42f579cf8bac4918ac9599bece71618afadd846.

Author: JoaoMarcos44. File changed: plugins/platforms/slack/adapter.py (+32, −7); test: tests/gateway/test_slack.py (+48).

What the PR does (verified from diff + live reviews)

  • Adds _resolve_turn_prompt_inputs shared resolver.
  • Replaces separate user_name/channel_name/channel_prompt/auto_skill lookups in _build_message_event with the shared call.
  • Applies the same resolver to _handle_slash_command and adds channel_name/user_name/channel_prompt/auto_skill to the slash-produced MessageEvent.
  • Adds _early_reject_unauthorized before the resolver in _handle_slash_command.
  • Adds positive parity test (test_slash_turn_matches_message_prompt_inputs) and zero-lookup negative test (test_unauthorized_slash_stops_before_name_enrichment).

Deeper findings NOT fully covered by existing reviews

  1. Partial-failure vulnerability in the shared resolver (not mentioned by ehz0ah or andrexibiza). The resolver awaits user_name then channel_name sequentially with no exception boundary. If the first succeeds and the second raises (Slack conversations.info failure), the exception bubbles out of _handle_slash_command after the Bolt Running /... acknowledgment has already been sent (line 6096–6104). There is no rollback of partial state, no fallback identity, and the MessageEvent is never built — the user sees a stuck ephemeral acknowledgment. The previous separate calls in _build_message_event had the same unbounded await behavior, so this is inherited, but centralizing it makes the single-point-of-failure more severe.

  2. Cross-turn sequence stability is untested (deeper than the parity test). The PR's positive test asserts slash.channel_prompt == message.channel_prompt at the same commit, but does not exercise the actual root-cause sequence: ordinary message → slash command → ordinary message in the same session with a cold cache. Without that, the claim that the shared resolver prevents drift is unverified against the real multi-turn session-state key change that #128720 reports. The regression should assert the session-context pin stays identical across the three-turn sequence.

  3. resolve_channel_skills inline import inside async method (line 4431). The original code had the import at module scope in _build_message_event; the PR moves it inside _resolve_turn_prompt_inputs. This introduces a per-call import overhead and hides the dependency from static analysis. More importantly, if gateway.platforms.base fails to import at call time (partial deployment, missing dependency), the exception occurs inside an async boundary that has already acknowledged the slash, making recovery harder than a top-level import failure.

  4. The goal-kickoff gap is deeper than the review states. Andrexibiza reproduced that _enqueue_goal_turn (gateway/slash_commands_goals.py 70–89) omits auto_skill. The PR fixes the producer side but does not touch the consumer side. The deeper issue: because the kickoff is built by a separate gateway module (not the adapter), the adapter's new shared resolver never reaches it. Even after this PR lands, a fresh /goal after /new or /reset will still drop the skill. A real fix requires either (a) propagating auto_skill through _enqueue_goal_turn, or (b) making the skill loader read from the adapter-level session state rather than the kickoff event. The PR does neither.

  5. No host-run evidence (confirmed). The PR checklist shows Hosted CI executed and green is unchecked. The body explicitly says upstream Actions are at action_required with zero jobs ("Upstream Actions on the previous fork head were held at action_required with zero jobs"). The rewritten head (62ca64f) also has no executed hosted jobs. I verify this independently: no run or check-run is listed for 62ca64f.

Confirmed unaddressed blocking/regression issues from existing reviews (not fixed at this head)

  • [P1 blocking — ehz0ah (r4142262250) + andrexibiza (5366190108)]: The _early_reject_unauthorized early return (adapter lines 6078–6081) treats a negative auth verdict as terminal for all conversations, including DMs. Rejected DMs must still reach the gateway's unauthorized_dm_behavior (pairing/decline/ignore) without name enrichment. The return also skips _stash_slash_context, so Bolt's ephemeral acknowledgment stays stuck. Reproduced with real authorization callbacks (PairingStore) at base f42f579cf8 and this head.
  • [P2 — andrexibiza (5366190108), inline r4144580175]: Cold-user/cold-channel metadata await delays authorized /stop before active-session cancellation. Reproduced with asyncio.Event-held users_info response; base cancels without await, this head blocks.
  • [P2 pre-existing/incomplete — andrexibiza (5366190108), inline r4144580188]: Goal kickoff (_enqueue_goal_turn) still drops auto_skill; pre-existing at base; not resolved by this PR.
  • [P1 follow-up — andrexibiza (5366525792)]: MPIM authorization/approval uses D prefix assumption; G id MPIMs get channel-level privileges. Related #128767 (24f07b98) addresses it; not present at this head.
  • [P1 follow-up — andrexibiza (5366525792)]: allowed_channels allowlist enforced by ordinary message gate but bypassed by native slash turns (C_BLOCKED admission + chat.postMessage delivery verified).

Verdict

The core parity fix is focused, well-tested, and correctly avoids cherry-picking from #128722. However, the blocking DM-admission regression (P1), the cold-metadata control dispatch regression (P2), and the deeper untested stability/accountability gaps above mean this head should not merge until the admission boundary is split (channel = drop; DM = minimal event to gateway without enrichment), the control-command path skips live metadata awaits, and the goal-kickoff consumer propagates the bound skill. Hosted CI remains at action_required with zero jobs — the passing local suites (272 passed, 1 skipped per andrexibiza; 481 passed, 1 skipped in expanded follow-up) cover suite hygiene but not the reproduced policy bypasses or the admission boundary.

@andrexibiza andrexibiza left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR #129046 — complete repair reconciliation and subsystem review

The normal Slack slash-turn prompt-parity repair works. The repair boundary is not yet complete: this head introduces rejected-DM policy loss and metadata-dependent control dispatch, while inherited conversation-policy and skill-lifecycle gaps remain. A shared resolver establishes field consistency; it does not establish admission parity, control responsiveness, or delivery of those fields to the consuming agent turn.

This is a new, additive reconciliation of the existing record. Existing reviews and inline discussions remain intact. Each claim below is kept visible with its source, present evidence, scope, and closure condition; repeated descriptions are not counted as independent defects.

1. Exact target and coverage

Property Verified target / evidence
PR head 62ca64f6fb4af66abf34da1346b71e3f132a1109
Base and verified merge base f42f579cf8bac4918ac9599bece71618afadd846
Change One commit; plugins/platforms/slack/adapter.py +32/−7 and tests/gateway/test_slack.py +48/−0
Review census before this review 4 submitted reviews, 6 inline comments in 6 threads, 0 issue-style PR comments; every thread was unresolved and not outdated
Review records reconciled ehz0ah: 5363312714; 5366190108; 5366525792; 5368798866
Linked requirement Issue #128720, including its expanded instruction-state lifecycle analysis
Source acquisition Exact GitHub commit archives after local partial-clone object acquisition timed out. Both changed head files were checked against GitHub's Git blob IDs. These are archived source trees, not Git worktree test runs.
Evidence vocabulary Executed: this reconciliation's fresh local run. Source-confirmed: the pinned implementation establishes the mechanism. Historical execution: the linked earlier review reports execution; not relabeled as a fresh run. Not established: evidence does not prove that stronger conclusion.

The six existing inline comments do not mean six distinct defects: ehz0ah's DM finding and the later composed DM finding describe the same newly introduced slash-path failure. The later review adds policy effects and the pre-existing ordinary-message sibling.

2. Full repair ledger

Priorities describe impact, not GitHub review permissions. P1 denotes a substantial policy/availability failure; P2 denotes a meaningful behavior or regression-coverage gap; P3 denotes structural maintainability debt. Introduced and inherited findings have different ownership even when both remain relevant to class closure.

ID / priority Repair line item and origin Exact-head evidence / present status Required behavior Closure evidence and ownership
R01 · P1 Native rejected-DM handling — introduced. I0, I1, all four review bodies. Slash ingress returns on a negative early authorization result before event construction, response-context storage, and gateway admission. Fresh adapter probe: base forwards one minimal event and stores response context; head forwards/stores neither; both make zero name lookups. Gateway policy owns pairing, one-time decline and ignore-owner diagnostics. Earlier V1 exercised those policy effects. Open. A rejected DM must remain excluded from agent execution and expensive enrichment, but still reach the gateway's configured unauthorized-DM policy with original identity, workspace and command response context. A rejected channel sender may be dropped. Denied DM × pair/decline/ignore through the real policy sink, including expected persisted policy state/diagnostic and reply routing; zero name/media enrichment; no agent start. Denied channel controls remain dropped. Direct regression repair in this PR.
R02 · P1 Ordinary-message rejected-DM sibling — inherited. Explicitly identified in V1/I1. Ordinary ingress already returned at the same early-negative gate on the base. This is not a new effect of moving name resolution into a helper. Pinned source confirms it still precedes the gateway DM-policy owner. Historical V1 reports the same result at both pins. Open sibling, not a second introduced regression. The ordinary producer needs the same minimal policy-only route as R01, without granting a denied sender normal enrichment or agent processing. Ordinary-DM policy matrix and denied-channel controls alongside R01. May be a separately tracked class repair, but copying this old behavior does not demonstrate that the new slash gate preserves the DM contract.
R03 · P2 Urgent control dispatch waits for cosmetic metadata — introduced. I2, V1/V3; overlaps the later timeout concern. Fresh event-barrier probes through the real adapter active-session/cancellation methods: base cancels the waiting task before any metadata response; head leaves /stop undispatched and the task alive until the held metadata response is released. Repeated separately for cold-user and cold-channel metadata. The new await precedes the control dispatcher. Open. Authorized interrupt/unblock controls must reach their owning dispatcher without waiting for display-name APIs. Prompt-consuming commands must retain complete prompt inputs. This is a dispatch-order repair, not permission to remove authorization or to replace all normal-turn names with unstable fallbacks. Hold metadata unresolved while asserting actual task cancellation/control handling; separately assert normal free-form and turn-producing-command parity. Include the registry's interrupt/unblock semantics rather than hardcoding every slash as a non-generating command. Direct regression repair in this PR.
R04 · P2 Goal kickoff drops the channel-bound skill — inherited consumer gap. I3, V1 and V3's “deeper goal” item. Fresh real kickoff-constructor probe at both pins: incoming auto_skill=["triage"], kickoff auto_skill=None, channel prompt preserved. The kickoff constructor omits the field; first-turn preparation only loads a supplied binding when the session qualifies as new. V1 separately reports the actual missing skill payload after reset. Open; the repeated description is the same repair item. Preserve authoritative skill metadata on kickoff event reconstruction; keep ongoing continuation semantics separate so instructions are not repeatedly injected. Fresh/reset /goal must reach the real skill loader and first prepared agent payload with the bound content. Direct question and subsequent continuation controls distinguish initial loading from repeated loading. Gateway-side owner, not solved by changing the Slack helper alone.
R05 · P1 MPIM native-command classification and session identity — inherited. I4, V2/V3. Messages recognize channel_type=mpim as DM-style S03; native commands still derive kind from the ID prefix S01. Source confirms different classification inputs and therefore different policy/session selection. V2 records prior composed admission results. Open at this head. Use workspace-scoped conversation classification consistently before selecting DM/channel policy and session identity. Preserve legacy private-channel behavior; unknown classification must not silently become a broader authorization grant. Cross-entrypoint parity for known group DMs, one-to-one DMs, legacy private channels, and unresolved classification. Related owner: #128767; its code is absent here and composition has not been certified.
R06 · P1 MPIM interactive authorization — inherited sibling of R05, separate interactive entrypoint. I4/V2/V3. Interactive authorization independently derives kind from the same prefix assumption. Interaction admission uses that decision. V2 reports an actual approval-state effect; this reconciliation rechecks source, not an executable bypass. Open at this head. Message, slash and interaction paths need one consistent conversation-kind contract while preserving explicit conversation grants and profile/workspace ownership. Cosmetic metadata and principal authorization are not substitutes for classification. Defensive interaction-policy coverage across conversation kinds and existing chat-scoped grants. No claim of fresh live Slack or command-execution testing is made. Coordinate with #128767 rather than duplicate its work.
R07 · P1 Live disable_dms policy on outstanding interactions — inherited, independently actionable within the same policy class. I4/V2. Message and slash producers check DM-disable policy, but the interaction entry chain does not perform that check. V2 reports the outstanding-prompt enable→disable case for both one-to-one and group DMs. Open; accurate classification alone does not close this row. Apply the current DM-disable decision at interaction handling, including prompts created before the policy changed. Preserve channel interaction behavior. Policy-transition tests with an already outstanding interaction, plus ordinary enabled-DM/channel controls. Separate acceptance row for the #128767 policy repair.
R08 · P1 Conversation allowlist on native turn-producing commands — inherited. I5, V2/V3. Ordinary conversation gate checks allowed_channels; the complete native handler does not. Gateway admission checks ignored_channels, a different policy. The documented contract exempts one-to-one DMs but not MPIMs. V2 contains historical delivery-boundary evidence. Open; a principal deny test does not test destination authorization. Enforce conversation allowlisting independently of mention/wake behavior before native enrichment and agent dispatch; retain the one-to-one DM exemption, MPIM restriction and unconditional ignored-channel exclusion. Tests must assert no admission, no enrichment and no public delivery for excluded conversations, plus allowed-conversation and DM controls. Slack ingress class repair; neither a mention-rule transplant nor ignored_channels alone closes it.
R09 · P2 Persistent regression coverage for the actual prompt-pin sequence and consuming boundaries — evidence/coverage gap, not a demonstrated remaining ordinary-sequence defect. V3 item 2. The added positive test executes slash→message and checks producer fields; its fixture replaces handle_message. It does not execute the message→slash→message pin lifecycle. Fresh probes in this reconciliation do: base changes A→B→A, head stays A→A→A. Behavior demonstrated working in the measured case; repository regression still narrower than the claim. Preserve a regression through the real pin owners and a single session, with positive configured prompt/identity/skill assertions. Add each R01–R08 contract at its owning boundary rather than equating mock-event parity with subsystem completion. Committed sequence test plus focused boundary tests. Separate same-session flat-routing cases from default thread-isolated conversations; include DM/turn-producing-command cases without asserting that every slash starts a model turn.
R10 · P3 Two already oversized files continue to grow — inherited structural debt increased by this diff. Exact archive counts: adapter 6,925→6,950 lines; Slack test file 5,727→5,775 lines. Repository code-shape guidance calls for splitting files approaching 2,000 lines. Not a runtime defect, and not created from zero by this PR. Route focused behavior-neutral extraction separately from behavior repair; preserve callable/patch seams and contributor lineage. New regression coverage should have a focused test home rather than continue accumulating everything in the large Slack test file. Extraction-only diff with behavioral equivalence evidence, followed by focused fixes/tests. This structural row must not be confused with proof of either runtime regression.

Five runtime defect classes are retained: rejected-DM lifecycle, control dispatch, skill propagation, conversation-kind/interaction policy, and conversation allowlisting. They are expanded into R01–R08 so their distinct producer, consumer and interaction repair obligations cannot disappear inside one label. Only R01 and R03 are newly introduced regressions in this commit. R09 is test coverage; R10 is structure.

Execution evidence remains a separate line item

ID State What remains
E01 — hosted execution Exact-head Actions records exist. CI, Docker and Nix each report action_required, and their jobs endpoints each return zero jobs. The additional collected label-rerun records also have zero jobs. The head's PR-scoped check rollup is null. Executed exact-head hosted results are still absent. This is neither CI green nor a failed product test. Workflow-approval state is distinct from a code repair and is not counted as another runtime defect.

3. Claim-by-claim evidentiary reconciliation

Every item in the latest review remains represented here. This table separates the factual mechanism from stronger descriptions that the present evidence does not establish. It does not erase or alter the earlier record.

Claim / source What the pinned source and fresh evidence establish Repair or verification implication
C01 — “Partial-failure vulnerability,” ordinary conversations.info failure escapes the shared resolver — V3 item 1 Both leaf resolvers wrap the awaited API operation in except Exception and cache an ID fallback S12. Fresh channel-only, user-only and both-failing transport fixtures all forward an event; on the head they retain the configured prompt and binding. Successful-user/failed-channel returns the display user plus raw channel ID. This is contrary evidence to the stated uncaught ordinary-API-exception mechanism. It does not prove immunity to every possible exception/cancellation. Failure handling and pending-I/O liveness are different properties. The demonstrated outstanding wait belongs to R03. A distinct partial-failure repair requires a concrete reachable exception outside the existing fallback boundary, not merely the absence of another outer try.
C02 — cross-turn stability is “unverified” — V3 item 2 V1 already reported a message→slash→message key probe. This reconciliation freshly executes the real key and both pin owners: head stable, base unstable in the flat same-session fixture. The added repository test still does not cover that lifecycle. R09 remains a coverage obligation. “No committed sequence regression” and “the sequence has never been exercised” are not interchangeable. No provider billing/cache-miss measurement is claimed.
C03 — the import was moved from module scope and now causes per-call/deployment failure — V3 item 3 The actual base-to-head patch removes from gateway.platforms.base import resolve_channel_skills from inside _build_message_event and places it inside _resolve_turn_prompt_inputs. It was already a function-local import. The adapter already imports its base class from that module. No fresh runtime failure or measured performance regression is established for this move. Dependency-placement concerns remain a verification item. A separate repair contract needs a reachable deployment failure or measured overhead; the diff does not show the asserted module-scope→local transition.
C04 — the goal gap is “deeper” because the shared resolver never reaches kickoff — V3 item 4 This is the same producer→reconstruction→consumer omission already in I3/V1, not a second independent goal defect. Fresh reconstruction confirms it at base and head. R04 owns the repair and the consuming-payload regression. The absence of a Slack call inside the gateway constructor is not itself wrong; dropping authoritative event metadata is the issue.
C05 — “no run or check-run is listed” — V3 item 5 The exact-head check-rollup/check-run surfaces are empty, but the Actions run surface contains eight records. Each collected run has zero jobs and action_required. CI run, Docker run, Nix run. E01: no executed hosted jobs. Absence of check runs does not imply absence of workflow-run records. Historical local counts in V0/V1/V2 remain historical; they are not this reconciliation's fresh counts.
C06 — absence of local wait_for proves an indefinite wait; Bolt ACK is stuck — later chat-only timeout concern, not a fifth published review There is no additional resolver-level deadline, but the pinned SDK has a 30-second default request timeout; the normal no-custom-session path constructs aiohttp.ClientTimeout(total=timeout). The adapter constructs the client without overriding that default S13. The Bolt listener awaits ack(...) before calling the slash handler. Neither an unlimited real-network wait nor an unsent ACK is established by the held-fixture experiment. R03 remains demonstrated even with finite HTTP timeouts. The running acknowledgement can be left without a completing application response; that is distinct from missing the initial ACK. No fixed total end-to-end latency is claimed: sequential requests, retries, custom sessions and cancellation have separate ownership.
C07 — a negative authorization gate proves full ingress parity — PR rationale and positive/negative test pair The gate is principal authorization. It does not select the right conversation kind, implement allowed destinations, preserve denied-DM policy delivery, or exempt urgent control dispatch from metadata waits. R01/R02/R05–R08 remain separately enumerable. Shared prompt inputs are a useful boundary but not an admission policy abstraction.
C08 — comparison with the old #128722 head settles the present alternative — PR body The PR body explicitly compares an older 5527ae... pin. Current #128722 is d125ad8a80fa39f36aa82011c25d340940a66963; its published changes include minimal denied-DM forwarding and interrupt/unblock lookup exemption. This does not alter #129046's unchanged bytes. Alternative-head comparisons must name their pins. No conclusion here relies on the competing PR still having its old lookup-order behavior, and no composition/whole-class closure is inferred from its existence.
C09 — exact provider cache misses / all slash turns / all-platform closure — linked issue discussion Fresh evidence measures prompt inputs and rendered context bytes under a single-session fixture, not provider cache accounting. Native commands that do not start an agent turn and default thread-isolated messages are different cases. The linked issue also enumerates downstream lifecycle owners outside Slack's two constructors. State the affected topology and measured property. Field parity closes the normal Slack producer omission, not every command, queued consumer, lifecycle transition, or platform.
C10 — “no copied code / correctly avoids cherry-picking” establishes provenance — PR body and V3 verdict This pass verifies the net patch, exact parent/base, commit count and disclosed alternative. It does not reconstruct every author's local development history or infer intent from similar code. No new authorship accusation or originality certification is made. Credit remains with ehz0ah for the earlier DM finding, the existing review authors for their reported evidence, and jonpol01 for the alternative and related repair work.

4. Subsystem review: ownership and current boundaries

Boundary / owner Actual responsibility Measured or source-confirmed state
Slack Bolt listener Acknowledge native command receipt, then enter the adapter ACK precedes name enrichment. Receipt acknowledgement is not command completion. S14
_handle_message ingress Conversation classification, DM-disable, early principal gate, destination/wake gate, then message assembly MPIM classification differs from slash/click paths; early denied-DM return is inherited. S03, S09
_handle_slash_command Native command normalization; conversation/source construction; response context; dispatch New early return and enrichment await both precede response-context storage and the gateway. S01
_resolve_turn_prompt_inputs Display names, configured channel prompt + bot identity, channel skill binding Normal producer parity works. Transport errors have leaf-level fallbacks. It is not the owner of admission or synthetic-event semantics. S12, S15
BasePlatformAdapter.handle_message and active-session dispatcher Session identity, busy guard, control bypass, cancellation, queued work Correctly receives/cancels only after the Slack producer dispatches the event. A downstream bypass cannot bypass an upstream metadata await. S04
GatewayRunner._hm_admit_event Receiving-adapter exclusion, authorization, rejected-DM policy, final admission Must receive a denied DM to produce its configured policy outcome. ignored_channels is not allowed_channels. S02
Prompt-pin owners in run_agent_cache.py Stable context bytes and channel inputs under session identity; persistence/recovery Fresh same-session normal sequence is stable at head. Names are inputs to the context key. S16
_enqueue_goal_turn Reconstruct kickoff/continuation turn and send through FIFO Copies the kickoff prompt but not its bound skill. S05
_hmwa_prepare_turn Decide first-turn work, load skill, acquire turn lease, read history, prepare agent input Presence on the original slash event is insufficient if a later constructor drops the binding or lifecycle state consumes initialization too early. S06
Interactive handlers Acknowledge interaction and apply the appropriate current policy before action handling Independent entry chain must participate in classification and live DM-disable policy. S07, S08

Diagram A — current head: prompt parity shares a helper; admission does not

%%{init: {"theme":"base","themeVariables":{"darkMode":true,"background":"#0d1117","primaryColor":"#16324f","primaryTextColor":"#e6edf3","primaryBorderColor":"#58a6ff","lineColor":"#8b949e","secondaryColor":"#33224f","tertiaryColor":"#1f2937","fontSize":"15px"},"flowchart":{"htmlLabels":true,"curve":"basis","nodeSpacing":30,"rankSpacing":70}}}%%
flowchart TD
    M["Ordinary Slack message"] --> MI["Message ingress: kind, DM-disable, principal gate"]
    MI ---> CG["Conversation allowlist and wake gate"]
    CG --> MB["_build_message_event"]
    N["Native slash command"] --> ACK["Bolt ACK: running"]
    ACK --> SI["Slash ingress: prefix kind, DM-disable, principal gate"]
    SI -->|"negative verdict: returns,<br/>including DMs"| DROP["No event or slash response context: R01"]
    SI -->|"accepted or indeterminate"| RES["Shared prompt-input resolver"]
    MB --> RES
    SI -.->|"does not traverse:<br/>R08"| CG
    MI -.->|"negative DM also returns:<br/>R02"| DROP
    click M "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L4629-L4664" "Pinned implementation"
    click MI "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L4629-L4664" "Pinned implementation"
    click CG "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L4182-L4221" "Pinned implementation"
    click MB "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L4709-L4720" "Pinned implementation"
    click N "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L1676-L1680" "Pinned implementation"
    click ACK "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L1676-L1680" "Pinned implementation"
    click SI "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L6062-L6104" "Pinned implementation"
    click DROP "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L6062-L6104" "Pinned implementation"
    click RES "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L4421-L4437" "Pinned implementation"
    classDef blue fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px;
    class M,N blue;
    classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px;
    class MI,CG,SI purple;
    classDef gray fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px;
    class ACK gray;
    classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px;
    class MB,RES amber;
    classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px;
    class DROP red;
    subgraph LEGEND["Legend — roles; evidence stays explicit"]
      direction LR
      L_blue["Entry / observations"]:::blue
      L_purple["Ownership / policy"]:::purple
      L_gray["Context / transient"]:::gray
      L_amber["Transitions / pending"]:::amber
      L_red["Failures"]:::red
    end
Loading
%%{init: {"theme":"base","themeVariables":{"darkMode":true,"background":"#0d1117","primaryColor":"#16324f","primaryTextColor":"#e6edf3","primaryBorderColor":"#58a6ff","lineColor":"#8b949e","secondaryColor":"#33224f","tertiaryColor":"#1f2937","fontSize":"15px"},"flowchart":{"htmlLabels":true,"curve":"basis","nodeSpacing":30,"rankSpacing":70}}}%%
flowchart TD
    RES["Shared prompt-input resolver"] --> EV["MessageEvent: names, prompt, auto_skill"]
    EV --> BA["Base adapter: session and control dispatch"]
    BA --> GA["Gateway admission and DM policy"]
    GA --> AG["Agent-turn preparation"]
    CLICK["Interactive action"] --> IA["Separate interactive policy: R06 and R07"]
    click RES "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L4421-L4437" "Pinned implementation"
    click EV "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L6062-L6104" "Pinned implementation"
    click BA "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/platforms/base.py#L3964-L4054" "Pinned implementation"
    click GA "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/run_inbound.py#L205-L299" "Pinned implementation"
    click AG "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/run_turn.py#L2045-L2100" "Pinned implementation"
    click CLICK "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L5537-L5554" "Pinned implementation"
    click IA "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L5479-L5525" "Pinned implementation"
    classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px;
    class RES,AG amber;
    classDef gray fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px;
    class EV gray;
    classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px;
    class BA,GA,IA purple;
    classDef blue fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px;
    class CLICK blue;
    subgraph LEGEND["Legend — roles; evidence stays explicit"]
      direction LR
      L_amber["Transitions / pending"]:::amber
      L_gray["Context / transient"]:::gray
      L_purple["Ownership / policy"]:::purple
      L_blue["Entry / observations"]:::blue
    end
Loading

The dotted allowlist edge denotes a missing traversal, not a production call. R05 concerns the incompatible conversation classification before the slash and interactive policy decisions. This graph does not imply that every ordinary message passes every gate or that every command reaches agent execution.

Diagram B — why a working downstream /stop bypass still waits

%%{init: {"theme":"base","themeVariables":{"darkMode":true,"background":"#0d1117","primaryColor":"#16324f","primaryTextColor":"#e6edf3","primaryBorderColor":"#58a6ff","lineColor":"#8b949e","secondaryColor":"#33224f","tertiaryColor":"#1f2937","fontSize":"15px"},"flowchart":{"htmlLabels":true,"curve":"basis","nodeSpacing":30,"rankSpacing":40}}}%%
flowchart TD
    B["Bolt listener: send running acknowledgement"] --> S["Slash producer: handle authorized stop"]
    S --> N["Name resolver / Slack SDK: await cold metadata"]
    N --> WAIT["While metadata is unresolved,<br/>stop has not reached dispatcher"]
    WAIT --> RETURN["Name response or handled error fallback"]
    RETURN --> DISPATCH["Slash producer: dispatch command event"]
    DISPATCH --> REPLY["Base adapter: handle control reply"]
    REPLY --> W["Cancel existing session task"]
    REPAIR["Repair: control dispatch must not<br/>depend on display-name I/O"] -.-> DISPATCH

    click B "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L1676-L1680" "Pinned implementation"
    click S "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L6062-L6104" "Pinned implementation"
    click N "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L4421-L4437" "Pinned implementation"
    click WAIT "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L6062-L6104" "Pinned implementation"
    click RETURN "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L3300-L3357" "Pinned implementation"
    click DISPATCH "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L6062-L6104" "Pinned implementation"
    click REPLY "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/platforms/base.py#L3964-L4054" "Pinned implementation"
    click W "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/platforms/base.py#L3964-L4054" "Pinned implementation"
    click REPAIR "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/platforms/base.py#L3964-L4054" "Pinned implementation"
    classDef blue fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px;
    class S,RETURN blue;
    classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px;
    class B,N,DISPATCH,REPLY,REPAIR amber;
    classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px;
    class WAIT red;
    classDef green fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px;
    class W green;
    subgraph LEGEND["Legend — roles; evidence stays explicit"]
      direction LR
      L_blue["Entry / observations"]:::blue
      L_amber["Transitions / pending"]:::amber
      L_red["Failures"]:::red
      L_green["Successful outcomes"]:::green
    end
Loading

The barrier fixture proves ordering, not infinite real-network latency. Both the metadata-error fallback and the SDK timeout can work while R03 still exists.

Diagram C — field parity is not consuming-turn parity

%%{init: {"theme":"base","themeVariables":{"darkMode":true,"background":"#0d1117","primaryColor":"#16324f","primaryTextColor":"#e6edf3","primaryBorderColor":"#58a6ff","lineColor":"#8b949e","secondaryColor":"#33224f","tertiaryColor":"#1f2937","fontSize":"15px"},"flowchart":{"htmlLabels":true,"curve":"basis","nodeSpacing":30,"rankSpacing":40}}}%%
flowchart TD
    E["Slash event: prompt and auto_skill"] --> Q{"Command path"}
    Q -->|"Direct question"| P["Turn preparation"]
    Q -->|"Goal kickoff"| K["New MessageEvent: prompt copied, auto_skill omitted"]
    K --> F["Adapter FIFO"]
    F --> P
    P --> N{"New-session obligation?"}
    N -->|"Yes, binding present"| L["Load bound skill"]
    N -->|"Binding absent"| SK["Skip skill: R04"]
    L --> T["Lease and transcript preparation"]
    SK --> T
    T --> OUT["Prepared agent input"]

    click E "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/plugins/platforms/slack/adapter.py#L6062-L6104" "Pinned implementation"
    click Q "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/slash_commands_goals.py#L70-L90" "Pinned implementation"
    click P "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/run_turn.py#L2045-L2100" "Pinned implementation"
    click K "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/slash_commands_goals.py#L70-L90" "Pinned implementation"
    click F "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/slash_commands_goals.py#L70-L90" "Pinned implementation"
    click N "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/run_turn.py#L2045-L2100" "Pinned implementation"
    click L "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/run_turn.py#L2045-L2100" "Pinned implementation"
    click SK "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/run_turn.py#L2045-L2100" "Pinned implementation"
    click T "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/run_turn.py#L2045-L2100" "Pinned implementation"
    click OUT "https://github.com/NousResearch/hermes-agent/blob/62ca64f6fb4af66abf34da1346b71e3f132a1109/gateway/run_turn.py#L2045-L2100" "Pinned implementation"
    classDef blue fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px;
    class E blue;
    classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px;
    class Q,N purple;
    classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px;
    class P,F,T amber;
    classDef gray fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px;
    class OUT gray;
    classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px;
    class K,SK red;
    classDef green fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px;
    class L green;
    subgraph LEGEND["Legend — roles; evidence stays explicit"]
      direction LR
      L_blue["Entry / observations"]:::blue
      L_purple["Ownership / policy"]:::purple
      L_amber["Transitions / pending"]:::amber
      L_gray["Context / transient"]:::gray
      L_red["Failures"]:::red
      L_green["Successful outcomes"]:::green
    end
Loading

Continuation prompt preservation and kickoff instruction inheritance are different contracts. A continuation fix cannot stand in for R04.

5. Related work and boundary of the repair

Artifact / inspected head Relationship What it does not prove for this head
#128722 · d125ad8a80fa39f36aa82011c25d340940a66963 Alternative direct Slack producer implementation. Current published diff includes denied-DM forwarding and control-command lookup exemption; the original PR comparison is historical. The heads are alternatives in the same handler, not a stack whose fixes can be assumed present here. No local composed or alternative-head green result is claimed in this reconciliation.
#128767 · 24f07b981da79e77db7651566cb36cc7efcf8f71 Dedicated MPIM classification/interactive-policy work; natural coordination owner for R05–R07. An open sibling does not close this PR's rows. Its unresolved-classification fallback and explicit conversation-grant semantics need assessment at the chosen composed head.
#126167 · ee67e13c132469b6b4085b4dbe68c57fef599c76 Synthetic-turn prompt-pin preservation, a separate event-identity problem. It does not establish that kickoff skills survive reconstruction or that every queued human turn refreshes both prompt components together.
#128788 · c2727ba500f22249dd729aba6365af3bb3c5e970 Discord sibling producer parity. Not a Slack admission repair and not proof of gateway-wide lifecycle closure.

Linked issue lifecycle ledger — separate owners, no hidden scope reduction

The expanded issue comment reports the following additional lifecycle work. These rows are source-revalidated inherited boundaries, not additional regressions introduced by this PR, and not fresh runtime reproductions of the comment's experiments. The cited gateway/Telegram files are byte-identical at this PR's head and base. R04 already accounts for the issue's goal-kickoff omission.

ID / recorded priority Existing class item Current pinned mechanism Owner and acceptance condition
L01 · P1 Synchronous bound-skill initialization occupies the gateway event loop Async turn preparation calls the synchronous loader directly S06; that loader constructs the skill payload in its calling thread L01S. This is distinct from one Slack command awaiting a network response. Gateway skill preparation: off-loop work with profile context preserved, then generation-owned application. A deliberately delayed loader must not prevent an unrelated control/heartbeat callback from progressing. No production latency is measured here.
L02 · P1 Queued turns can combine the next channel prompt with the preceding context prompt The follow-up path records/persists pending_event.channel_prompt, then recursively passes turn_ctx.context_prompt with the new source L02S. The independent context-pin owner is not invoked in that preparation segment. Queued-turn preparation: derive, execute and persist one coherent context/channel snapshot under the conversation fence; distinguish authoritative human changes from pin-preserving synthetic inputs. Existing configuration-removal behavior must remain representable.
L03 · P2 Failed preparation spends first-turn skill eligibility _hmwa_open_session clears is_fresh_reset L03S before skill load and transcript read; the later TranscriptReadError path returns S06. Session/bootstrap lifecycle: retain the instruction-initialization obligation until recoverable state contains the instructions, with failed-preparation retry and successfully persisted bootstrap controls.
L04 · P2 Partial prompt-pin state after eviction skips durable recovery Agent eviction clears the context pin but leaves the channel pin L04S; rehydration returns when either pin remains S16. A subsequent synthetic render may therefore use non-authoritative metadata instead of the prior context. Prompt-pin/cache lifecycle: separate agent/render cache invalidation from conversation instruction authority. Verify ordinary eviction, cold restart, and real privacy/capability changes; blindly restoring old bytes across a policy change is not the contract.
L05 · P2 Non-generating commands allocate an empty session before the first actual question /retry allocates before checking for a prior user message L05A; goal-manager lookup also allocates L05B. Reused entries touch updated_at L05C, while first-turn detection includes timestamp equality L03S. Session/bootstrap lifecycle: /retry with no prior question and /goal status must not consume the bound-instruction obligation. Verify the first actual prepared question and a nonduplicating subsequent turn. This has a different trigger from L03.
L06 · P2 Telegram topic recovery changes the destination after enrichment Telegram builds topic name, skill and prompt from its initial thread L06A; recovery subsequently replaces the thread ID L06B. Telegram plus gateway routing: establish the effective topic before enrichment, or refresh the dependent name/prompt/skill together. Outside this Slack change; no Telegram runtime was exercised in this reconciliation.

These rows remain visible because constructor parity cannot close the linked issue's broader instruction-state lifecycle. They do not authorize attributing every downstream defect to this adapter commit.

6. Fresh execution receipts and limits

Canonical repository test runner

Executed separately on exact head and base archives, through the repository's scripts/run_tests.sh, native Windows CPython 3.14.3, isolated state/home/temp directories and one worker. No product source was edited.

Selected files Base Head
tests/gateway/test_slack.py 211 passed 213 passed
tests/gateway/test_slack_channel_skills.py 4 passed 4 passed
tests/gateway/test_prompt_tail_freeze.py 9 passed 9 passed
Focused total 224 passed, 0 failed 226 passed, 0 failed

Command at each pin: scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_slack_channel_skills.py tests/gateway/test_prompt_tail_freeze.py with an explicit isolated HERMES_PYTHON, HERMES_TEST_WORKERS=1, and HERMES_TEST_FILE_RETRIES=0.

Expanded verification also completed: all 36 tests/gateway/test_slack*.py files plus test_multiplex_interactive_auth.py and test_prompt_tail_freeze.py — 38 files: base 503 passed / 0 failed; head 505 passed / 0 failed, with 100% of selected files completed and process exit 0 at both pins. The expanded runs include the focused files above; the counts are not additive. The shell-equivalent selection is scripts/run_tests.sh tests/gateway/test_slack*.py tests/gateway/test_multiplex_interactive_auth.py tests/gateway/test_prompt_tail_freeze.py; the saved invocation receipts enumerate every selected path explicitly. The real Slack dependencies were installed, so historical runs with dependency-dependent skips need not have identical counts.

One identical no-network probe across both pins

Case Base observation Head observation Interpretation
Cold-start ordinary→slash→ordinary, real context key and both pin owners A→B→A A→A→A Normal same-session parity repaired.
User lookup succeeds, channel lookup raises an ordinary exception Slash makes no metadata request and lacks prompt inputs Event delivered with display user + raw channel ID, configured prompt and skill Leaf fallback works; no observed uncaught partial failure.
User fails / both fail / no-failure control Slash still has no enriched inputs Each delivers with the appropriate fallback or names; prompt and skill retained Failure and healthy controls distinguish resilience from an inert handler.
Explicitly denied native DM One captured event and one stored response context; zero lookups Neither event nor context; zero lookups New adapter-side policy-delivery regression. The gateway policy effects are source-confirmed and historically exercised, not rerun by this capture test.
Explicitly denied channel control Minimal event reaches downstream capture Dropped before capture Deliberate negative-channel behavior, distinct from DM policy.
Authorized stop, cold user metadata held Task canceled before metadata response Task alive until response is released R03 reproduced through actual active-session/cancellation methods.
Authorized stop, cold channel metadata held Task canceled before metadata response Task alive until response is released Same regression through the other resolver.
Real goal kickoff constructor with a bound input Skill omitted; prompt retained Skill omitted; prompt retained R04 inherited and still open.

The probe emits 10 machine-readable case rows per pin; the error scenarios are grouped for readability above. A successful probe means the stated observation was verified, including remaining defects. It does not mean those defects are repaired.

Runtime boundary: real imported pinned adapter, base dispatcher/cancellation, pin owners/renderer, event types and goal constructor; substituted Slack transport, runner reply, waiting task body, in-memory pin storage and captured FIFO. No live Slack, provider, actual terminal command, or approval-state bypass was executed. Security-policy rows R05–R08 are source revalidation plus attributed historical review evidence, not a fresh exploitation claim. The same-session fixture binds one key explicitly; it does not establish every session-routing topology.

Environment boundary: Slack SDK 3.44.1, Slack Bolt 1.30.0, aiohttp 3.14.3, pytest 9.1.1, pytest-asyncio 1.4.0. The full development-group install could not resolve pinned ty==0.0.82; the focused environment was installed from the repository's runtime/slack requirements plus pytest tools. It is not a full locked development environment. Because these are source archives, the runner's Git-based tracked-file precompile enumeration reported “not a git repository”; selected test subprocesses nonetheless completed successfully. Hosted CI remains E01.

Byte receipts

Subject Verified value
Head adapter Git blob 90d6529f11cfdd6c4351c57e4d0c017bb514e4b4
Head adapter SHA-256 34d9ec5004ac48900237fac04929ba945e90f03821786130f98354136a02a57d
Head Slack test Git blob 05bdf2ee211271f3b30bfd7fbb81ab78acd79488
Head Slack test SHA-256 c4ec9cd55f8ba2a3342079225f82341bcfa21576ba41745ec3c08d3c7a4e9350
Base adapter SHA-256 1825b0d48f7da1933c3c7eebe5d72438cf76067dc447832de6de47018ffbf1af

7. Required completion boundary

  1. Preserve the working parity repair. Names, configured channel prompt, bot identity and first-turn binding must not disappear from genuine prompt-consuming slash turns.
  2. Repair the introduced ordering/lifecycle regressions. Denied DMs retain the policy-only response path; urgent authorized controls do not await cosmetic metadata.
  3. Keep inherited repairs explicitly owned. Ordinary denied-DM handling, goal kickoff, MPIM classification/interactions and conversation allowlisting stay open until their named contracts are verified at the chosen head or a verified composition. A sibling link is not completion evidence.
  4. Prove consumers, not just constructors. Repository regressions must follow the same-session pin sequence, the real cancellation boundary, the denied-DM policy sink and the first skill-consuming turn.
  5. Keep evidence states separate. Passing selected local tests, passing behavior probes, source-confirmed policy gaps, historical execution and approval-held hosted workflows answer different questions.

That is the reconciliation: the central normal-turn repair is real; the two introduced regressions are real; the inherited subsystem obligations remain enumerable; and none of those facts is settled by counting green mocks, repeating a claim, or pointing at another open PR.

Comment thread plugins/platforms/slack/adapter.py
Comment thread plugins/platforms/slack/adapter.py Outdated
Comment thread tests/gateway/test_slack.py
@JoaoMarcos44
JoaoMarcos44 force-pushed the fix/slack-slash-turn-inputs branch from 2d958d0 to d350e87 Compare September 30, 2026 22:03

@ehz0ah ehz0ah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed exact head d350e87497b441461f2d42243da85aff39597bed against current main at 6b2fe92af6a95ea6a5caa309e05c5643cdd79de.

The two repair commits address the earlier denied-DM policy path, goal kickoff skill propagation, MPIM and interaction classification, channel allowlisting, and same-session prompt pin coverage.

One control-dispatch gap remains. /approve and /deny must bypass an active session because the agent may be waiting on them. This head still resolves Slack names before dispatching those commands. The inline comment includes a focused reproduction. /stop has the same dependency in an uncached G... group DM because conversation classification still happens first.

The three changed test files passed with 314 tests and 1 skipped. A clean merge with current main passed the same suite. Two focused barrier probes reproduced the remaining waits on both the PR head and the current-main composition. Ruff and git diff --check passed. GitHub currently reports no check runs or commit statuses. The CI, Docker, and Nix workflows are held at action_required with zero jobs.

This remaining control-path issue is blocking.

Comment thread plugins/platforms/slack/adapter.py Outdated

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Context. Slack native slash turns (/hermes <q>, /queue …) build a MessageEvent without chat_name/user_name/channel_prompt/auto_skill, so the gateway's session-context and channel-prompt pins flip on the slash turn and flip back on the next message (two agent rebuilds / prompt-cache misses per slash turn, #128720). Reviewed head d350e874 merged onto current origin/main.

What it does. Commit 1 adds the four inputs via a shared _resolve_turn_prompt_inputs. Commits 2–3 then grow into an admission rewrite: _classify_conversation (conversations.info for every G… id, fail-closed), allowed_channels/ignored_channels on slash + Block Kit clicks, _authorize_interaction_user, live disable_dms on clicks, denied-DM forwarding, a 1-line auto_skill on goal kickoff — +690/-32 across 5 files, ~10 new tests.

Premise vs main. Confirmed: adapter.py::_handle_slash_command on main builds the event bare (L6061-6067) while _build_message_event resolves names/prompt/skill. Real bug.

Overlap with #128722. Same issue, submitted ~6h earlier, 25 LOC in the slash handler + one 83-line test file (2 parametrized invariants), already carries the denied-DM forwarding and skips lookups for interrupt_then_dispatch and /approve//deny. #128722 is the better base: minimal, no new admission policy, and it passes every control probe below where this head does not.

Confirmed defects (probe: users.info held open / conversations.info raising, merged trees for main, #128722, this head):

  • /approve and /deny wait for name lookups before dispatch — adapter.py:6196 only exempts is_interrupt_then_dispatch; /approve//deny have busy_policy="dispatch" and hit await _resolve_turn_prompt_inputs at :6203. Head: dispatched_before_release=0; main and #128722: 1. ehz0ah's claim CONFIRMED. Fix: also exempt cmd.name in ("approve","deny") (what #128722 does).
  • Legit-flow regression: in a legacy G… private channel, /stop and /approve are dropped outright when conversations.info raises (e.g. missing groups:read) or omits is_mpim — _classify_conversation returns None (:3364, :3374) and _handle_slash_command returns at :6170-6174. Main and #128722 dispatch all three cases. Same fail-closed at _authorize_interaction_user :5599-5600 refuses approval-button clicks in such channels — an agent waiting on approval can no longer be unblocked. That is the symptom class #4926 fixed. Fail-closed on a lookup failure for a control command is the wrong direction; classification must not sit in front of controls.

Guardrail check (denied DM). gateway/run_inbound.py untouched by both PRs; _hm_admit_event still receives the denied DM with chat_type=dm and runs unauthorized_dm_behavior (your pair/decline/ignore test passes merged: 315 passed). Rejected channel senders still drop. Not weakened — but identical to #128722's if rejected and not is_dm: return.

Scope that should split out (own PRs, not the P0 fix): MPIM classification + interaction auth (#128767 territory), allowed_channels/ignored_channels on slash and clicks, _cached_turn_prompt_inputs, create_handoff_thread getattr tweak. The 1-line _enqueue_goal_turn(auto_skill=…) is a real pre-existing gap worth landing as a credited delta on #128722.

Looks good. Parity test, message→slash→message pin test (A→A→A), denied-DM policy matrix; all 315 tests in the three touched files pass under the merged shape. Ruff/diff-check clean.

Merge decision is the maintainer's. My read: close as superseded by #128722 and salvage the goal-kickoff one-liner onto it with credit.

@teknium1 teknium1 added the needs-decision Awaiting maintainer decision before any implementation label Oct 1, 2026
jonpol01 pushed a commit to jonpol01/hermes-agent that referenced this pull request Oct 1, 2026
_enqueue_goal_turn rebuilt the kickoff turn with the message id and
channel prompt but not auto_skill, so the turn the agent actually runs
for `/goal <text>` carried no channel skill binding. A resume
continuation still carries none.

Ported from NousResearch#129046 (5ee2a10) onto NousResearch#128722 as a credited delta, per
the maintainer review of NousResearch#129046. The code line is unchanged; the
docstring line was adjusted to name bound skills when porting.

Refs NousResearch#130316

Copy link
Copy Markdown
Author

Review follow-up on head f3ec460.

The two confirmed control-path defects from the latest reviews are now addressed:

  1. /approve and /deny join the lookup-free control path, so they do not wait on users.info before active-session dispatch.
  2. /stop, /approve and approval buttons in ambiguous legacy G... surfaces no longer depend on a successful conversations.info lookup when Hermes already has trusted session state. Slash controls derive the class from active session keys; approval buttons use their embedded gateway-owned session key. Parsing is delegated to the canonical gateway.run_busy._same_chat_key_slots helper, including workspace/profile/key-tail handling.

The follow-up also covers the alternate-operator case: the clicker's user id is not used to reconstruct the session key, so per-user group sessions do not reintroduce the metadata dependency. DM/group authorization, disable_dms, ignored/allowed-channel policy and ordinary unresolved-G... fail-closed behavior remain in place.

New barriers cover active legacy-private G... /approve, active MPIM /stop, approval clicks with conversations.info forbidden, and a DM-vs-group authorization negative control.

I then re-ran a 10-pass source/merge audit against the final head. No additional blocker surfaced. The inline blocking thread has been answered and resolved.

Validation caveat: GitHub's CI, Docker and Nix workflows for this fork head are still action_required, with no green status receipt. I am not claiming CI-green until maintainers approve/run those workflows.

@ehz0ah ehz0ah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Request changes conclusion. GitHub does not allow this account to set the formal review state on this repository.

Reviewed exact head f3ec460def446b913b4995e40f8d335af955c78d.

Motivation

Issue #128720 is real. Slack slash turns must carry the same prompt inputs as ordinary messages without weakening admission policy or delaying controls.

Approach

The shared prompt resolver fixes normal turn parity. The follow-up commits also preserve denied-DM policy, apply channel and interaction gates, propagate the first goal skill, and recover ambiguous G... conversation type from trusted session keys for active controls and approval buttons.

Concrete changes

The latest two commits close the prior /approve, /deny, active MPIM /stop, and approval-button metadata blockers. The session-key parser uses the canonical gateway helper, and ordinary unresolved G... ingress remains fail-closed.

Risk and validation

Two introduced dispatch-order problems remain. Concurrent native /queue commands can reach the real busy-session FIFO in reverse order when the first metadata lookup is slow. The emergency /pause command also still waits for users.info before it reaches the gateway. The inline comments contain focused reproductions and closure tests.

The maintained Slack and goal-dispatch suites pass at the PR head with 317 passed and 1 skipped. The same suites pass on a clean merge with current upstream/main, also with 317 passed and 1 skipped. git diff --check passes, and main has not changed the five PR files since the merge base. The three focused review probes fail on the two behaviors above.

Hosted CI has no check runs or commit statuses. CI, Docker, and Nix are held at action_required, so there is no green hosted receipt.

Overall assessment

Request changes. The earlier blockers are repaired, but command order and the emergency pause path must remain independent of cosmetic Slack metadata before this can merge.

self._cached_turn_prompt_inputs(channel_id, team_id, user_id)
)
else:
channel_name, user_name, channel_prompt, auto_skill = (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P1] Preserve slash arrival order before handing work to the gateway

This await can let a later callback overtake an earlier callback for the same session. I held the first users.info call open, then sent two native /queue commands. The adapter handed them over as second, first. The same probe through BasePlatformAdapter and GatewayBusySessionMixin stored the pending item and FIFO overflow in that reversed order. Current main performs no metadata await at this point, so this is introduced by the new enrichment path.

Please serialize the handoff per session before the first fallible await, with cancellation-safe release. Add a regression that holds the first lookup, starts a second /queue, and verifies the real busy-session FIFO remains first, second. A cancelled waiter must not strand later commands.

from hermes_cli.commands import is_interrupt_then_dispatch, resolve_command
command_def = resolve_command(command_name) if command_name else None
urgent_control = bool(command_name and is_interrupt_then_dispatch(command_name))
lookup_free_control = urgent_control or bool(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Keep emergency /pause independent of Slack metadata

lookup_free_control covers interrupt commands plus /approve and /deny, but /pause has busy_policy="dispatch" and is excluded. I held users.info open for an authorized native /pause maintenance call. The gateway handoff count stayed at zero until the lookup was released. Current main performs no name lookup on this path.

/pause is registered as the global emergency pause command, so a metadata outage must not delay it. Include pause in the lookup-free control policy and add a barrier test that asserts gateway dispatch while both name APIs remain unresolved.

@andrexibiza andrexibiza left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CR · RCA · subsystem analysis — f3ec460def446b913b4995e40f8d335af955c78d

The original prompt-input omission is repaired, and the latest active-session/session-key fixes are real. This head still has three introduced control regressions: idle G-conversation controls are dropped, emergency pause waits for display-name metadata, and concurrent queue commands can reverse order. A fourth finding documents the inherited first-goal initialization gap and the test that bypasses its eligibility gate. The correct goal field copy should be retained, but it does not establish fresh-session payload delivery.

This is a COMMENT review. Scope: all five changed files and their admission, classification, control, FIFO, goal-manager, session-store, prompt-pin and skill-loading consumers. All five commits are authored by JoaoMarcos44. Original parent: f42f579; current upstream runtime frozen at eaecc99. Historical conclusions below are reconciled against the prior complete ledger, the later d350 review, maintainer review, and author's latest closure claim. At final capture there are 12 threads: 10 resolved, six outdated, and two open pause/FIFO threads from the concurrent current-head review. Resolved metadata does not establish runtime closure.

Current findings and measured effects

ID Priority / origin Trigger → actual effect Repair boundary
F1 P2, introduced by the active-key control revision Complete an ordinary message in a private G channel or MPIM, then issue an authorized /new or /reset. No active key/kind cache remains, so the slash returns before dispatch despite healthy metadata. All five lookup-free controls were dropped in the fixture. Matching active-session /approve is a positive control. Preserve trusted workspace-scoped kind across idle lifecycle; retain the busy path without metadata and fail closed for truly unknown kinds.
F2 P2, remaining introduced R03 regression Hold cold users.info for an authorized /pause in a known C channel. Real ESTOP remains absent until the cosmetic response is released. Emergency controls must reach their actual effect after policy admission, before display-name enrichment.
F3 P2, introduced with prompt enrichment Hold the first /queue first on metadata; let /queue second finish enrichment. The real Base adapter and runner FIFO store second, first. Serialize same-session enrichment through completed handoff, with cancellation/failure recovery and control bypass.
F4 P2, inherited bootstrap gap; incomplete closure evidence First /goal build it copies auto_skill=["triage"], but manager allocation plus turn lookup makes created_at != updated_at; the real preparation gate skips the loader. Direct first question and goal after a real reset load the payload. Preserve initialization obligation until a consuming turn succeeds; test the real lifecycle, rather than calling the loader below its gate. Existing owner: #130316.

New inline comments cover F1 and F4 with exact changed-RIGHT anchors, consumer links, contrasting cases and acceptance conditions. F2 and F3 are retained with independent consumer receipts and referenced through the existing pause thread and FIFO thread. The latter records P1; this review rates the demonstrated sequencing regression P2. F1–F3 reproduce on current head and in the current-main composition. F4 is also reproduced there, but is explicitly not attributed to the new one-line field copy.

Fresh execution receipts

The same independent, no-network barrier/consumer probe was run in fresh processes against the original parent, original PR commit, d350, current head, pinned main and a current-main composition. It uses real Slack producer methods, real Base session dispatch, actual FIFO storage, persisted pairing, actual ESTOP writes in a temporary home, real goal-manager/SessionStore lifecycle and _hmwa_prepare_turn. Skill payload construction is a sentinel leaf; preparation stops at the turn-lease boundary before model execution. No product source was edited.

Source Idle private-G /new delivered ESTOP before metadata release Actual two-command FIFO First-goal payload Goal-after-reset payload Denied native-DM pairing
Parent f42f579 Yes Yes first, second No No Yes
Original PR 62ca64f Yes No second, first No No No
d350e874 Yes No second, first No Yes Yes
Head f3ec460 No No second, first No Yes Yes
Main eaecc99 Yes Yes first, second No No Yes
Current-main composition No No second, first No Yes Yes
#128722 eb546b6 adapter on the baseline consumer Yes Yes first, second No Yes Yes

For F1, each row first completed an ordinary message through the real Base adapter; all current-head private/MPIM controls then dropped, while a matching active private-channel approval dispatched. For F2/F3, release is controlled by an asyncio event, not a timed network assumption. For F4, the first direct question is the positive sibling and a real SessionStore.reset_session supplies the reset marker. Denied ordinary DMs create zero pairing rows/replies on parent, head and main; denied native slashes create one row/reply on head.

Sources were materialized from GitHub blob identities into hash-verified snapshots, not represented as a completed Git checkout. The composition uses the exact generated-merge adapter/goals from 6b35ce7 with the verified eaecc99 runtime (2,216 selected source/config/support files). The 18 upstream commits after that merge's main parent did not change Slack or gateway files. This is focused composition evidence, not an entire merged-repository test run. Transport clients return fixtures; absent Slack SDK packages use the repository's import stubs. No live Slack delivery, provider execution, billing/cache-hit measurement, Telegram execution or production latency was tested.

Probe SHA-256: 4d24203f21c8b3f3c85c4963d023cd2e56d0b1023a9a9d970658ffdcf92ee14d. Head result SHA-256: 0b99ddbf8cd9301ca57d4efa0f9129976161a1ba0bacfb79c311652f76e98e4d.

Canonical command: HERMES_PYTHON=<isolated Python 3.14.7> bash scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_slack_approval_buttons.py tests/hermes_cli/test_goal_dispatch.py.

  • Head: 247 Slack tests passed, one skipped; 10 goal tests passed, 60 failed during test state setup at wait_on(os.getpid()) because the process-liveness probe reported the running PID dead.
  • The same 60 setup failures occur with the unchanged parent runtime. A separate child-process diagnostic also reports its live PID missing from psutil; a single-case instrumented run passes. These are a local process-visibility limitation, not evidence that this diff broke goal command parity. No green whole goal-suite claim is made.
  • Running the head Slack tests on d350 gives 244 passed, three failed, one skipped: active G approval, active MPIM stop, and approval-button key recovery. All three pass on head. This provides fresh negative/positive sensitivity for the latest repairs.
  • The concurrent reviewer reports 317 passed / one skipped on head and a clean current-main merge, plus failing pause/FIFO probes. Those green goal-suite counts are attributed to that review, not to this environment.
  • The newly added loader test passes on head while the independent first-goal lifecycle probe still misses the payload. These measure different boundaries.

RCA and subsystem boundaries

Slack's shared prompt-input resolver repairs names, channel prompt and skill-binding parity for normal turns. Conversation-kind authority remains separate from display-name caches. The active-session classifier only sees busy ownership, so its absence cannot stand in for the conversation's idle identity. Conversely, normal turn enrichment now awaits before gateway handoff, affecting both control timing and ordering. Goal reconstruction preserves the field, but the instruction consumer is gated by session lifecycle state. These are four distinct boundaries.

Each diagram has an embedded semantic legend and pinned node links; evidence confidence remains explicit in the findings and controls. The original three Mermaid sources were rendered successfully with Mermaid CLI in a dark theme and visually inspected before this link/palette update.

flowchart TD
  M["Ordinary message"] --> MC["Payload conversation<br/>type"]
  S["Native slash"] --> SC["API, cache or active key"]
  A["Approval click"] --> AC["Embedded session key"]
  MC --> P["Distributed policy gates"]
  SC --> P
  AC --> P
  P --> D{Denied DM?}
  D -->|native slash| R["Pair, decline or ignore"]
  D -->|ordinary message| X["Early drop: policy<br/>skipped"]
  P -->|allowed message or slash| B["Base session dispatch"]
  P -->|allowed click| AR["Approval resolver"]
  B --> C["Control consumer"]
  B --> T["Turn preparation and<br/>pins"]
  subgraph QA_LEGEND["Legend — roles; evidence stays explicit"]
    Q_blue["Entry / observations"]
    Q_gray["Context / transient"]
    Q_purple["Ownership / policy"]
    Q_green["Successful outcomes"]
    Q_red["Failures"]
    Q_amber["Transitions / pending"]
  end
  classDef blue fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px
  class M,S,A,Q_blue blue
  classDef gray fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px
  class MC,AC,Q_gray gray
  classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
  class SC,P,D,AR,Q_purple purple
  classDef green fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
  class R,Q_green green
  classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
  class X,Q_red red
  classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
  class B,C,T,Q_amber amber
  click M "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L4767-L4790" "Pinned source"
  click MC "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L4767-L4790" "Pinned source"
  click S "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L6232-L6309" "Pinned source"
  click SC "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L6241-L6264" "Pinned source"
  click A "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L5662-L5728" "Pinned source"
  click AC "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L3343-L3385" "Pinned source"
  click P "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_inbound.py#L268-L307" "Pinned source"
  click D "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_inbound.py#L275-L294" "Pinned source"
  click R "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_inbound.py#L281-L295" "Pinned source"
  click X "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L4780-L4784" "Pinned source"
  click B "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/platforms/base.py#L3990-L4054" "Pinned source"
  click AR "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L5809-L5832" "Pinned source"
  click C "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/platforms/base.py#L4028-L4054" "Pinned source"
  click T "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_turn.py#L2045-L2085" "Pinned source"
Loading

This is a contract map: policy checks are distributed across producer and gateway methods, rather than one shared policy implementation. Sources: ordinary admission, native admission, interactive admission, Base dispatch, gateway denied-DM policy. The ordinary denied-DM branch still drops before that owner; the native branch now reaches it.

flowchart TD
  S["Native slash"] --> K{Lookup-free set?}
  K -->|stop, new, reset, approve, deny| C["Active key or kind cache"]
  C -->|known class| B["Control reaches consumer"]
  C -->|idle, uncached G| U["Conversation kind<br/>unknown"]
  U --> D["Return before dispatch"]
  K -->|pause or queue| N["Await display-name<br/>metadata"]
  N -->|pause response held| E["ESTOP stays disengaged"]
  N -->|second queue overtakes first| F["FIFO stores second,<br/>first"]
  N -->|response released| T["Gateway handoff"]
  subgraph QA_LEGEND["Legend — roles; evidence stays explicit"]
    Q_blue["Entry / observations"]
    Q_purple["Ownership / policy"]
    Q_gray["Context / transient"]
    Q_green["Successful outcomes"]
    Q_red["Failures"]
    Q_amber["Transitions / pending"]
  end
  classDef blue fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px
  class S,Q_blue blue
  classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
  class K,Q_purple purple
  classDef gray fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px
  class C,U,Q_gray gray
  classDef green fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
  class B,Q_green green
  classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
  class D,E,F,Q_red red
  classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
  class N,T,Q_amber amber
  click S "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L6232-L6309" "Pinned source"
  click K "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L6241-L6258" "Pinned source"
  click C "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L3386-L3404" "Pinned source"
  click B "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/platforms/base.py#L4028-L4054" "Pinned source"
  click U "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L6253-L6264" "Pinned source"
  click D "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L6258-L6264" "Pinned source"
  click N "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L6279-L6307" "Pinned source"
  click E "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_busy.py#L992-L1007" "Pinned source"
  click F "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_busy.py#L105-L115" "Pinned source"
  click T "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/plugins/platforms/slack/adapter.py#L6290-L6307" "Pinned source"
Loading

Sources: control selection and early return, active-kind recovery, await and handoff, real ESTOP consumer, FIFO owner. ESTOP pauses new work; it does not kill an existing worker.

flowchart TD
  G["First goal command"] --> M["Manager creates session"]
  M --> Q["Kickoff copies skill<br/>binding"]
  Q --> R["Turn lookup touches<br/>timestamp"]
  R --> F{New-session gate?}
  F -->|created differs from updated| X["Loader skipped: payload<br/>absent"]
  D["First direct question"] --> N["One route creation"]
  N --> F
  Z["Real session reset"] --> F
  F -->|equal timestamps or reset flag| L["Skill payload loaded"]
  L --> P["Prepared model-bound<br/>text"]
  subgraph QA_LEGEND["Legend — roles; evidence stays explicit"]
    Q_blue["Entry / observations"]
    Q_teal["Durable state"]
    Q_amber["Transitions / pending"]
    Q_purple["Ownership / policy"]
    Q_red["Failures"]
    Q_green["Successful outcomes"]
    Q_gray["Context / transient"]
  end
  classDef blue fill:#172554,stroke:#60a5fa,color:#eff6ff,stroke-width:2px
  class G,D,Q_blue blue
  classDef teal fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa,stroke-width:2px
  class M,N,Q_teal teal
  classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
  class Q,R,Z,Q_amber amber
  classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
  class F,Q_purple purple
  classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
  class X,Q_red red
  classDef green fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
  class L,Q_green green
  classDef gray fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px
  class P,Q_gray gray
  click G "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/slash_commands_goals.py#L70-L90" "Pinned source"
  click M "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_goals.py#L54-L91" "Pinned source"
  click Q "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/slash_commands_goals.py#L70-L90" "Pinned source"
  click R "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/session.py#L1014-L1022" "Pinned source"
  click F "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_turn.py#L512-L543" "Pinned source"
  click X "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_turn.py#L2078-L2085" "Pinned source"
  click D "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_turn.py#L2045-L2052" "Pinned source"
  click N "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/session.py#L1035-L1061" "Pinned source"
  click Z "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/session.py#L1123-L1146" "Pinned source"
  click L "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_turn.py#L2078-L2085" "Pinned source"
  click P "https://github.com/NousResearch/hermes-agent/blob/f3ec460def446b913b4995e40f8d335af955c78d/gateway/run_turn.py#L2045-L2085" "Pinned source"
Loading

Sources: manager allocation, correct kickoff copy, activity touch, freshness gate, gated skill consumer, test bypassing the gate. Timestamp activity and completed instruction initialization need separate state.

Prior repair ledger — no silent closure

Prior ID Current status Evidence / remaining ownership
R01 · P1 native rejected DM Repaired in the tested path. Independent original-commit→head pairing changes 0→1; committed pair/decline/ignore policy tests pass and retain response context without name enrichment. Tests.
R02 · P1 ordinary rejected-DM sibling Open, inherited. Fresh parent/head/main producer→policy probe yields zero pending pairings/replies; native head yields one. The early return remains before the policy owner. Requires the minimal policy-only ordinary-DM route, plus the full policy matrix.
R03 · P2 urgent-control metadata wait Partially repaired. Active C stop passes on d350/head; active G approval, MPIM stop and button-key recovery fail on d350 and pass on head. F2 retains the pause gap; F1 adds an idle-state availability regression.
R04 · P2 goal skill omission Field copy and reset consumption repaired; first-session lifecycle still open. Fresh reset probe loads the payload on d350/head and not parent. F4 / L05 covers the remaining initialization gate, distinct from the original omission.
R05 · P1 MPIM native classification/session kind Known-kind classification repaired; availability qualification remains. Known MPIM/private classification tests pass; unresolved G fails closed. F1 demonstrates the cold/idle control case is still unavailable. Tests.
R06 · P1 MPIM interaction policy Known-kind principal checks repaired; full grant parity not certified. Enabled/denied principals and exact-key recovery pass. The test uses an injected principal predicate; it does not independently certify explicit chat-scoped grants under the full runner auth chain. Preserve that acceptance row. Interaction owner, chat-grant owner.
R07 · P1 live DM-disable interaction policy Repaired in tested known-DM cases. Live policy is checked at handling and the committed transition/approval tests pass. Classification/recovery still supplies the policy context. Owner.
R08 · P1 native conversation allowlist Gate repaired in the tested matrix. Allowed/denied destination tests pass before prompt enrichment, with one-to-one DM exemption and MPIM restriction. Tests.
R09 · P2 actual pin-sequence regression Core sequence coverage repaired. Committed real pin-owner message→slash→message test passes, with same-session inputs and A→A→A assertions. This does not measure provider cache charges or certify all thread topologies. Test.
R10 · P3 code shape Open structural debt, not a runtime blocker. Adapter is 7,155 lines; Slack test file 6,205. Use focused behavior-neutral extraction and focused test homes, preserving seams and contributor credit. Repository guidance.
E01 hosted execution Open. Every commit lacks green check/status/Actions receipts; details below.

Earlier claim adjudication retained

Claim IDs Current conclusion
C01 partial-failure escape Ordinary API exceptions remain caught by leaf resolvers; do not revive an uncaught-exception finding without a reachable counterexample. Pending I/O is the separately demonstrated F2/F3 boundary.
C02 stability never exercised Incorrect as a blanket claim; historical probes and the now-committed pin-owner sequence exist. Scope remains a flat same-session fixture.
C03 moved module import causes deployment failure The import was already function-local. No such measured failure is established.
C04 deeper goal omission Original reconstruction omission is R04; the now-reproduced first-turn eligibility issue is the distinct L05/F4 lifecycle owner.
C05 no workflow runs Empty check-runs do not mean empty Actions runs. Runs exist and require action.
C06 indefinite wait / missing Bolt ACK The held fixture proves dispatch ordering, not infinite real HTTP latency. Bolt ACK precedes the handler.
C07 principal deny proves all ingress parity Insufficient; R02 and F1–F4 remain independently observable.
C08 stale alternative comparison Current alternative is eb546b6, not the old compared pin. The table is an adapter comparison with bounded consumer scope.
C09 all-platform / exact provider-cache closure Not measured. Constructor and pin-byte parity do not close all downstream lifecycle owners.
C10 provenance / cherry-pick assertions No inference about private development history. Verified credited goal delta 71c7ce6 on #128722 is authored by JoaoMarcos44 and explicitly attributes #129046.

Carried lifecycle obligations from the expanded issue ledger

These are inherited, source-revalidated obligations, not six additional freshly reproduced regressions. No closure is inferred from this Slack patch.

ID Pinned mechanism / owner Status in this pass
L01 Synchronous skill loading on async preparation Event-loop isolation remains a gateway skill-preparation obligation; no latency probe here.
L02 Queued follow-up combines new channel inputs with prior context Coherent queued prompt snapshot remains a follow-up/pin-owner obligation; no fresh full agent-chain probe.
L03 Reset freshness consumed before later transcript failure and failure return Retry must preserve initialization obligation until durable success; no failure/retry closure receipt.
L04 Eviction clears only context pin; rehydration returns if either pin remains Agent-cache versus instruction-authority recovery remains separate; no eviction/restart closure receipt.
L05 Manager/retry allocation and timestamp gate First-goal case freshly reproduced as F4. No independent /retry or /goal status closure claim.
L06 Telegram topic binding before event creation then gateway topic recovery Separate Telegram/routing owner; no Telegram execution in this pass.

CI and integration decision

Commit Actions receipt Check-runs / commit statuses
62ca64f Eight runs, all action_required (CI) None; combined status pending
5ee2a10 No runs listed None; combined status pending
d350e87 Four runs, all action_required (CI) None; combined status pending
315b292 Three runs, all action_required (CI) None; combined status pending
f3ec460 Three runs, all action_required (CI, Docker, Nix) None; combined status pending

Retain the demonstrated prompt-parity, denied-native-DM, classification/policy and goal-copy repairs. Resolve F1–F3 before treating this change as merge-ready; replace the F4 consumer-closure claim with lifecycle evidence and keep the inherited owners explicit. #128722 already supplies measured pause/FIFO behavior and the credited goal delta; coordinate that narrow repair with the separate MPIM policy work in #128767 rather than treating competing PRs as whole-class certification. Hosted CI must produce green receipts for every commit, and the local goal setup limitation still needs a clean execution environment.

if self._is_ignored_channel(channel_id):
return
classification = (
self._active_session_conversation_classification(channel_id, team_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Preserve control dispatch after an ordinary G conversation becomes idle

This path treats an absent active worker as absent conversation authority. _active_session_conversation_classification only examines the kind cache and _active_sessions; ordinary message ingress does not populate that cache. After a successful ordinary message in a legacy private channel or MPIM finishes, /new, /reset, /stop, /approve and /deny return at line 6264 without reaching the gateway, even with healthy Slack metadata and an authorized caller. The same isolated producer → real BasePlatformAdapter → consumer probe delivers all five controls on d350e874 and pinned main eaecc99; current head delivers zero. A matching active private-channel /approve still dispatches, so the missing case is the normal idle lifecycle, not the repaired busy path.

Retain workspace-scoped conversation authority from ordinary/persisted session state and provide a safe cold-idle classification path, while preserving the lookup-free busy path and fail-closed policy for genuinely unknown kinds. Cover first control and ordinary-message → completed worker → control for both G kinds, with DM/channel restrictions intact.

Owners: active-key recovery, ordinary ingress, control consumer.

)
from gateway.run import GatewayRunner
consumer = object.__new__(GatewayRunner)
consumer._hmwa_auto_load_skills(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Exercise the first goal through the real new-session gate

Calling _hmwa_auto_load_skills directly bypasses the eligibility condition in _hmwa_prepare_turn. The copied field is now correct, but a first /goal build it still produces no bound skill payload: real goal-manager lookup creates the route, the real queued-turn lookup touches updated_at, and _hmwa_open_session consequently reports neither equal timestamps nor a reset flag. The loader is never called despite kickoff auto_skill=['triage']. The same command → manager → FIFO → SessionStore → prepare probe loads the sentinel for a first direct question and for /goal after a real reset. These results hold on current head and the current-main composition, stopping at the turn-lease boundary before model execution.

Replace the claimed consumer regression with that lifecycle chain and assert payload presence in prepared model-bound text. Initialization eligibility needs its own durable obligation until the first successful consuming turn; merely adding this field cannot provide it. This is an inherited bootstrap gap, already tracked by #130316 / prior L05, rather than a regression introduced by the correct one-line field copy. Keep continuation non-reinjection coverage.

Owners: manager route allocation, activity touch, freshness decision, gated consumer.

@kvnloo kvnloo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Current promotion decision: close this branch as superseded by #128722.

The shared prompt resolver and latest active-session fixes are real, but exact head f3ec460def still has the reproduced introduced regressions documented in this thread: idle legacy-G... controls can be dropped after an ordinary conversation finishes, /pause remains metadata-dependent, and concurrent /queue handoff can reverse. #128722 eb546b6a67 preserves the same core Slack prompt parity while passing those three scenarios and retaining denied-DM policy.

#128722 itself still needs a current-main rebase and a production-GatewayRunner.pre_gateway_dispatch queue-order receipt before promotion; that is now the shortest path. The /goal auto_skill field copy from @JoaoMarcos44 should retain authorship/credit when carried forward, while #130316 remains the separate first-turn eligibility defect.

teknium1 pushed a commit that referenced this pull request Oct 16, 2026
_enqueue_goal_turn rebuilt the kickoff turn with the message id and
channel prompt but not auto_skill, so the turn the agent actually runs
for `/goal <text>` carried no channel skill binding. A resume
continuation still carries none.

Ported from #129046 (5ee2a10) onto #128722 as a credited delta, per
the maintainer review of #129046. The code line is unchanged; the
docstring line was adjusted to name bound skills when porting.

Refs #130316

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins needs-decision Awaiting maintainer decision before any implementation P0 Critical — data loss, security, crash loop platform/slack Slack app adapter sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Slack slash-command turns drop the channel prompt and source names, flipping the prompt pins

6 participants