Skip to content

fix(helm): accept "multibot-mentions" in allowUserMessages validation#472

Merged
thepagent merged 3 commits intoopenabdev:mainfrom
brettchien:fix/helm-multibot-mentions-validation
Apr 19, 2026
Merged

fix(helm): accept "multibot-mentions" in allowUserMessages validation#472
thepagent merged 3 commits intoopenabdev:mainfrom
brettchien:fix/helm-multibot-mentions-validation

Conversation

@brettchien
Copy link
Copy Markdown
Contributor

@brettchien brettchien commented Apr 19, 2026

What problem does this solve?

The openab binary added MultibotMentions to the AllowUsers enum in #464, but the Helm chart's validation block in charts/openab/templates/configmap.yaml was not updated. Any user who follows docs/discord.md § Multi-Bot Setup and sets allowUserMessages: "multibot-mentions" is blocked by a template fail before the pod ever starts:

Error: UPGRADE FAILED: execution error at (openab/templates/configmap.yaml:48:8):
agents.kiro.discord.allowUserMessages must be one of: involved, mentions — got: multibot-mentions

Fixes #471
Refs #464

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491365150664560881/1495429672920416357

At a Glance

user values.yaml                         chart template (configmap.yaml)
allowUserMessages: "multibot-mentions"   has $val (list "involved" "mentions")
          │                                       │
          ▼                                       ▼
   ┌────────────┐                         ┌─────────────────────────────┐
   │ helm       │ ──── template fail ───▶ │ "must be one of: involved,  │
   │ upgrade    │                         │  mentions — got: ...-mentions"│
   └────────────┘                         └─────────────────────────────┘
                                                  ❌ blocks valid config
Binary side (same config):
   ┌────────────┐   ┌──────────────────────┐   ┌──────────────────────┐
   │ config.rs  │──▶│ AllowUsers::          │──▶│ works correctly      │
   │ deserialize│   │ MultibotMentions     │   │ (#464 discord.rs /   │
   └────────────┘   └──────────────────────┘   │  slack.rs branches)  │
                                                └──────────────────────┘

Prior Art & Industry Research

  • OpenClaw — ACP bind layer, no Helm chart with allow-mode validation. No analogue.
  • Hermes Agent — skills-based, no persistent service config validation. No analogue.
  • openab itselfallowBotMessages already validates with has ... (list "off" "mentions" "all") and has corresponding unit tests. This PR aligns allowUserMessages with that existing pattern.

Proposed Solution

Add "multibot-mentions" to the has allow-list for both Discord and Slack, plus update the documentation comment and charts/openab/values.yaml user-facing docs. Mirror the existing allowBotMessages test pattern for allowUserMessages.

-    {{- if not (has $cfg.discord.allowUserMessages (list "involved" "mentions")) }}
-    {{- fail (printf "agents.%s.discord.allowUserMessages must be one of: involved, mentions — got: %s" $name $cfg.discord.allowUserMessages) }}
+    {{- if not (has $cfg.discord.allowUserMessages (list "involved" "mentions" "multibot-mentions")) }}
+    {{- fail (printf "agents.%s.discord.allowUserMessages must be one of: involved, mentions, multibot-mentions — got: %s" $name $cfg.discord.allowUserMessages) }}

Same one-line change at the Slack block. Added 4 new helm-unittest cases covering involved / mentions / multibot-mentions renders plus the invalid-value rejection (the test file previously only covered allowBotMessagesallowUserMessages had no test coverage).

Why this approach

  • Symmetric with the existing allowBotMessages pattern — same validation style, same test style, no architectural change.
  • Smallest surface that fixes the bug — no refactor, no new abstraction, pure catch-up.
  • Keeps the helpful error message — users who typo the value still get a useful must be one of: ... message, now accurate.

Alternatives Considered

Alternative Rejected because
Drop the chart validation entirely and rely on binary-side deserializer errors Loses the template-time feedback (fails after pod start rather than before); inconsistent with existing allowBotMessages check.
Use a regex instead of has ... list Less readable; the existing pattern is a list; matching existing style is simpler.
Only accept hyphen form ("multibot-mentions") and leave underscore ("multibot_mentions") as binary-only Chart is the user-facing surface; accepting only hyphen form matches docs/discord.md and keeps errors clear. Rust deserializer still accepts both internally.

Validation

  • helm template openab ./charts/openab --set agents.kiro.discord.allowUserMessages=multibot-mentions renders allow_user_messages = "multibot-mentions" successfully.
  • helm template ... --set allowUserMessages=bogus still fails with the updated error message listing all three valid values.
  • helm template ... --set allowUserMessages=mentions still renders the pre-existing behaviour (backward-compatible).
  • helm unittest charts/openab → 18 passed, 0 failed (including 4 new test cases for allowUserMessages).
$ helm unittest charts/openab

### Chart [ openab ] charts/openab

 PASS  adapter enablement gating	charts/openab/tests/adapter-enablement_test.yaml
 PASS  allowBotMessages & trustedBotIds	charts/openab/tests/configmap_test.yaml

Charts:      1 passed, 1 total
Test Suites: 2 passed, 2 total
Tests:       18 passed, 18 total

The openab binary added the `MultibotMentions` variant to `AllowUsers`
in openabdev#464, but the Helm chart's validation block was not updated. Any
user who follows docs/discord.md § Multi-Bot Setup and sets
`allowUserMessages: "multibot-mentions"` is blocked by a template fail
before the pod ever starts.

Fix:
- Add `"multibot-mentions"` to the `has ... (list ...)` check for both
  Discord and Slack in `charts/openab/templates/configmap.yaml`.
- Update the trailing documentation comment describing the mode.
- Document the new option (and its semantics) in `charts/openab/values.yaml`.
- Add helm-unittest cases covering renders + invalid-value rejection for
  `allowUserMessages` (the test file previously only covered
  `allowBotMessages`).

Verified:

    $ helm template openab ./charts/openab \
        --set agents.kiro.discord.allowUserMessages=multibot-mentions \
        ... | grep allow_user_messages
        allow_user_messages = "multibot-mentions"

    $ helm unittest charts/openab
    Charts:      1 passed, 1 total
    Test Suites: 2 passed, 2 total
    Tests:       18 passed, 18 total

Fixes openabdev#471
Refs openabdev#464

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@brettchien brettchien requested a review from thepagent as a code owner April 19, 2026 14:17
@thepagent
Copy link
Copy Markdown
Collaborator

taking a look

chaodu-agent and others added 2 commits April 19, 2026 14:33
- Error messages now include '(use hyphen form, not underscore)' to
  help users who copy from Rust config (which accepts both forms)
- Add Slack-side allowUserMessages test for multibot-mentions

Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@chaodu-agent chaodu-agent left a comment

Choose a reason for hiding this comment

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

LGTM ✅

Reviewed the full diff across 3 commits:

  • Core fix: multibot-mentions added to allow-list for both Discord and Slack validation
  • Error messages now hint about hyphen form to prevent underscore copy-paste footgun
  • Test coverage: 5 new cases (Discord × 4 + Slack × 1), all 19 pass
  • values.yaml docs updated with clear mode descriptions

CI green, no nits remaining. Ship it 🚀

@thepagent thepagent merged commit b3e1f92 into openabdev:main Apr 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(helm): chart rejects "multibot-mentions" for allowUserMessages despite binary supporting it (#464)

3 participants