Skip to content

Commit 783f8c3

Browse files
committed
fix: discord client bugs
1 parent f14ae24 commit 783f8c3

File tree

5 files changed

+90
-6
lines changed

5 files changed

+90
-6
lines changed

.agents/AGENTS.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,8 @@ log:
211211
212212
## Code Patterns
213213
214+
## Code Patterns
215+
214216
### Go Conventions
215217
216218
- **Store-based resources**: All entities managed via admin API, persisted to `data/store.json`
@@ -235,6 +237,14 @@ log:
235237
- **Session middleware**: `SessionEnsure` prevents overwriting existing sessions (protects ContextGuard summaries). `SessionStateSeed` injects empty outputKey values so flow agents don't fail on template vars
236238
- **Flow wrapAgent pattern**: Same agent can appear in multiple flow steps — `wrapAgent()` creates uniquely-named delegate agents to satisfy ADK's single-parent constraint
237239

240+
### Design Philosophy
241+
242+
**DRY, KISS, elegant, and decoupled does not mean over-engineered.** When two options exist — one simple and one cleverly abstracted — prefer the simple one. Specifically:
243+
244+
- Don't avoid a straightforward API call (with built-in cache + HTTP fallback) just to use a lower-level cache-only lookup that may silently miss. `s.Channel()` is the right call in discordgo; `s.State.Channel()` is an unnecessary micro-optimisation that trades reliability for nothing.
245+
- Don't redesign function signatures (variadic, extra parameters, new types) when the problem is already solved at the call site with one extra line.
246+
- Complexity is only justified when it removes real duplication or real coupling — not hypothetical ones.
247+
238248
### Frontend Conventions (admin-ui)
239249

240250
- **Vue 3 Composition API**: `<script setup>` everywhere, no Options API

