Skip to content

feat(discord): add request-owned guild settings to discord_admin - #86432

Open
andrexibiza wants to merge 10 commits into
NousResearch:mainfrom
andrexibiza:feat/discord-api-guild-settings
Open

andrexibiza wants to merge 10 commits into
NousResearch:mainfrom
andrexibiza:feat/discord-api-guild-settings

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements Discord Feature Package A5 as a request-owned action on the existing discord_admin authority.

Exact head: d4364a5d960ae0057b078059cdb9ecb6e0101322
Current-main parent: 4a5b6dd4512a10c3c18da3e5b9e5c7fb681cbfbb
Current diff: seven files; no second public model tool.

Root cause closed

The prior branch had two contradictory shapes:

  1. a safe, transport-free PATCH /guilds/{guild_id} request builder; and
  2. a separate discord_guild_settings model tool that duplicated token, allowlist, transport, and error ownership.

That second tool could not participate correctly in discord.server_actions: the canonical allowlist validates names against discord_tool._ACTIONS, where the parallel action did not exist. Explicit allowlists therefore made the action impossible to enable, while tests hid the defect by monkeypatching the loader.

This head removes that parallel ownership model. Bounded feature modules now contribute action metadata to the canonical discord / discord_admin tools, while the existing owner retains credentials, config policy, REST transport, dynamic schema, and 403 enrichment.

Runtime contract

edit_current_guild_settings(settings) is exposed only through discord_admin and:

  • requires an active Discord request context;
  • requires an authenticated Discord requester;
  • derives the target guild exclusively from HERMES_SESSION_SCOPE_ID;
  • ignores any model-supplied guild_id for this action;
  • resolves the active-profile bot token through the canonical Discord owner;
  • honors the real discord.server_actions allowlist;
  • rejects non-object, unsupported, malformed, out-of-range, and empty PATCH bodies before transport;
  • preserves explicit false, 0, and null values;
  • returns canonical MANAGE_GUILD guidance on Discord 403 responses.

The scalar request builder remains transport-free and restricted to approved settings.

Architectural seam

  • tools/discord_api/action_registry.py defines bounded action metadata and rejects duplicate ownership/schema ambiguity.
  • tools/discord_action_extensions_tool.py composes discovered actions into the existing public tools without growing tools/discord_tool.py or registering another model surface.
  • tools/discord_api/guild_settings_action.py owns only A5 request-context validation and payload composition; credentials and transport remain canonical.

This seam is reusable by the remaining Discord admin packages instead of repeating the same ownership bug.

Verification

Local pre-publication receipts:

python -m py_compile <new implementation and test files>
pytest -q tests/tools/test_discord_action_registry.py tests/tools/test_discord_guild_settings_consumer.py
19 passed in 0.06s

The original guild-settings builder suite is retained. Hosted acceptance is exact-head CI attached to d4364a5d960ae0057b078059cdb9ecb6e0101322; no earlier green run is inherited.

Fixes #86431
Part of #79564

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets platform/discord Discord bot adapter labels Aug 14, 2026
@andrexibiza andrexibiza changed the title feat(discord): scalar guild-settings REST action (omniscience A5) feat(discord): scalar guild-settings REST action Aug 15, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

feat(discord): scalar guild-settings REST action

A clean, well-validated pure request builder; the test matrix (ranges, types, snowflake bounds, allowlists, only-provided-fields) is thorough. Observations:

  1. tools/discord_api/guild_settings.pyconsumer wiring: the diff contains only the builder + tests. If nothing in this PR calls edit_guild_request (i.e. the tool/action surface that sends the PATCH is a follow-up), this is currently dead code — worth stating the intended consumer (e.g. a discord_guild_settings tool or a /guild-settings command) so reviewers know it's not orphaned. If a follow-up PR wires it, consider landing the two together so the feature is end-to-end testable.
  2. _validate_snowflake — string snowflakes are validated against ^[0-9]{1,20}$ and int(value) <= _SNOWFLAKE_MAX, but the original string (leading zeros preserved) is returned as-is and later embedded in the request path/body. A "00000000000000000001" would pass validation and be sent verbatim; Discord may or may not tolerate leading zeros. Consider normalizing accepted string snowflakes to their canonical integer string (or returning the int) to keep the wire format predictable.
  3. Empty payload: edit_guild_request(GUILD_ID) returns {"method": "PATCH", "path": ..., "json": {}}. Discord's API will likely reject an empty PATCH body with a 400 (no fields to modify). Consider raising GuildSettingsError("no guild settings provided") when fields is empty, so callers fail fast with a clear message instead of a confusing API error.
  4. _validate_snowflake bool-before-int ordering is correct (bool is an int subclass) — good. afk_timeout bounds (60–3600) match the documented Discord range. The _FIELD_VALIDATORS allowlist pattern is the right call vs. forwarding arbitrary keys.
  5. Minor: __all__ exports the range constants but not the validator helpers — fine. The premium_progress_bar_enabled bool validator correctly rejects 1/0 ints.

