From bb5122580a5a8b53bfdd81bf75cad817dac1cac7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 31 Mar 2026 15:08:50 +0100 Subject: [PATCH] feat(environment,contextmenu): expand with new queries and lifecycle Environment: - QueryFocusFollowsMouse IPC query - 9 new tests (accent colour, file manager, focus follows mouse) Context Menu: - TaskUpdate (remove-then-add semantics) - TaskDestroy (remove + release) - QueryGetAll - OnShutdown cleanup - 12 new tests All 17 packages build and test clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/contextmenu/messages.go | 16 +++ pkg/contextmenu/service.go | 53 +++++++++ pkg/contextmenu/service_test.go | 185 ++++++++++++++++++++++++++++++++ pkg/environment/messages.go | 3 + pkg/environment/platform.go | 1 + pkg/environment/service.go | 2 + pkg/environment/service_test.go | 98 +++++++++++++++-- 7 files changed, 352 insertions(+), 6 deletions(-) diff --git a/pkg/contextmenu/messages.go b/pkg/contextmenu/messages.go index 32acb5e..32e7037 100644 --- a/pkg/contextmenu/messages.go +++ b/pkg/contextmenu/messages.go @@ -12,6 +12,10 @@ type QueryGet struct { // QueryList returns all registered context menus. Result: map[string]ContextMenuDef type QueryList struct{} +// QueryGetAll returns all registered context menus. Equivalent to QueryList. +// Result: map[string]ContextMenuDef +type QueryGetAll struct{} + // TaskAdd registers a named context menu. Replaces if already exists. type TaskAdd struct { Name string `json:"name"` @@ -23,6 +27,18 @@ type TaskRemove struct { Name string `json:"name"` } +// TaskUpdate replaces an existing context menu's definition. Error: ErrorMenuNotFound if missing. +type TaskUpdate struct { + Name string `json:"name"` + Menu ContextMenuDef `json:"menu"` +} + +// TaskDestroy removes a context menu and releases all associated resources. +// Error: ErrorMenuNotFound if missing. +type TaskDestroy struct { + Name string `json:"name"` +} + // ActionItemClicked is broadcast when a context menu item is clicked. type ActionItemClicked struct { MenuName string `json:"menuName"` diff --git a/pkg/contextmenu/service.go b/pkg/contextmenu/service.go index f6d97e4..0386fc3 100644 --- a/pkg/contextmenu/service.go +++ b/pkg/contextmenu/service.go @@ -22,6 +22,15 @@ func (s *Service) OnStartup(ctx context.Context) error { return nil } +func (s *Service) OnShutdown(ctx context.Context) error { + // Destroy all registered menus on shutdown to release platform resources + for name := range s.registeredMenus { + _ = s.platform.Remove(name) + } + s.registeredMenus = make(map[string]ContextMenuDef) + return nil +} + func (s *Service) HandleIPCEvents(c *core.Core, msg core.Message) error { return nil } @@ -34,6 +43,8 @@ func (s *Service) handleQuery(c *core.Core, q core.Query) (any, bool, error) { return s.queryGet(q), true, nil case QueryList: return s.queryList(), true, nil + case QueryGetAll: + return s.queryList(), true, nil default: return nil, false, nil } @@ -63,6 +74,10 @@ func (s *Service) handleTask(c *core.Core, t core.Task) (any, bool, error) { return nil, true, s.taskAdd(t) case TaskRemove: return nil, true, s.taskRemove(t) + case TaskUpdate: + return nil, true, s.taskUpdate(t) + case TaskDestroy: + return nil, true, s.taskDestroy(t) default: return nil, false, nil } @@ -104,3 +119,41 @@ func (s *Service) taskRemove(t TaskRemove) error { delete(s.registeredMenus, t.Name) return nil } + +func (s *Service) taskUpdate(t TaskUpdate) error { + if _, exists := s.registeredMenus[t.Name]; !exists { + return ErrorMenuNotFound + } + + // Re-register with updated definition — remove then add + if err := s.platform.Remove(t.Name); err != nil { + return coreerr.E("contextmenu.taskUpdate", "platform remove failed", err) + } + + err := s.platform.Add(t.Name, t.Menu, func(menuName, actionID, data string) { + _ = s.Core().ACTION(ActionItemClicked{ + MenuName: menuName, + ActionID: actionID, + Data: data, + }) + }) + if err != nil { + return coreerr.E("contextmenu.taskUpdate", "platform add failed", err) + } + + s.registeredMenus[t.Name] = t.Menu + return nil +} + +func (s *Service) taskDestroy(t TaskDestroy) error { + if _, exists := s.registeredMenus[t.Name]; !exists { + return ErrorMenuNotFound + } + + if err := s.platform.Remove(t.Name); err != nil { + return coreerr.E("contextmenu.taskDestroy", "platform remove failed", err) + } + + delete(s.registeredMenus, t.Name) + return nil +} diff --git a/pkg/contextmenu/service_test.go b/pkg/contextmenu/service_test.go index edab171..abd1e2b 100644 --- a/pkg/contextmenu/service_test.go +++ b/pkg/contextmenu/service_test.go @@ -297,3 +297,188 @@ func TestQueryList_Bad_NoService(t *testing.T) { _, handled, _ := c.QUERY(QueryList{}) assert.False(t, handled) } + +// --- TaskUpdate --- + +func TestTaskUpdate_Good(t *testing.T) { + // Update replaces items on an existing menu + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, _, _ = c.PERFORM(TaskAdd{ + Name: "edit-menu", + Menu: ContextMenuDef{Name: "edit-menu", Items: []MenuItemDef{{Label: "Cut", ActionID: "cut"}}}, + }) + + _, handled, err := c.PERFORM(TaskUpdate{ + Name: "edit-menu", + Menu: ContextMenuDef{Name: "edit-menu", Items: []MenuItemDef{ + {Label: "Cut", ActionID: "cut"}, + {Label: "Copy", ActionID: "copy"}, + }}, + }) + require.NoError(t, err) + assert.True(t, handled) + + result, _, _ := c.QUERY(QueryGet{Name: "edit-menu"}) + def := result.(*ContextMenuDef) + assert.Len(t, def.Items, 2) + assert.Equal(t, "Copy", def.Items[1].Label) +} + +func TestTaskUpdate_Bad_NotFound(t *testing.T) { + // Update on a non-existent menu returns ErrorMenuNotFound + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, handled, err := c.PERFORM(TaskUpdate{ + Name: "ghost", + Menu: ContextMenuDef{Name: "ghost"}, + }) + assert.True(t, handled) + assert.ErrorIs(t, err, ErrorMenuNotFound) +} + +func TestTaskUpdate_Ugly_PlatformRemoveError(t *testing.T) { + // Platform Remove fails mid-update — error is propagated + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, _, _ = c.PERFORM(TaskAdd{ + Name: "tricky", + Menu: ContextMenuDef{Name: "tricky"}, + }) + + mp.mu.Lock() + mp.removeErr = ErrorMenuNotFound // reuse sentinel as a platform-level error + mp.mu.Unlock() + + _, handled, err := c.PERFORM(TaskUpdate{ + Name: "tricky", + Menu: ContextMenuDef{Name: "tricky", Items: []MenuItemDef{{Label: "X", ActionID: "x"}}}, + }) + assert.True(t, handled) + assert.Error(t, err) +} + +// --- TaskDestroy --- + +func TestTaskDestroy_Good(t *testing.T) { + // Destroy removes the menu and releases platform resources + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, _, _ = c.PERFORM(TaskAdd{Name: "doomed", Menu: ContextMenuDef{Name: "doomed"}}) + + _, handled, err := c.PERFORM(TaskDestroy{Name: "doomed"}) + require.NoError(t, err) + assert.True(t, handled) + + result, _, _ := c.QUERY(QueryGet{Name: "doomed"}) + assert.Nil(t, result) + + _, ok := mp.Get("doomed") + assert.False(t, ok) +} + +func TestTaskDestroy_Bad_NotFound(t *testing.T) { + // Destroy on a non-existent menu returns ErrorMenuNotFound + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, handled, err := c.PERFORM(TaskDestroy{Name: "nonexistent"}) + assert.True(t, handled) + assert.ErrorIs(t, err, ErrorMenuNotFound) +} + +func TestTaskDestroy_Ugly_PlatformError(t *testing.T) { + // Platform Remove fails — error is propagated but service remains consistent + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, _, _ = c.PERFORM(TaskAdd{Name: "frail", Menu: ContextMenuDef{Name: "frail"}}) + + mp.mu.Lock() + mp.removeErr = ErrorMenuNotFound + mp.mu.Unlock() + + _, handled, err := c.PERFORM(TaskDestroy{Name: "frail"}) + assert.True(t, handled) + assert.Error(t, err) +} + +// --- QueryGetAll --- + +func TestQueryGetAll_Good(t *testing.T) { + // QueryGetAll returns all registered menus (equivalent to QueryList) + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, _, _ = c.PERFORM(TaskAdd{Name: "x", Menu: ContextMenuDef{Name: "x"}}) + _, _, _ = c.PERFORM(TaskAdd{Name: "y", Menu: ContextMenuDef{Name: "y"}}) + + result, handled, err := c.QUERY(QueryGetAll{}) + require.NoError(t, err) + assert.True(t, handled) + all := result.(map[string]ContextMenuDef) + assert.Len(t, all, 2) + assert.Contains(t, all, "x") + assert.Contains(t, all, "y") +} + +func TestQueryGetAll_Bad_Empty(t *testing.T) { + // QueryGetAll on an empty registry returns an empty map + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + result, handled, err := c.QUERY(QueryGetAll{}) + require.NoError(t, err) + assert.True(t, handled) + all := result.(map[string]ContextMenuDef) + assert.Len(t, all, 0) +} + +func TestQueryGetAll_Ugly_NoService(t *testing.T) { + // No contextmenu service — query is unhandled + c, _ := core.New(core.WithServiceLock()) + _, handled, _ := c.QUERY(QueryGetAll{}) + assert.False(t, handled) +} + +// --- OnShutdown --- + +func TestOnShutdown_Good_CleansUpMenus(t *testing.T) { + // OnShutdown removes all registered menus from the platform + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, _, _ = c.PERFORM(TaskAdd{Name: "alpha", Menu: ContextMenuDef{Name: "alpha"}}) + _, _, _ = c.PERFORM(TaskAdd{Name: "beta", Menu: ContextMenuDef{Name: "beta"}}) + + require.NoError(t, c.ServiceShutdown(t.Context())) + + assert.Len(t, mp.menus, 0) +} + +func TestOnShutdown_Bad_NothingRegistered(t *testing.T) { + // OnShutdown with no menus — no-op, no error + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + assert.NoError(t, c.ServiceShutdown(t.Context())) +} + +func TestOnShutdown_Ugly_PlatformRemoveErrors(t *testing.T) { + // Platform Remove errors during shutdown are silently swallowed + mp := newMockPlatform() + _, c := newTestContextMenuService(t, mp) + + _, _, _ = c.PERFORM(TaskAdd{Name: "stubborn", Menu: ContextMenuDef{Name: "stubborn"}}) + + mp.mu.Lock() + mp.removeErr = ErrorMenuNotFound + mp.mu.Unlock() + + // Shutdown must not return an error even if platform Remove fails + assert.NoError(t, c.ServiceShutdown(t.Context())) +} diff --git a/pkg/environment/messages.go b/pkg/environment/messages.go index 8813dc1..f021e0c 100644 --- a/pkg/environment/messages.go +++ b/pkg/environment/messages.go @@ -16,6 +16,9 @@ type TaskOpenFileManager struct { Select bool `json:"select"` } +// QueryFocusFollowsMouse returns whether the platform uses focus-follows-mouse. Result: bool +type QueryFocusFollowsMouse struct{} + // ActionThemeChanged is broadcast when the system theme changes. type ActionThemeChanged struct { IsDark bool `json:"isDark"` diff --git a/pkg/environment/platform.go b/pkg/environment/platform.go index 3e403f9..b513dab 100644 --- a/pkg/environment/platform.go +++ b/pkg/environment/platform.go @@ -7,6 +7,7 @@ type Platform interface { Info() EnvironmentInfo AccentColour() string OpenFileManager(path string, selectFile bool) error + HasFocusFollowsMouse() bool OnThemeChange(handler func(isDark bool)) func() // returns cancel func } diff --git a/pkg/environment/service.go b/pkg/environment/service.go index 8f4d18a..6cc4192 100644 --- a/pkg/environment/service.go +++ b/pkg/environment/service.go @@ -61,6 +61,8 @@ func (s *Service) handleQuery(c *core.Core, q core.Query) (any, bool, error) { return s.platform.Info(), true, nil case QueryAccentColour: return s.platform.AccentColour(), true, nil + case QueryFocusFollowsMouse: + return s.platform.HasFocusFollowsMouse(), true, nil default: return nil, false, nil } diff --git a/pkg/environment/service_test.go b/pkg/environment/service_test.go index 76ec531..45fcc15 100644 --- a/pkg/environment/service_test.go +++ b/pkg/environment/service_test.go @@ -6,23 +6,26 @@ import ( "sync" "testing" + coreerr "forge.lthn.ai/core/go-log" "forge.lthn.ai/core/go/pkg/core" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) type mockPlatform struct { - isDark bool - info EnvironmentInfo - accentColour string - openFMErr error - themeHandler func(isDark bool) - mu sync.Mutex + isDark bool + info EnvironmentInfo + accentColour string + openFMErr error + focusFollowsMouse bool + themeHandler func(isDark bool) + mu sync.Mutex } func (m *mockPlatform) IsDarkMode() bool { return m.isDark } func (m *mockPlatform) Info() EnvironmentInfo { return m.info } func (m *mockPlatform) AccentColour() string { return m.accentColour } +func (m *mockPlatform) HasFocusFollowsMouse() bool { return m.focusFollowsMouse } func (m *mockPlatform) OpenFileManager(path string, selectFile bool) error { return m.openFMErr } @@ -131,3 +134,86 @@ func TestThemeChange_ActionBroadcast_Good(t *testing.T) { require.NotNil(t, r) assert.False(t, r.IsDark) } + +// --- GetAccentColor --- + +func TestQueryAccentColour_Bad_Empty(t *testing.T) { + // accent colour := "" — still returns handled with empty string + mock := &mockPlatform{ + isDark: false, + accentColour: "", + info: EnvironmentInfo{OS: "linux", Arch: "amd64"}, + } + c, err := core.New(core.WithService(Register(mock)), core.WithServiceLock()) + require.NoError(t, err) + require.NoError(t, c.ServiceStartup(t.Context(), nil)) + + result, handled, err := c.QUERY(QueryAccentColour{}) + require.NoError(t, err) + assert.True(t, handled) + assert.Equal(t, "", result) +} + +func TestQueryAccentColour_Ugly_NoService(t *testing.T) { + // No environment service — query is unhandled + c, _ := core.New(core.WithServiceLock()) + _, handled, _ := c.QUERY(QueryAccentColour{}) + assert.False(t, handled) +} + +// --- OpenFileManager --- + +func TestTaskOpenFileManager_Bad_Error(t *testing.T) { + // platform returns an error on open + openErr := coreerr.E("test", "file manager unavailable", nil) + mock := &mockPlatform{openFMErr: openErr} + c, err := core.New(core.WithService(Register(mock)), core.WithServiceLock()) + require.NoError(t, err) + require.NoError(t, c.ServiceStartup(t.Context(), nil)) + + _, handled, err := c.PERFORM(TaskOpenFileManager{Path: "/missing", Select: false}) + assert.True(t, handled) + assert.ErrorIs(t, err, openErr) +} + +func TestTaskOpenFileManager_Ugly_NoService(t *testing.T) { + // No environment service — task is unhandled + c, _ := core.New(core.WithServiceLock()) + _, handled, _ := c.PERFORM(TaskOpenFileManager{Path: "/tmp", Select: false}) + assert.False(t, handled) +} + +// --- HasFocusFollowsMouse --- + +func TestQueryFocusFollowsMouse_Good_True(t *testing.T) { + // platform reports focus-follows-mouse enabled (Linux/X11 sloppy focus) + mock := &mockPlatform{focusFollowsMouse: true} + c, err := core.New(core.WithService(Register(mock)), core.WithServiceLock()) + require.NoError(t, err) + require.NoError(t, c.ServiceStartup(t.Context(), nil)) + + result, handled, err := c.QUERY(QueryFocusFollowsMouse{}) + require.NoError(t, err) + assert.True(t, handled) + assert.Equal(t, true, result) +} + +func TestQueryFocusFollowsMouse_Bad_False(t *testing.T) { + // platform reports focus-follows-mouse disabled (Windows/macOS default) + mock := &mockPlatform{focusFollowsMouse: false} + c, err := core.New(core.WithService(Register(mock)), core.WithServiceLock()) + require.NoError(t, err) + require.NoError(t, c.ServiceStartup(t.Context(), nil)) + + result, handled, err := c.QUERY(QueryFocusFollowsMouse{}) + require.NoError(t, err) + assert.True(t, handled) + assert.Equal(t, false, result) +} + +func TestQueryFocusFollowsMouse_Ugly_NoService(t *testing.T) { + // No environment service — query is unhandled + c, _ := core.New(core.WithServiceLock()) + _, handled, _ := c.QUERY(QueryFocusFollowsMouse{}) + assert.False(t, handled) +}