fix(admin): automod-bypass reports success when a Discord role change fails by SebTardif · Pull Request #26 · openclaw/hermit

SebTardif · 2026-08-15T21:14:32Z

What Problem This Solves

Fixes an issue where Discord admins running /admin automod-bypass-toggle would be told the automod bypass role was added or removed even when Discord rejected the role change (missing permissions, role hierarchy, or API error). The member's roles stayed the same while the command reported a new security state.

That false success is worse than a failed command: staff can believe someone is exempt from automod when they are not, or believe a bypass was revoked when it is still attached.

Why This Change Was Made

/admin automod-bypass-toggle now waits for addRole / removeRole to finish. If Discord rejects the change, the command replies with an ephemeral error and does not claim success. The previous success copy is sent only after the role update resolves. /admin inactivity-warn is unchanged.

User Impact

Admins get an accurate private reply about whether automod bypass was actually granted or removed. A Discord API failure no longer looks like a completed security change.

Evidence

Live bun run of /tmp/proof-hermit-automod.ts against this branch. The script calls the previous fire-and-forget handler and the patched AutomodBypassToggle.run with a Discord member whose addRole / removeRole throws Missing Permissions, then prints the command replies.

$ bun /tmp/proof-hermit-automod.ts
BEFORE addRole reject
[
  {
    "content": "Added automod bypass role to <@user-123>.",
    "ephemeral": true
  }
]
BEFORE removeRole reject
[
  {
    "content": "Removed automod bypass role from <@user-123>.",
    "ephemeral": true
  }
]
AFTER addRole reject
[
  {
    "content": "Failed to add automod bypass role.",
    "ephemeral": true
  }
]
AFTER removeRole reject
[
  {
    "content": "Failed to remove automod bypass role.",
    "ephemeral": true
  }
]
AFTER addRole ok
[
  {
    "content": "Added automod bypass role to <@user-123>.",
    "ephemeral": true
  }
]
AFTER removeRole ok
[
  {
    "content": "Removed automod bypass role from <@user-123>.",
    "ephemeral": true
  }
]

Before this patch the command always printed the success sentence. After the patch a rejected role update prints Failed to add/remove automod bypass role. and the success sentence is reserved for a resolved role update.

Real behavior proof

Behavior or issue addressed: /admin automod-bypass-toggle reported that automod bypass was added or removed even when Discord rejected the role change, so staff could act on a false security state.
Real environment tested: macOS 26.6.1 Darwin 25.6.0 arm64, Node v26.7.0, bun 1.3.14, patched checkout /tmp/oc-impl-hermit-automod on fix/automod-bypass-role-errors (base 97d3731).
Exact steps or command run after this patch:
```
bun /tmp/proof-hermit-automod.ts
```

Evidence after fix: terminal output from the patched command class:

$ bun /tmp/proof-hermit-automod.ts
AFTER addRole reject
[
  {
    "content": "Failed to add automod bypass role.",
    "ephemeral": true
  }
]
AFTER removeRole reject
[
  {
    "content": "Failed to remove automod bypass role.",
    "ephemeral": true
  }
]
AFTER addRole ok
[
  {
    "content": "Added automod bypass role to <@user-123>.",
    "ephemeral": true
  }
]
AFTER removeRole ok
[
  {
    "content": "Removed automod bypass role from <@user-123>.",
    "ephemeral": true
  }
]

Observed result after fix: A rejected addRole / removeRole now replies Failed to add automod bypass role. or Failed to remove automod bypass role. (ephemeral). The success sentence is printed only after the role update resolves.
What was not tested: A live Discord guild with a real role hierarchy or missing-permissions token. The inactivity-warn thread member path is unchanged.

Origin

This behavior landed in 3e71d04 (admin and team lead commands, 2026-04-07) and has been present for 130 days. user.addRole(...).catch(() => {}) / user.removeRole(...).catch(() => {}) discarded the Discord error, then the command still replied with the success sentence.

Sibling /role already awaits addRole / removeRole before replying. Beta Pings already replies privately when the Discord role update throws.

Await addRole/removeRole on /admin automod-bypass-toggle. Reply with an ephemeral error when Discord rejects the role change, and only report success after the role update resolves. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

clawsweeper · 2026-08-15T21:14:35Z

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