.agents/DISCORD_CLIENT.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ discordgo.IntentGuildMessageReactions |
5353
discordgo.IntentDirectMessageReactions
5454
```
5555

56+
`IntentGuilds` is intentionally omitted. Thread/channel metadata is resolved on demand via `s.Channel()` (HTTP fallback) rather than relying on the state cache being fully populated.
57+
5658
**Message Content Intent** is privileged. Must be toggled ON in Developer Portal → Bot → Privileged Gateway Intents. Without it, `m.Content` is always empty.
5759

5860
## Data Model
@@ -186,13 +188,15 @@ Uses `msgutil.SplitMessage(text, msgutil.DiscordMaxMessageLength)` with Discord'
186188
## Access control
187189

188190
```go
189-
func (c *Client) isAllowed(userID, channelID string) bool
191+
func (c *Client) isAllowed(userID string, channelIDs ...string) bool
190192
```
191193

192194
- Both lists empty → open access
193-
- Either list populated → OR allowlist: user allowed if in `AllowedUsers` OR channel in `AllowedChannels`
195+
- Either list populated → OR allowlist: user allowed if in `AllowedUsers` OR any of the `channelIDs` in `AllowedChannels`
194196
- Neither matches → denied (silently dropped, logged at debug level)
195197

198+
**Thread IDs are different from their parent channel ID.** When a message arrives inside a thread, `m.ChannelID` is the thread's own ID — not the parent channel. `onMessageCreate` resolves the parent ID via `s.Channel()` (state cache + HTTP fallback) and passes both to `isAllowed`, so that a thread created from an allowed channel is itself allowed.
199+
196200
## Wiring (main.go)
197201

198202
Same pattern as Telegram and Slack:

.agents/RELEASE_NOTES_TEMPLATE.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# Release Notes Template
2+
3+
## Format
4+
5+
```markdown
6+
## vX.Y.Z
7+
8+
### 🐛 Bug fixes
9+
10+
- **Short title describing the fix** — What was broken, why it was broken, what was changed to fix it.
11+
12+
### ✨ Features
13+
14+
- **Short title describing the feature** — What it does and why it's useful.
15+
16+
### 🔧 Improvements
17+
18+
- **Short title describing the improvement** — What changed and what's better now.
19+
20+
### 💥 Breaking changes
21+
22+
- **Short title** — What changed, what breaks, and how to migrate.
23+
```
24+
25+
## Rules
26+
27+
- **English only**, plain language — no jargon, no internal references
28+
- **Only include sections that have entries** — omit empty sections entirely
29+
- **One bullet per change** — if a fix touches multiple files, it's still one bullet
30+
- **Bullet structure**: `**Title** — cause, effect, fix` (for bugs) or `**Title** — what it does` (for features)
31+
- **No emojis inside bullets** — only on section headings
32+
- **Version follows semver**: `MAJOR.MINOR.PATCH`
33+
- `PATCH` — bug fixes only
34+
- `MINOR` — new features, backwards compatible
35+
- `MAJOR` — breaking changes
36+
37+
## Section icons
38+
39+
| Section | Icon |
40+
|---------|------|
41+
| Bug fixes | 🐛 |
42+
| Features ||
43+
| Improvements | 🔧 |
44+
| Breaking changes | 💥 |
45+
46+
## Example
47+
48+
```markdown
49+
## v0.15.1
50+
51+
### 🐛 Bug fixes
52+
53+
- **Discord: thread messages ignored when `allowedChannels` is set** — Messages inside a thread were silently dropped even when the thread was created from an allowed channel. Discord threads have their own ID, different from the parent channel, so they were failing the permission check. The bot now also checks the parent channel ID when deciding whether to allow a message.
54+
55+
- **Discord/Slack: `threadHistoryLimit` could not be updated from the Admin UI** — When saving a client from the edit form, the value was sent as a string instead of a number. The backend rejected the request with a deserialization error. The form now correctly sends integer and number fields as numbers.
56+
```

frontend/admin-ui/src/views/clients/ClientDialog.vue

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,11 @@ async function save() {
464464
if (Array.isArray(val) && val.length) {
465465
typeCfg[key] = val
466466
}
467+
} else if (propSchema.type === 'integer' || propSchema.type === 'number') {
468+
const n = Number(val)
469+
if (!isNaN(n) && val !== '' && val !== null && val !== undefined) {
470+
typeCfg[key] = propSchema.type === 'integer' ? Math.trunc(n) : n
471+
}
467472
} else if (val?.toString().trim()) {
468473
typeCfg[key] = val.toString().trim()
469474
}

server/clients/discord/bot.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,11 @@ func (c *Client) onMessageCreate(s *discordgo.Session, m *discordgo.MessageCreat
119119
return
120120
}
121121

122-
if !c.isAllowed(m.Author.ID, m.ChannelID) {
122+
parentID := ""
123+
if ch, err := s.Channel(m.ChannelID); err == nil && isDiscordThread(ch.Type) {
124+
parentID = ch.ParentID
125+
}
126+
if !c.isAllowed(m.Author.ID, m.ChannelID, parentID) {
123127
c.logger.Debug("Unauthorized Discord message", "user", m.Author.ID, "channel", m.ChannelID)
124128
return
125129
}
@@ -861,16 +865,21 @@ func (c *Client) setAuthHeader(req *http.Request) {
861865
}
862866
}
863867

864-
func (c *Client) isAllowed(userID, channelID string) bool {
868+
// isAllowed checks whether a user/channel combination is permitted.
869+
// channelIDs accepts the direct channel and optionally the parent channel ID
870+
// (for threads, whose IDs differ from the parent channel they belong to).
871+
func (c *Client) isAllowed(userID string, channelIDs ...string) bool {
865872
cfg := c.clientDef.Config.Discord
866873
if len(cfg.AllowedUsers) == 0 && len(cfg.AllowedChannels) == 0 {
867874
return true
868875
}
869876
if len(cfg.AllowedUsers) > 0 && slices.Contains(cfg.AllowedUsers, userID) {
870877
return true
871878
}
872-
if len(cfg.AllowedChannels) > 0 && slices.Contains(cfg.AllowedChannels, channelID) {
873-
return true
879+
for _, id := range channelIDs {
880+
if id != "" && slices.Contains(cfg.AllowedChannels, id) {
881+
return true
882+
}
874883
}
875884
return false
876885
}

0 commit comments

Comments
 (0)