Command system phases 2+3: authority, busy policy, native registration - #629
Merged
Conversation
Phase 2 — dispatch moves to the inbound router, before the channel queue. Control commands execute on a control plane (no inbound message, busy-immune, work mid-turn via shared response-mode/turn-active cells on ChannelState). Binding-level authority lists (+ adapter-instance defaults) gate state-mutating commands, with /help and /status always available and denials naming what the sender can run. Agent commands rewrite to MessageContent::Command; queued commands are acknowledged, Reject-policy commands refuse with a /stop pointer. Also fixes the coalesce-flush ordering so control commands don't wait out a batched LLM turn, and the @botName strip now checks it's actually our bot. Phase 3 — native registration generated from the registry. Discord: application commands with diff-only sync (guild-scoped from bindings, global otherwise, cap-aware), interaction handling with ephemeral deferral for control replies. Telegram: setMyCommands menu with hyphen->underscore mangling that folds back on parse. Slack: one /spacebot umbrella with subcommands; the config-driven command alias system (SlackCommandConfig, slack_command_agent_id) is gone. Ephemeral replies now render in portal (SSE) instead of vanishing. Parity test walks command x platform so a command can't silently miss a surface.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis change adds authority-gated commands, shared inbound dispatch, live response controls, native command registration, authority configuration, binding management, and autonomy and wake API schemas. ChangesCommand access and native command flow
Configuration, binding, and API contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jamiepine
marked this pull request as ready for review
August 9, 2026 09:36
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/content/docs/(configuration)/config.mdx (1)
657-700: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument authority settings for every supported adapter.
AdapterAuthorityDefaults::from_configalso reads authority lists for Telegram instances, Twitch, Signal, Mattermost, and Email. Add these fields to their root and named-instance tables. Operators otherwise cannot discover all supported authority configuration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/`(configuration)/config.mdx around lines 657 - 700, Extend the configuration documentation to include the authority setting for every supported adapter handled by AdapterAuthorityDefaults::from_config. Add the appropriate authority rows to the root and named-instance tables for Telegram, Twitch, Signal, Mattermost, and Email, matching the existing defaults and descriptions used by the implementation.src/main.rs (1)
1317-1334: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winResolve binding scope for preassigned portal messages.
PortalSendRequestsetsInboundMessage.agent_id, andBinding::matches_routecan route a portal message by matching that agent ID. Because dispatch already bypassesresolve_agent_for_messageandmatched_binding_authoritywhenagent_idis present, bindingsettingsandauthorityare skipped. Computebinding_settingsandbinding_authorityeven whenmessage.agent_idis already set, using the same route match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.rs` around lines 1317 - 1334, Update the agent-selection branch around message.agent_id so preassigned portal messages also resolve their matching binding metadata. When agent_id is already present, load the current bindings and use the same route-matching logic to populate binding_settings and binding_authority; retain the existing resolved-agent behavior and fallback resolution for messages without an agent_id.
🧹 Nitpick comments (1)
src/commands/native.rs (1)
107-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
ArgSpec::hint()for the choice branch.
ArgSpec::hint()already formatsChoiceas[a|b](src/commands/registry.rslines 142-148). The explicitif let ArgSpec::Choice(...)branch duplicates that formatting. A future change tohint()will silently diverge from the Telegram menu.♻️ Proposed simplification
let mut description = def.description.to_string(); - if let ArgSpec::Choice(options) = def.args { - description.push_str(&format!(" [{}]", options.join("|"))); - } else if let Some(hint) = def.args.hint() { + if let Some(hint) = def.args.hint() { description.push_str(&format!(" {hint}")); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/native.rs` around lines 107 - 111, In the command description construction, remove the explicit ArgSpec::Choice branch and rely on def.args.hint() for all argument types, including choices. Preserve the existing formatting produced by ArgSpec::hint() so choice options remain displayed as [a|b] consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Line 800: Update the authority configuration row in the table to include the
missing [] default cell before its description, preserving the existing type and
description columns.
In `@src/commands/access.rs`:
- Around line 101-105: Update the binding selection logic in the access command
to distinguish an omitted binding_authority from an explicitly empty list,
ensuring the empty list is preserved and does not fall through to
adapter_default. Keep adapter fallback only for the omitted-list case, and add a
regression test covering an explicitly empty binding_authority with a restricted
adapter default.
In `@src/commands/control.rs`:
- Around line 170-192: In src/commands/control.rs lines 170-192, update the
PortalConversationStore flow so a get failure does not become default settings
followed by an upsert; instead return a failure reply or use an atomic
response-mode-only update. Apply the same safeguard in src/commands/control.rs
lines 196-220 for ChannelSettingsStore: preserve existing settings through an
atomic response-mode-only update or return a failure reply, and do not report
success after persistence fails.
In `@src/commands/dispatch.rs`:
- Around line 115-118: Serialize SetResponseMode execution per conversation
instead of spawning each control command independently in the dispatch path.
Update the task flow around plane.execute and send_ephemeral to enqueue these
state-mutating commands in a per-conversation FIFO executor, while preserving
existing handling for other commands and response delivery.
In `@src/commands/native.rs`:
- Around line 112-116: Update the minimum description check in the command
description handling to use character count rather than String::len() bytes.
Keep the existing fallback to /{command}, and align the guard with the
chars().count() unit used by the test.
In `@src/messaging/discord.rs`:
- Around line 1143-1170: Update sync_application_commands to iterate all
configured guild IDs rather than only ready_guilds, using each ID to fetch and
register commands independently. Preserve the existing in-sync fast path, but
log fetch, registration, and non-member/guild-create errors with the guild ID
while continuing to process the remaining configured guilds instead of silently
skipping them.
- Around line 1009-1064: Add Discord-specific validation in
build_discord_create_commands before constructing commands passed to
set_commands or set_global_commands. Validate spec.name, spec.description,
option hints, and each choice against Discord’s field limits, excluding or
handling invalid entries so one invalid spec cannot fail the batch while
preserving the dropped-count behavior. Add a Discord limits test modeled on
telegram_menu_entries_satisfy_platform_limits.
In `@src/messaging/slack.rs`:
- Around line 487-517: Filter the resolved command in the Slack subcommand
handling flow so commands with `def.availability.slack` set to false are treated
as unresolved. Preserve `REGISTRY.resolve(&subcommand)` alias and
case-insensitive matching, but only dispatch when the resolved definition is
available on `Surface::Slack`; otherwise continue through the existing
usage/listing response path.
---
Outside diff comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Around line 657-700: Extend the configuration documentation to include the
authority setting for every supported adapter handled by
AdapterAuthorityDefaults::from_config. Add the appropriate authority rows to the
root and named-instance tables for Telegram, Twitch, Signal, Mattermost, and
Email, matching the existing defaults and descriptions used by the
implementation.
In `@src/main.rs`:
- Around line 1317-1334: Update the agent-selection branch around
message.agent_id so preassigned portal messages also resolve their matching
binding metadata. When agent_id is already present, load the current bindings
and use the same route-matching logic to populate binding_settings and
binding_authority; retain the existing resolved-agent behavior and fallback
resolution for messages without an agent_id.
---
Nitpick comments:
In `@src/commands/native.rs`:
- Around line 107-111: In the command description construction, remove the
explicit ArgSpec::Choice branch and rely on def.args.hint() for all argument
types, including choices. Preserve the existing formatting produced by
ArgSpec::hint() so choice options remain displayed as [a|b] consistently.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bd67cf43-72f5-40f0-a9cf-ec7d75631f89
📒 Files selected for processing (27)
docs/content/docs/(configuration)/config.mdxdocs/content/docs/(messaging)/discord-setup.mdxdocs/content/docs/(messaging)/slack-setup.mdxinterface/src/api/schema.d.tssrc/agent/channel.rssrc/agent/process_control.rssrc/api/bindings.rssrc/api/messaging.rssrc/cli/binding.rssrc/commands.rssrc/commands/access.rssrc/commands/control.rssrc/commands/dispatch.rssrc/commands/native.rssrc/commands/registry.rssrc/config.rssrc/config/load.rssrc/config/toml_schema.rssrc/config/types.rssrc/config/watcher.rssrc/conversation/settings.rssrc/main.rssrc/messaging/discord.rssrc/messaging/email.rssrc/messaging/slack.rssrc/messaging/telegram.rstests/context_dump.rs
💤 Files with no reviewable changes (1)
- src/api/messaging.rs
…d native registration - binding authority is now Option: omitted defers to the adapter default, explicit [] opens the scope even when the adapter default is restricted - response-mode changes fail loudly: a settings read/write error no longer upserts defaults or reports a false confirmation, and the live cell only updates after a successful persist - SetResponseMode executes inline in the router so mode changes apply in delivery order; read-only control commands stay on spawned tasks - discord: validate command names/choices and clamp descriptions before bulk registration so one bad spec can't fail the batch; sync every configured guild instead of only READY payload guilds, logging per-guild failures - slack: /spacebot subcommands only resolve commands available on slack - telegram: description minimum uses char count; choice hints reuse ArgSpec::hint() - portal sends with a preassigned agent_id resolve binding settings/authority - docs: authority rows for telegram instances, twitch, signal, mattermost, email; bindings table gets its missing default column
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/bindings.rs (1)
57-58: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve explicit empty authority lists in API requests.
Vec<String>with#[serde(default)]makes an omitted value andauthority: []identical. The create and update writers then omit the TOML key for both cases. A client cannot create a binding-level open override when an adapter default restricts access.
src/api/bindings.rs#L57-L58: Use an optional authority field so omitted and explicit empty values remain distinct.src/api/bindings.rs#L169-L170: Preserve the same distinction for update requests.src/api/bindings.rs#L502-L508: Writeauthority = []when the request containsSome(vec![]).src/api/bindings.rs#L888-L897: Remove the key only when authority is omitted according to the update contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/bindings.rs` around lines 57 - 58, Preserve omitted versus explicitly empty authority values across the binding API. In src/api/bindings.rs lines 57-58 and 169-170, change the create and update request authority fields to optional values without collapsing explicit empty arrays. In lines 502-508, write authority = [] for Some(vec![]), while omitting the key only for None; apply the same omission rule in the update writer at lines 888-897, using the relevant create/update request and writer symbols.src/messaging/discord.rs (1)
1212-1218: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRegister guild commands before removing global commands.
set_global_commands(http, Vec::new())runs before the guild fetch and registration loop. If a guild registration then fails, that guild can have neither its former global commands nor its new guild commands until a laterReadyevent. Sync all configured guilds first. Clear global commands only after every target guild is confirmed in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/messaging/discord.rs` around lines 1212 - 1218, Reorder the command synchronization flow around ApplicationCommand::set_global_commands: complete the guild fetch and registration loop for every configured guild first, and only clear non-empty global commands after all target guilds have been confirmed in sync. Preserve the existing warning behavior for failures while ensuring a guild registration failure does not leave that guild without either command set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Line 710: Update every named-instance authority row in
docs/content/docs/(configuration)/config.mdx at lines 710, 734, 781, 808, and
836 to document that an empty authority list means open access by adding “Empty
= open” to each description.
In `@src/commands/control.rs`:
- Around line 222-244: Update persist_response_mode and the underlying
ChannelSettingsStore/PortalConversationStore write paths to prevent
read-modify-write races when changing response_mode. Use an existing
per-conversation lock, optimistic revision check, or atomic response_mode-only
update, and ensure concurrent settings writers cannot overwrite fields changed
after the initial read.
In `@src/messaging/discord.rs`:
- Around line 1017-1051: Rename the newly introduced abbreviated variables to
descriptive names: in src/messaging/discord.rs lines 1017-1051, rename the
discord_spec_violation parameter spec; in src/messaging/discord.rs lines
1077-1094, rename the spec closure and collection variables; and in
src/messaging/slack.rs lines 428-430, rename the def closure variable. Use names
such as command_spec and command_definition consistently at each affected site.
---
Outside diff comments:
In `@src/api/bindings.rs`:
- Around line 57-58: Preserve omitted versus explicitly empty authority values
across the binding API. In src/api/bindings.rs lines 57-58 and 169-170, change
the create and update request authority fields to optional values without
collapsing explicit empty arrays. In lines 502-508, write authority = [] for
Some(vec![]), while omitting the key only for None; apply the same omission rule
in the update writer at lines 888-897, using the relevant create/update request
and writer symbols.
In `@src/messaging/discord.rs`:
- Around line 1212-1218: Reorder the command synchronization flow around
ApplicationCommand::set_global_commands: complete the guild fetch and
registration loop for every configured guild first, and only clear non-empty
global commands after all target guilds have been confirmed in sync. Preserve
the existing warning behavior for failures while ensuring a guild registration
failure does not leave that guild without either command set.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 35e32062-09d5-4cf9-92fa-fca32ac11738
📒 Files selected for processing (13)
docs/content/docs/(configuration)/config.mdxinterface/src/api/schema.d.tssrc/api/bindings.rssrc/commands/access.rssrc/commands/control.rssrc/commands/dispatch.rssrc/commands/native.rssrc/config.rssrc/config/toml_schema.rssrc/config/types.rssrc/main.rssrc/messaging/discord.rssrc/messaging/slack.rs
🚧 Files skipped from review as they are similar to previous changes (8)
- interface/src/api/schema.d.ts
- src/commands/dispatch.rs
- src/config/types.rs
- src/config.rs
- src/main.rs
- src/config/toml_schema.rs
- src/commands/access.rs
- src/commands/native.rs
Comment on lines
+1017
to
+1051
| fn discord_spec_violation(spec: &crate::commands::native::NativeCommandSpec) -> Option<String> { | ||
| use crate::commands::native::NativeArg; | ||
|
|
||
| // Registry names are ASCII; Discord additionally allows lowercase | ||
| // unicode letters, which no command uses. | ||
| let name_valid = !spec.name.is_empty() | ||
| && spec.name.chars().count() <= DISCORD_NAME_MAX | ||
| && spec | ||
| .name | ||
| .chars() | ||
| .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-' || c == '_'); | ||
| if !name_valid { | ||
| return Some(format!( | ||
| "name must be 1-{DISCORD_NAME_MAX} lowercase [a-z0-9_-] characters" | ||
| )); | ||
| } | ||
| if let Some(NativeArg::Choice { options }) = &spec.arg { | ||
| if options.len() > DISCORD_CHOICE_CAP { | ||
| return Some(format!( | ||
| "{} choices exceed discord's cap of {DISCORD_CHOICE_CAP}", | ||
| options.len() | ||
| )); | ||
| } | ||
| // A truncated choice value would dispatch different args than the | ||
| // user picked, so over-long choices invalidate the command instead. | ||
| if options | ||
| .iter() | ||
| .any(|option| option.is_empty() || option.chars().count() > DISCORD_CHOICE_VALUE_MAX) | ||
| { | ||
| return Some(format!( | ||
| "choice values must be 1-{DISCORD_CHOICE_VALUE_MAX} characters" | ||
| )); | ||
| } | ||
| } | ||
| None |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use descriptive variable names.
Replace newly introduced spec and def variables with names such as command_spec and command_definition.
src/messaging/discord.rs#L1017-L1051: rename thespecparameter indiscord_spec_violation.src/messaging/discord.rs#L1077-L1094: rename thespecclosure and collection variables.src/messaging/slack.rs#L428-L430: rename thedefclosure variable.
As per coding guidelines, “Don't abbreviate variable names. Use queue not q, message not msg, channel not ch.”
📍 Affects 2 files
src/messaging/discord.rs#L1017-L1051(this comment)src/messaging/discord.rs#L1077-L1094src/messaging/slack.rs#L428-L430
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/messaging/discord.rs` around lines 1017 - 1051, Rename the newly
introduced abbreviated variables to descriptive names: in
src/messaging/discord.rs lines 1017-1051, rename the discord_spec_violation
parameter spec; in src/messaging/discord.rs lines 1077-1094, rename the spec
closure and collection variables; and in src/messaging/slack.rs lines 428-430,
rename the def closure variable. Use names such as command_spec and
command_definition consistently at each affected site.
Source: Coding guidelines
…w-feedback # Conflicts: # src/agent/channel.rs # tests/context_dump.rs
…feedback - response_mode persists via a single json_set statement in both stores, so a mode change can't race concurrent whole-row settings writers and restore stale fields - bindings API create/update requests take Option<Vec> for authority: omitted writes no TOML key, [] writes an explicit empty list that opens the scope - discord: stale global commands are cleared only after every configured guild is confirmed in sync, so a failed guild registration keeps the globals as a fallback until the next ready re-sync - docs: named-instance authority rows note that empty means open
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/control.rs (1)
210-236: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGuard response_mode updates from whole-settings writes.
persist_response_modenow patches onlyresponse_mode, but/channels/{channel_id}/settingsPUT reads the full channel settings, thenChannelSettingsStore::upsertwrites the entireConversationSettingsvalue. A concurrent settings update that completes after this patch can overwrite the new mode. Use atomic JSON updates for the full-settings flow too, or make mode the final write after settings changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/control.rs` around lines 210 - 236, Update the full-settings PUT flow around ChannelSettingsStore::upsert so it cannot overwrite a concurrently persisted response mode. Replace the whole-settings write with an atomic JSON/settings update that preserves the current response_mode, or perform the response_mode patch after the full settings write; ensure persist_response_mode remains protected from later stale upserts.
🧹 Nitpick comments (1)
src/conversation/channel_settings.rs (1)
45-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBoth
ChannelSettingsStore::set_response_modeandPortalConversationStore::set_response_modeimplement the identical block that serializesResponseModeto its JSON string form (serde_json::to_value(mode)→.as_str()→.to_string(), with the same error message). Extract this into one shared helper, for example a method onResponseModeinsrc/conversation/settings.rs, so the two stores stay in sync if the serialization ever changes.
src/conversation/channel_settings.rs#L45-L77: replace the inline serialization block with a call to the shared helper.src/conversation/portal.rs#L190-L221: replace the inline serialization block with a call to the same shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/conversation/channel_settings.rs` around lines 45 - 77, Extract the duplicated ResponseMode serialization logic into one shared helper, preferably on ResponseMode in src/conversation/settings.rs, preserving the existing error behavior and returned string. Replace the inline serde_json conversion in ChannelSettingsStore::set_response_mode at src/conversation/channel_settings.rs:45-77 and PortalConversationStore::set_response_mode at src/conversation/portal.rs:190-221 with calls to that helper so both stores remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/commands/control.rs`:
- Around line 210-236: Update the full-settings PUT flow around
ChannelSettingsStore::upsert so it cannot overwrite a concurrently persisted
response mode. Replace the whole-settings write with an atomic JSON/settings
update that preserves the current response_mode, or perform the response_mode
patch after the full settings write; ensure persist_response_mode remains
protected from later stale upserts.
---
Nitpick comments:
In `@src/conversation/channel_settings.rs`:
- Around line 45-77: Extract the duplicated ResponseMode serialization logic
into one shared helper, preferably on ResponseMode in
src/conversation/settings.rs, preserving the existing error behavior and
returned string. Replace the inline serde_json conversion in
ChannelSettingsStore::set_response_mode at
src/conversation/channel_settings.rs:45-77 and
PortalConversationStore::set_response_mode at src/conversation/portal.rs:190-221
with calls to that helper so both stores remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ef3562ef-6805-4005-906a-9d7aaed9055d
📒 Files selected for processing (14)
docs/content/docs/(configuration)/config.mdxinterface/src/api/schema.d.tssrc/agent/channel.rssrc/api/bindings.rssrc/commands/control.rssrc/config/load.rssrc/config/toml_schema.rssrc/config/types.rssrc/config/watcher.rssrc/conversation/channel_settings.rssrc/conversation/portal.rssrc/main.rssrc/messaging/discord.rstests/context_dump.rs
🚧 Files skipped from review as they are similar to previous changes (8)
- tests/context_dump.rs
- src/config/types.rs
- src/api/bindings.rs
- src/agent/channel.rs
- src/config/load.rs
- src/messaging/discord.rs
- src/main.rs
- docs/content/docs/(configuration)/config.mdx
… setter - the channel's in-queue SetResponseMode arm still did a spawned read-modify-write with a default fallback; it now uses ChannelSettingsStore::set_response_mode, so a stale upsert can't restore old fields over a concurrent settings write or router-side mode change - ResponseMode::as_setting_str replaces the duplicated serde string conversion in both stores
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phases 2 and 3 of the command system design (docs/design-docs/slash-commands.md). Stacks on the phase 1 registry from #626.
Phase 2 — access and busy policy
Dispatch moves out of the channel actor into the inbound router, before the message queue — where the matched binding already lives. Control commands now execute on a control plane (
src/commands/control.rs) without creating an inbound message, which makes them busy-immune by construction:/quietlands mid-turn instead of sitting behind an in-flight LLM turn. Live state is shared through two cells onChannelState(turn-active flag, response mode) reachable viaChannelControlHandle, so mode changes apply immediately and the router can answer "busy?" without entering the queue.Authority is opt-in per scope:
Resolution is binding list first, then an adapter-instance
authoritydefault, open when neither is set — existing configs behave exactly as before./helpand/statusare always available, denials name the commands the sender can run, and/helpoutput is filtered to what the caller can actually use. Agent commands rewrite toMessageContent::Command; queued-behind-a-turn commands get an ephemeral acknowledgment, andBusyPolicy::Rejectrefuses with a pointer to/stop(no command in the current set uses Reject yet — it's wired for the phase 4 set).Fixes along the way: the coalesce flush no longer runs a full batched turn before a control command is handled;
/status@someotherbotin a Telegram group is no longer answered (the parser now checks the addressed bot is us); ephemeral replies render in the portal via SSE instead of being silently dropped.Phase 3 — native registration
All generated from the registry through platform-neutral specs in
src/commands/native.rs:ready: fetch, compare, skip when identical, bulk overwrite when not. Guild-scoped when bindings declare guilds, global otherwise, cap-aware with one log line counting anything dropped. Slash-command interactions defer (ephemeral for Control), and replies resolve the deferral via an interaction-token map; expired tokens fall back to normal channel sends.setMyCommandsat adapter start. Hyphenated names mangle to underscores (/mention_only) and the parser folds them back on resolve./spacebotumbrella with subcommands over the existing Socket Mode connection. The config-driven alias system is gone:SlackCommandConfig, its loader, fingerprint segments, and the never-readslack_command_agent_idmetadata are deleted (the oldagent_idmapping never routed anything — routing was already binding-based).A parity test walks command × platform, so a command available on a surface but missing from its native listing fails the build instead of silently disappearing.
Docs:
authorityrows in the config reference, Slack slash-command setup section,applications.commandsadded to the Discord invite scopes.Testing
New unit tests for authority resolution (wildcard, scope precedence, empty-list semantics, denial text), parser addressing, and the native spec generators including Telegram naming limits and the parity walk.
just gate-prgreen: fmt, clippy -Dwarnings, 954 lib tests, integration targets compile, typegen regenerated.Not in this PR: the phase 4 command set (
/stop,/retry,/memory,/model, ...),GET /api/commands+POST /api/channels/:id/commandfor the portal palette, and Mattermost native ephemeral posts (router replies degrade to regular messages there).