clawsweeper · 2026-08-15T21:17:57Z

Codex review: needs real behavior proof before merge. Reviewed August 22, 2026, 8:08 AM ET / 12:08 UTC.

ClawSweeper review

What this changes

The PR awaits Discord automod-bypass role changes, reports failures privately, and adds success and rejection regression tests.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 5 items remain

The role-update fix remains necessary because current main still suppresses Discord role errors before reporting success. The behavioral approach is sound, but the branch still has the two previously reported repository-policy defects and only mock-based proof.

Priority: P2
Reviewed head: 8e708e2a6c251d05af2e161d3f4fb8c5c1280f1d

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The core repair is focused and tested, but unresolved response-policy findings and mock-only proof keep it below merge-ready.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The terminal transcript exercises injected member-method failures rather than a real Discord client and guild role mutation; add redacted after-fix live Discord evidence before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment `@clawsweeper re-review`.
Patch quality	🦐 gold shrimp (3/6)	2 actionable review findings remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The terminal transcript exercises injected member-method failures rather than a real Discord client and guild role mutation; add redacted after-fix live Discord evidence before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment `@clawsweeper re-review`.
Evidence reviewed	6 items	Current-main defect: Current main suppresses rejected role mutations with an empty catch, then immediately sends a success reply in both add and remove paths. Focused behavioral repair: The PR awaits each role mutation and returns from its rejection path before the success reply; its added tests cover both failures and successes. Carbon response pattern: The established Beta Pings role-error path uses Carbon Container and TextDisplay components for replies, including its failure response.
Findings	2 actionable findings	[P2] Use Carbon v2 components for both failure replies [P2] Move automod failure copy into response configuration
Security	None	None.

Live Verification

Command: bun test tests/adminAutomodBypass.test.ts

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

FAIL expect_output: 4 pass

How this fits together

The guild-only admin command receives a Discord interaction, changes a member’s automod-bypass role, then sends the administrator a private result. Its reply determines whether staff can trust the reported moderation state after Discord accepts or rejects the role mutation.

flowchart LR
A[Discord administrator] --> B[Admin command]
B --> C[Member role update]
C --> D{Discord accepts?}
D -->|Yes| E[Private success reply]
D -->|No| F[Private failure reply]

Before merge

Add real behavior proof - Needs real behavior proof before merge: The terminal transcript exercises injected member-method failures rather than a real Discord client and guild role mutation; add redacted after-fix live Discord evidence before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Use Carbon v2 components for both failure replies (P2) - The new rejection paths reply with raw content, despite the applicable repository policy requiring Carbon v2 components. Follow the local Beta Pings error pattern with a Container and TextDisplay, and update the test to inspect the serialized component text.
Move automod failure copy into response configuration (P2) - The new failure text is hard-coded in the command even though repository policy keeps automod response templates in src/config/automod-messages.json. Centralize both failure templates there so command behavior does not create a second copy source.
Resolve merge risk (P1) - The supplied proof invokes synthetic member methods rather than Discord’s real role-mutation API, so it cannot establish the after-fix behavior in a guild with actual permission or hierarchy failure.
Complete next step (P2) - The two code-policy findings are mechanical and confined to the command, response configuration, and focused regression test; the contributor must separately supply real Discord proof.

Findings

[P2] Use Carbon v2 components for both failure replies — src/commands/admin.ts:171-174
[P2] Move automod failure copy into response configuration — src/commands/admin.ts:172

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and regression-test delta	production +18, tests +109	The 18-line behavior change is covered by four focused success and rejection cases, though the response-format assertions need updating with the Carbon repair.

Merge-risk options

Maintainer options:

Decide the mitigation before merge
Keep the await-and-return behavior, render the new failure replies with Carbon v2 components backed by automod response configuration, then attach redacted live Discord success and rejection evidence.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep the await-and-return behavior, render the new failure replies with Carbon v2 components backed by automod response configuration, then attach redacted live Discord success and rejection evidence.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: current main discards a rejected addRole or removeRole promise and then unconditionally sends the corresponding success reply.

Is this the best way to solve the issue?

No: awaiting the role operation is the right behavioral core, but the new failure replies must follow the repository’s Carbon v2 and config-driven response conventions.

Full review comments:

