From f13b37309dc988b124be48a591b4fffd829ad424 Mon Sep 17 00:00:00 2001 From: stpwin Date: Fri, 17 Apr 2026 11:03:41 +0700 Subject: [PATCH] fix(config): preserve explicit mention_only=false in GroupTriggerConfig GroupTriggerConfig.MentionOnly was tagged `json:"mention_only,omitempty"`. Because Go's bool zero value is false, `omitempty` silently drops the field whenever a user sets it to false. On the next LoadConfig, the missing field is filled from defaultChannels() (defaults.go:483-484), which hardcodes `{"mention_only": true}` for channels like LINE and Matrix. LoadConfig then calls `defer SaveConfig(...)` on any migration branch (config.go:1042, 1083, 1122), writing `true` back to disk. Net effect: users who explicitly opt out of mention-only mode see their setting silently revert to true after any upgrade or version migration. DiscordConfig.MentionOnly already uses the plain `json:"mention_only"` tag for this reason; this change aligns GroupTriggerConfig with the same convention. Add a regression test that fails pre-patch (JSON round-trips to `{}`) and passes post-patch (explicit false survives marshal/unmarshal). --- pkg/config/config.go | 7 ++- pkg/config/group_trigger_mention_only_test.go | 43 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 pkg/config/group_trigger_mention_only_test.go diff --git a/pkg/config/config.go b/pkg/config/config.go index 5bc96fb12..d73af9fb1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -307,7 +307,12 @@ func (d *AgentDefaults) GetModelName() string { // GroupTriggerConfig controls when the bot responds in group chats. type GroupTriggerConfig struct { - MentionOnly bool `json:"mention_only,omitempty"` + // MentionOnly must always serialize so an explicit `false` survives the + // LoadConfig → SaveConfig roundtrip in migration paths (config.go:1042, + // 1083, 1122). With `,omitempty`, Go would omit the bool zero value and + // defaultChannels() would re-inject the hardcoded `true` default + // (defaults.go:483-484), silently reverting the user's choice. + MentionOnly bool `json:"mention_only"` Prefixes []string `json:"prefixes,omitempty"` } diff --git a/pkg/config/group_trigger_mention_only_test.go b/pkg/config/group_trigger_mention_only_test.go new file mode 100644 index 000000000..eb633a4c6 --- /dev/null +++ b/pkg/config/group_trigger_mention_only_test.go @@ -0,0 +1,43 @@ +package config + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestGroupTriggerConfig_MentionOnly_False_Roundtrips documents the regression +// where an explicit `mention_only: false` is silently lost on a JSON roundtrip +// because the struct tag `json:"mention_only,omitempty"` causes Go to omit the +// field whenever its value equals the bool zero value (false). +// +// Chain of damage in production: +// 1. User edits config.json and sets "mention_only": false. +// 2. Gateway loads config → LoadConfig triggers defer SaveConfig on any +// migration path (config.go:1042/1083/1122). +// 3. SaveConfig re-marshals the struct. `,omitempty` drops the false field. +// 4. The next load merges with defaultChannels() (defaults.go:483-484), +// which hardcodes `{"mention_only": true}` for the "line" channel. +// 5. User's explicit false silently becomes true. +// +// The fix is to remove `,omitempty` from the tag so false always serializes, +// matching the pattern already used by DiscordConfig.MentionOnly (no omitempty). +func TestGroupTriggerConfig_MentionOnly_False_Roundtrips(t *testing.T) { + original := GroupTriggerConfig{MentionOnly: false} + + data, err := json.Marshal(original) + require.NoError(t, err) + + // The bug: with `,omitempty`, the false field is omitted entirely, so the + // JSON becomes "{}" and the user's explicit choice is indistinguishable + // from "never set". A caller that fills in defaults for missing fields + // will then replace false with true. + assert.Contains(t, string(data), `"mention_only":false`, + "explicit false must serialize — otherwise defaults logic overwrites it; see defaults.go:483-484") + + var decoded GroupTriggerConfig + require.NoError(t, json.Unmarshal(data, &decoded)) + assert.False(t, decoded.MentionOnly, "mention_only should survive roundtrip as false") +}