andrexibiza commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Current-head/topology verification complete at be1ac30b0be133825dfc8441131097ff7cc759da against current main bdc5b1f74c26e6240deb8f067ade6dc91c00e8f2.

The branch is still builder-only. The actual production owner is the existing model-facing discord_admin surface in tools/discord_tool.py, which already owns profile-scoped bot-token resolution, REST transport, discord.server_actions policy, dynamic schema filtering, and enriched 403 handling. The guild-settings action should compose into that owner rather than introduce a second public Discord admin tool.

The current builder also has four contract defects:

  • int/string snowflakes are preserved rather than canonicalized to one decimal representation;
  • zero and leading-zero aliases are accepted;
  • an empty {} PATCH is allowed;
  • afk_timeout is treated as an arbitrary 60–3600 range and nsfw_level is exposed even though Modify Guild accepts the discrete AFK values and does not accept nsfw_level as an input.

I prepared the current-main repair with a real discord_admin consumer, positive decimal-string snowflake canonicalization, fail-closed empty payload handling, exact AFK enum validation, explicit-null preservation, and removal of the unsupported field. Focused verification is green: 25 passed; compileall also passes.

@andrexibiza
andrexibiza force-pushed the feat/discord-api-guild-settings branch from fb76942 to b089e9d Compare August 19, 2026 18:19
@andrexibiza
andrexibiza force-pushed the feat/discord-api-guild-settings branch from b089e9d to d8015d5 Compare August 19, 2026 18:28
@andrexibiza

Copy link
Copy Markdown
Contributor Author

Implementation complete — guild-settings consumer now executes the validated PATCH

Applied the handoff packet to a fresh worktree at PR branch feat/discord-api-guild-settings, rebased onto main via the branch (base ae6578aff8).

Commit

d8015d586619 feat(discord): register guild-settings consumer on discord_admin toolset

Pushed to axl-fork:feat/discord-api-guild-settings (force-with-lease). Verified remote head == d8015d586619b0d28dbe0b2677c585299b77b7d7.

Files changed (source of truth = packet files/)

File Status
tools/discord_api/guild_settings.py updated to packet version (hardened builder: [0-9]{1,64} snowflake regex, >0 canonical decimal range, Final annotations, EDITABLE_GUILD_SETTINGS export, transport-free docstring)
tools/discord_guild_settings_tool.py new — consumer registering discord_guild_settings under discord_admin, active-profile token resolution via _get_bot_token(), builder→tool error conversion pre-I/O, HTTP 403 → MANAGE_GUILD mapping
tests/tools/test_discord_guild_settings.py updated to packet version (canonicalization + boundary coverage)
tests/tools/test_discord_guild_settings_tool.py new — registry contract (admin-only, closed schema), validated-PATCH execution, empty-patch/unsupported-arg rejection before I/O, no-token failure, 403→MANAGE_GUILD

Verification (real checkout at exact pushed head d8015d586619)

$ python -m pytest tests/tools/test_discord_guild_settings.py tests/tools/test_discord_guild_settings_tool.py -v
....................... 58 passed in 7.31s

$ python -m py_compile tools/discord_api/guild_settings.py tools/discord_guild_settings_tool.py tests/tools/test_discord_guild_settings.py tests/tools/test_discord_guild_settings_tool.py
COMPILE_OK

$ git diff --check
DIFF_CHECK_CLEAN