[P2] Use Carbon v2 components for both failure replies — src/commands/admin.ts:171-174
The new rejection paths reply with raw content, despite the applicable repository policy requiring Carbon v2 components. Follow the local Beta Pings error pattern with a Container and TextDisplay, and update the test to inspect the serialized component text.
Confidence: 0.99
[P2] Move automod failure copy into response configuration — src/commands/admin.ts:172
The new failure text is hard-coded in the command even though repository policy keeps automod response templates in src/config/automod-messages.json. Centralize both failure templates there so command behavior does not create a second copy source.
Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 17a621b47dba.

Labels

Label justifications:

P2: A bounded admin-command defect can falsely report a moderation role state, but it is not a core-runtime outage.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The terminal transcript exercises injected member-method failures rather than a real Discord client and guild role mutation; add redacted after-fix live Discord evidence before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Acceptance criteria:

[P1] bun test tests/adminAutomodBypass.test.ts.
[P1] bun run typecheck.

What I checked:

Current-main defect: Current main suppresses rejected role mutations with an empty catch, then immediately sends a success reply in both add and remove paths. (src/commands/admin.ts:167, 17a621b47dba)
Focused behavioral repair: The PR awaits each role mutation and returns from its rejection path before the success reply; its added tests cover both failures and successes. (src/commands/admin.ts:169, 8e708e2a6c25)
Carbon response pattern: The established Beta Pings role-error path uses Carbon Container and TextDisplay components for replies, including its failure response. (src/components/betaPingsButton.ts:66, 17a621b47dba)
Repository policy: The applicable repository policy requires Carbon v2 response components and keeps automod response templates in src/config/automod-messages.json; both added failure strings remain raw payload content in command code. (AGENTS.md:3)
Unresolved prior findings: The reviewed head is unchanged from the prior ClawSweeper cycle, so its two previously raised findings remain applicable rather than being new late findings. (src/commands/admin.ts:171, 8e708e2a6c25)
Feature-history routing: Hannes Rudolph has the strongest recent current-main history on the admin command, including the two latest non-PR changes to that file. (src/commands/admin.ts:14, ae6ce064361e)

Likely related people:

Hannes Rudolph: Recent current-main commits ae6ce06 and 7de378b both touched the admin-command surface; related role-response work also appears in the Beta Pings implementation. (role: recent area contributor; confidence: medium; commits: ae6ce064361e, 7de378b8dc60, 7638cdb1bb7a; files: src/commands/admin.ts, src/components/betaPingsButton.ts, src/services/betaPings.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

Replace the two raw failure payloads with config-backed Carbon v2 response components and update the focused test.
Add redacted live Discord evidence showing a rejected role mutation yields the failure reply and a successful mutation yields the success reply.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)

reviewed 2026-08-15T21:16:51.794Z sha 8e708e2 :: needs real behavior proof before merge. :: [P2] Use Carbon v2 components for the failure replies
reviewed 2026-08-21T14:07:52.438Z sha 8e708e2 :: needs real behavior proof before merge. :: [P2] Use Carbon v2 components for the failure replies | [P2] Move automod failure copy into the response config
reviewed 2026-08-21T22:59:30.267Z sha 8e708e2 :: needs real behavior proof before merge. :: [P2] Use Carbon v2 components for failure replies | [P2] Move automod failure copy into response configuration

Navigation Menu

Uh oh!

Uh oh!

fix(admin): automod-bypass reports success when a Discord role change fails - #26

fix(admin): automod-bypass reports success when a Discord role change fails#26
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/automod-bypass-role-errors

SebTardif commented Aug 15, 2026

Uh oh!

clawsweeper Bot commented Aug 15, 2026

Uh oh!

clawsweeper Bot commented Aug 15, 2026 •

edited

Loading

Security

Review metrics

Merge-risk options

Technical review

Labels

Evidence

Rank-up moves

Rating scale

Workflow

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

SebTardif commented Aug 15, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Real behavior proof

Origin

Uh oh!

clawsweeper Bot commented Aug 15, 2026

Uh oh!

clawsweeper Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review

What this changes

Merge readiness

Review scores

Verification

Live Verification

How this fits together

Before merge

Findings

Security

Review metrics

Merge-risk options

Technical review

Labels

Evidence

Rank-up moves

Rating scale

Workflow

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Aug 15, 2026 •

edited

Loading