From 0bbd8f081e77579bbfd7a809ca15f284e9235bd6 Mon Sep 17 00:00:00 2001 From: SiYue-ZO <2835601846@qq.com> Date: Tue, 9 Jun 2026 10:48:17 +0800 Subject: [PATCH 1/3] fix: add DmScope field to SessionConfig to persist dm_scope setting The frontend sends dm_scope as part of the session config, but the backend SessionConfig struct lacked the corresponding field. Go's encoding/json silently discards unknown fields, so the value was lost on every PATCH request. Additionally, MarshalJSON only emitted the session block when Dimensions or IdentityLinks were set, so even a stored dm_scope would not appear in GET responses. - Add DmScope string field with json tag 'dm_scope' to SessionConfig - Update MarshalJSON condition to include session when DmScope is set --- pkg/config/config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index c71217e5b..f3cbf43ad 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -257,7 +257,7 @@ func (c *Config) MarshalJSON() ([]byte, error) { Alias: (*Alias)(c), } - if len(c.Session.Dimensions) > 0 || len(c.Session.IdentityLinks) > 0 { + if len(c.Session.Dimensions) > 0 || len(c.Session.IdentityLinks) > 0 || c.Session.DmScope != "" { sessionCfg := c.Session aux.Session = &sessionCfg } @@ -349,6 +349,7 @@ type DispatchSelector struct { type SessionConfig struct { Dimensions []string `json:"dimensions,omitempty"` IdentityLinks map[string][]string `json:"identity_links,omitempty"` + DmScope string `json:"dm_scope,omitempty"` } // RoutingConfig controls the intelligent model routing feature. From 921d753cc0b6595faae4a64117ce94d490b3ba00 Mon Sep 17 00:00:00 2001 From: SiYue-ZO <2835601846@qq.com> Date: Tue, 9 Jun 2026 11:03:33 +0800 Subject: [PATCH 2/3] fix: wire dm_scope into runtime session isolation dimensions The dm_scope field was stored in config but never translated into the dimensions array that the routing layer actually consumes. This meant changing the session isolation scope in the UI had no effect at runtime. Add ApplyDmScope() to SessionConfig which maps the user-facing dm_scope values (per-channel-peer, per-channel, per-peer, global) to the corresponding dimension arrays. Call it in LoadConfig post-processing and in both the PATCH and PUT API handlers. Includes table-driven tests covering all dm_scope values and the precedence rule (explicit dimensions > derived from dm_scope). --- pkg/config/config.go | 22 +++++++++++++++ pkg/config/config_test.go | 59 +++++++++++++++++++++++++++++++++++++++ web/backend/api/config.go | 2 ++ 3 files changed, 83 insertions(+) diff --git a/pkg/config/config.go b/pkg/config/config.go index f3cbf43ad..20cdd74c2 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -352,6 +352,26 @@ type SessionConfig struct { DmScope string `json:"dm_scope,omitempty"` } +// ApplyDmScope translates the user-facing dm_scope value into the internal +// dimensions array that the routing layer consumes. It is a no-op when +// DmScope is empty or when Dimensions is already set (explicit Dimensions +// take precedence over the derived value). +func (s *SessionConfig) ApplyDmScope() { + if s.DmScope == "" || len(s.Dimensions) > 0 { + return + } + switch s.DmScope { + case "per-channel-peer": + s.Dimensions = []string{"chat", "sender"} + case "per-channel": + s.Dimensions = []string{"chat"} + case "per-peer": + s.Dimensions = []string{"sender"} + case "global": + s.Dimensions = nil + } +} + // RoutingConfig controls the intelligent model routing feature. // When enabled, each incoming message is scored against structural features // (message length, code blocks, tool call history, conversation depth, attachments). @@ -1477,6 +1497,8 @@ func LoadConfig(path string) (*Config, error) { cfg.Agents.Defaults.Workspace = filepath.Join(homePath, pkg.WorkspaceName) } + cfg.Session.ApplyDmScope() + return cfg, nil } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index ea90fafe4..12cd70734 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1728,6 +1728,65 @@ func TestDefaultConfig_SessionDimensions(t *testing.T) { } } +func TestSessionConfig_ApplyDmScope(t *testing.T) { + tests := []struct { + name string + dmScope string + dimensions []string + want []string + }{ + { + name: "per-channel-peer", + dmScope: "per-channel-peer", + want: []string{"chat", "sender"}, + }, + { + name: "per-channel", + dmScope: "per-channel", + want: []string{"chat"}, + }, + { + name: "per-peer", + dmScope: "per-peer", + want: []string{"sender"}, + }, + { + name: "global", + dmScope: "global", + want: nil, + }, + { + name: "explicit dimensions take precedence", + dmScope: "per-channel-peer", + dimensions: []string{"sender"}, + want: []string{"sender"}, + }, + { + name: "empty dm_scope is no-op", + dmScope: "", + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &SessionConfig{ + DmScope: tt.dmScope, + Dimensions: tt.dimensions, + } + s.ApplyDmScope() + if len(s.Dimensions) != len(tt.want) { + t.Fatalf("Dimensions = %v, want %v", s.Dimensions, tt.want) + } + for i, v := range tt.want { + if s.Dimensions[i] != v { + t.Errorf("Dimensions[%d] = %q, want %q", i, s.Dimensions[i], v) + } + } + }) + } +} + func TestDefaultConfig_WorkspacePath_Default(t *testing.T) { t.Setenv("PICOCLAW_HOME", "") diff --git a/web/backend/api/config.go b/web/backend/api/config.go index b37958040..50d9b0293 100644 --- a/web/backend/api/config.go +++ b/web/backend/api/config.go @@ -76,6 +76,7 @@ func (h *Handler) handleUpdateConfig(w http.ResponseWriter, r *http.Request) { http.Error(w, fmt.Sprintf("Invalid JSON: %v", err), http.StatusBadRequest) return } + cfg.Session.ApplyDmScope() if execAllowRemoteOmitted(body) { cfg.Tools.Exec.AllowRemote = config.DefaultConfig().Tools.Exec.AllowRemote } @@ -181,6 +182,7 @@ func (h *Handler) handlePatchConfig(w http.ResponseWriter, r *http.Request) { http.Error(w, fmt.Sprintf("Merged config is invalid: %v", err), http.StatusBadRequest) return } + newCfg.Session.ApplyDmScope() // Restore security fields (tokens/keys) from the loaded config before validation, // because private fields are lost during JSON round-trip. From ef002d9a5d5e39456004040dcf61fdbe54706535 Mon Sep 17 00:00:00 2001 From: SiYue-ZO <2835601846@qq.com> Date: Thu, 11 Jun 2026 16:00:18 +0800 Subject: [PATCH 3/3] fix: ensure dm_scope and dimensions stay in sync across all config paths The reviewer identified two bugs in the original PR: 1. PATCH /api/config leaves session.dimensions stale: LoadConfig() derives dimensions from the old dm_scope, and the merge carries those stale dimensions forward. ApplyDmScope() then exits early because dimensions is already populated, causing a mismatch between dm_scope (new) and dimensions (old). 2. Legacy/default configs omit dm_scope in GET response: configs with explicit dimensions but no dm_scope (including DefaultConfig) return no dm_scope field, causing the frontend to fall back to its default ('per-channel-peer'), which may not match the actual dimensions. Fix: - Add DeriveDmScope() to reverse-map known dimensions arrays to dm_scope when dm_scope is empty. - Call it in LoadConfig(), PUT handler, PATCH handler, and ResetToDefaults() for consistent normalization. - In PATCH handler, clear stale dimensions from the merge result when the patch contains session.dm_scope but not session.dimensions, allowing ApplyDmScope() to re-derive from the new scope. - Add comprehensive unit tests for DeriveDmScope() and scope transition scenarios. --- pkg/config/config.go | 24 +++++++++ pkg/config/config_test.go | 101 ++++++++++++++++++++++++++++++++++++++ web/backend/api/config.go | 16 ++++++ 3 files changed, 141 insertions(+) diff --git a/pkg/config/config.go b/pkg/config/config.go index 20cdd74c2..655e39aa3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -7,6 +7,7 @@ import ( "math/rand" "os" "path/filepath" + "slices" "strconv" "strings" "sync/atomic" @@ -372,6 +373,26 @@ func (s *SessionConfig) ApplyDmScope() { } } +// DeriveDmScope sets DmScope based on Dimensions when DmScope is empty. +// This handles legacy/fresh configs that only have explicit Dimensions +// without a corresponding DmScope value, ensuring the API response always +// includes a dm_scope that matches the actual runtime dimensions. +func (s *SessionConfig) DeriveDmScope() { + if s.DmScope != "" || len(s.Dimensions) == 0 { + return + } + switch { + case slices.Equal(s.Dimensions, []string{"chat", "sender"}): + s.DmScope = "per-channel-peer" + case slices.Equal(s.Dimensions, []string{"chat"}): + s.DmScope = "per-channel" + case slices.Equal(s.Dimensions, []string{"sender"}): + s.DmScope = "per-peer" + } + // Dimensions not matching any known scope mapping (custom array) + // is fine — DmScope stays empty and the UI can handle it. +} + // RoutingConfig controls the intelligent model routing feature. // When enabled, each incoming message is scored against structural features // (message length, code blocks, tool call history, conversation depth, attachments). @@ -1498,6 +1519,7 @@ func LoadConfig(path string) (*Config, error) { } cfg.Session.ApplyDmScope() + cfg.Session.DeriveDmScope() return cfg, nil } @@ -1720,6 +1742,8 @@ func ResetToDefaults(configPath string) error { return fmt.Errorf("backup before reset: %w", err) } cfg := DefaultConfig() + cfg.Session.ApplyDmScope() + cfg.Session.DeriveDmScope() if err := cfg.SecurityCopyFrom(configPath); err != nil { logger.WarnF("could not preserve security config", map[string]any{"error": err}) } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 12cd70734..52f8f4782 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1787,6 +1787,107 @@ func TestSessionConfig_ApplyDmScope(t *testing.T) { } } +func TestSessionConfig_DeriveDmScope(t *testing.T) { + tests := []struct { + name string + dimensions []string + dmScope string + wantScope string + }{ + { + name: "per-channel-peer from dimensions", + dimensions: []string{"chat", "sender"}, + wantScope: "per-channel-peer", + }, + { + name: "per-channel from dimensions", + dimensions: []string{"chat"}, + wantScope: "per-channel", + }, + { + name: "per-peer from dimensions", + dimensions: []string{"sender"}, + wantScope: "per-peer", + }, + { + name: "custom dimensions does not set scope", + dimensions: []string{"chat", "extra"}, + wantScope: "", + }, + { + name: "empty dimensions does not set scope", + dimensions: nil, + wantScope: "", + }, + { + name: "existing dm_scope is not overwritten", + dimensions: []string{"chat", "sender"}, + dmScope: "per-channel", + wantScope: "per-channel", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &SessionConfig{ + DmScope: tt.dmScope, + Dimensions: tt.dimensions, + } + s.DeriveDmScope() + if s.DmScope != tt.wantScope { + t.Errorf("DmScope = %q, want %q", s.DmScope, tt.wantScope) + } + }) + } +} + +func TestSessionConfig_ApplyDmScope_ClearsStaleDimensions(t *testing.T) { + // Simulates the PATCH handler scenario: dm_scope changed but stale + // dimensions remain from the old scope. After clearing dimensions, + // ApplyDmScope should re-derive from the new dm_scope. + tests := []struct { + name string + dmScope string + want []string + }{ + { + name: "per-channel-peer to per-channel", + dmScope: "per-channel", + want: []string{"chat"}, + }, + { + name: "per-channel-peer to per-peer", + dmScope: "per-peer", + want: []string{"sender"}, + }, + { + name: "per-channel-peer to global", + dmScope: "global", + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &SessionConfig{ + DmScope: tt.dmScope, + Dimensions: []string{"chat", "sender"}, // stale from per-channel-peer + } + // Simulate what the PATCH handler does: clear dimensions when dm_scope changes + s.Dimensions = nil + s.ApplyDmScope() + if len(s.Dimensions) != len(tt.want) { + t.Fatalf("Dimensions = %v, want %v", s.Dimensions, tt.want) + } + for i, v := range tt.want { + if s.Dimensions[i] != v { + t.Errorf("Dimensions[%d] = %q, want %q", i, s.Dimensions[i], v) + } + } + }) + } +} + func TestDefaultConfig_WorkspacePath_Default(t *testing.T) { t.Setenv("PICOCLAW_HOME", "") diff --git a/web/backend/api/config.go b/web/backend/api/config.go index 50d9b0293..f567fe524 100644 --- a/web/backend/api/config.go +++ b/web/backend/api/config.go @@ -77,6 +77,7 @@ func (h *Handler) handleUpdateConfig(w http.ResponseWriter, r *http.Request) { return } cfg.Session.ApplyDmScope() + cfg.Session.DeriveDmScope() if execAllowRemoteOmitted(body) { cfg.Tools.Exec.AllowRemote = config.DefaultConfig().Tools.Exec.AllowRemote } @@ -165,6 +166,20 @@ func (h *Handler) handlePatchConfig(w http.ResponseWriter, r *http.Request) { // Recursively merge patch into base mergeMap(base, patch) + + // When the patch updates dm_scope, the old derived dimensions from the + // base must be cleared so that ApplyDmScope() can re-derive them from + // the new dm_scope value. Otherwise the stale dimensions survive the + // merge and ApplyDmScope() exits early due to its precedence guard. + if sess, ok := base["session"].(map[string]any); ok { + if patchSess, patchHasSession := patch["session"].(map[string]any); patchHasSession { + if _, hasDmScope := patchSess["dm_scope"]; hasDmScope { + if _, hasDimsInPatch := patchSess["dimensions"]; !hasDimsInPatch { + delete(sess, "dimensions") + } + } + } + } if err = normalizeChannelArrayFields(base); err != nil { http.Error(w, fmt.Sprintf("Invalid channel array field: %v", err), http.StatusBadRequest) return @@ -183,6 +198,7 @@ func (h *Handler) handlePatchConfig(w http.ResponseWriter, r *http.Request) { return } newCfg.Session.ApplyDmScope() + newCfg.Session.DeriveDmScope() // Restore security fields (tokens/keys) from the loaded config before validation, // because private fields are lost during JSON round-trip.