Additional checks run against the same checkout:

  • Consumer imports and registers: registry._tools["discord_guild_settings"] resolves ✓
  • _discord_request signature confirmed: accepts body: kwarg ✓
  • Existing tools/discord_tool.py imports cleanly alongside the new consumer ✓
  • MANIFEST.sha256 verified 8/8 hashes against the packet contents before apply ✓

Acceptance gates — status

  • ✅ Consumer registered and reachable through discord_admin tool
  • ✅ Valid scalar update reaches the existing Discord REST transport (_discord_request)
  • ✅ Empty PATCH payloads fail before I/O (GuildSettingsError: "no guild settings provided", test_consumer_rejects_empty_patch_before_io)
  • ✅ Snowflake canonicalization covered (test_snowflakes_are_canonical_decimal_strings + test_invalid_guild_id_rejected)
  • ✅ Active-profile token selection covered (test_consumer_fails_when_active_profile_has_no_token)
  • ✅ Permission failures retain actionable Discord semantics (test_consumer_maps_manage_guild_403 → MANAGE_GUILD)
  • ✅ PR head pushed to fork

⚠️ CI status

Exact-head GitHub Actions did not trigger on the fork push: gh pr checks reports "no checks reported" for the pull_request event despite 3 pushes to axl-fork:feat/discord-api-guild-settings and 4+ minutes of polling (0 workflow runs, empty statusCheckRollup, mergeStateStatus: DIRTY, isCrossRepository: true). Fork PR workflow execution on this repo may require maintainer approval or an explicit trigger. Local green against the exact pushed head (58 passed + compile + diff --check clean) is provided as the proof gate per the handoff's own criterion (CI green or non-green proven unrelated). Will re-trigger CI if/when an explicit dispatch is available.

@andrexibiza andrexibiza changed the title feat(discord): scalar guild-settings REST action WIP(discord): scalar guild-settings authority gate pending Aug 20, 2026
@andrexibiza
andrexibiza marked this pull request as draft August 20, 2026 15:12
@andrexibiza
andrexibiza marked this pull request as ready for review August 20, 2026 15:20
@andrexibiza andrexibiza changed the title WIP(discord): scalar guild-settings authority gate pending feat(discord): gate scalar guild settings on authoritative guild ownership Aug 20, 2026
Rebuild A5 on current main as the issue-scoped pure request-builder contract. Remove the premature model-callable mutator and its arbitrary guild target; preserve only approved-scalar validation and focused tests.

Fixes NousResearch#86431
Part of NousResearch#79564
@andrexibiza
andrexibiza force-pushed the feat/discord-api-guild-settings branch from d8015d5 to d272399 Compare August 20, 2026 15:37
@andrexibiza andrexibiza changed the title feat(discord): gate scalar guild settings on authoritative guild ownership feat(discord): add scalar guild-settings REST action Aug 20, 2026
Close the allowlist ownership gap by registering bounded Discord actions
through the existing discord_admin authority. Keep credentials, config,
transport, and 403 policy with the canonical owner, remove the parallel
public tool, and merge current upstream main before publication.

Signed-off-by: Axl Ibiza <andrexibiza@gmail.com>
@andrexibiza andrexibiza changed the title feat(discord): add scalar guild-settings REST action feat(discord): add request-owned guild settings to discord_admin Aug 20, 2026
andrexibiza added a commit to andrexibiza/hermes-agent that referenced this pull request Aug 20, 2026
Correct the Discord REST v10 wire contract, attach permission overwrite
operations to the canonical discord_admin owner, and fail closed unless the
target channel belongs to the active request guild. This stack depends on the
bounded Discord action seam in NousResearch#86432.

Signed-off-by: Axl Ibiza <andrexibiza@gmail.com>

Copy link
Copy Markdown
Contributor Author

Exact-head closure receipt

Verified the live A5 guild-settings head d4364a5d960ae0057b078059cdb9ecb6e0101322 directly:

  • CI 32410094144success
  • Docker Build, Test, and Publish 32410093194success
  • Nix flake check 32410093189success
  • unresolved formal review threads — 0

This closes the author-side consumer/topology/hosted-verification residue for A5: the action is now owned by the canonical discord_admin surface rather than a parallel public tool, and the current merge object is exact-head green. The broader Discord Feature Parity campaign remains governed independently by its campaign ledger.

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/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have platform/discord Discord bot adapter type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discord A5: scalar guild-settings REST action (REST v10)

3 participants