feat(display): refactor to closure Register pattern, add config IPC handlers

Register(wailsApp) returns factory closure for WithService. OnStartup registers config query/task handlers synchronously. Handles window.QueryConfig, systray.QueryConfig, menu.QueryConfig. Remove ServiceName() — auto-derived as 'display' by WithService. Remove ServiceStartup/Startup — replaced by OnStartup (Startable interface). Remove actions.go — replaced by sub-package message types. Add EventTrayClick and EventTrayMenuItemClick event types. Co-Authored-By: Virgil <virgil@lethean.io>
2026-03-13 13:30:26 +00:00 · 2026-03-13 13:30:26 +00:00 · d29cf62e6f
commit d29cf62e6f
parent 3afa0c84d3
4 changed files with 151 additions and 106 deletions
--- a/pkg/display/actions.go
+++ b/pkg/display/actions.go
@ -1,9 +0,0 @@
-// pkg/display/actions.go
-package display
-
-import "forge.lthn.ai/core/gui/pkg/window"
-
-// ActionOpenWindow is an IPC message type requesting a new window.
-type ActionOpenWindow struct {
-	window.Window
-}
--- a/pkg/display/display.go
+++ b/pkg/display/display.go
@ -17,68 +17,104 @@ import (
 type Options struct{}

 // Service manages windowing, dialogs, and other visual elements.
-// It composes window.Manager, systray.Manager, and menu.Manager.
+// It orchestrates sub-services (window, systray, menu) via IPC and bridges
+// IPC actions to WebSocket events for TypeScript apps.
 type Service struct {
 	*core.ServiceRuntime[Options]
-	app      App
-	config   Options
-	windows  *window.Manager
-	tray     *systray.Manager
-	menus    *menu.Manager
-	notifier *notifications.NotificationService
-	events   *WSEventManager
-}
-
-// newDisplayService contains the common logic for initializing a Service struct.
-func newDisplayService() (*Service, error) {
-	return &Service{}, nil
+	wailsApp   *application.App
+	app        App
+	config     Options
+	configData map[string]map[string]any
+	windows    *window.Manager
+	tray       *systray.Manager
+	menus      *menu.Manager
+	notifier   *notifications.NotificationService
+	events     *WSEventManager
 }

 // New is the constructor for the display service.
 func New() (*Service, error) {
-	s, err := newDisplayService()
-	if err != nil {
-		return nil, err
+	return &Service{
+		configData: map[string]map[string]any{
+			"window":  {},
+			"systray": {},
+			"menu":    {},
+		},
+	}, nil
+}
+
+// Register creates a factory closure that captures the Wails app.
+// Pass nil for testing without a Wails runtime.
+func Register(wailsApp *application.App) func(*core.Core) (any, error) {
+	return func(c *core.Core) (any, error) {
+		s, err := New()
+		if err != nil {
+			return nil, err
+		}
+		s.ServiceRuntime = core.NewServiceRuntime[Options](c, Options{})
+		s.wailsApp = wailsApp
+		return s, nil
 	}
-	return s, nil
 }

-// Register creates and registers a new display service with the given Core instance.
-func Register(c *core.Core) (any, error) {
-	s, err := New()
-	if err != nil {
-		return nil, err
+// OnStartup loads config and registers IPC handlers synchronously.
+// CRITICAL: config handlers MUST be registered before returning —
+// sub-services depend on them during their own OnStartup.
+func (s *Service) OnStartup(ctx context.Context) error {
+	s.loadConfig()
+
+	// Register config query/task handlers — available NOW for sub-services
+	s.Core().RegisterQuery(s.handleConfigQuery)
+	s.Core().RegisterTask(s.handleConfigTask)
+
+	// Initialise Wails wrappers if app is available (nil in tests)
+	if s.wailsApp != nil {
+		s.app = newWailsApp(s.wailsApp)
+		s.events = NewWSEventManager(newWailsEventSource(s.wailsApp))
+		s.events.SetupWindowEventListeners()
 	}
-	s.ServiceRuntime = core.NewServiceRuntime[Options](c, Options{})
-	return s, nil
+
+	return nil
 }

-// ServiceName returns the canonical name for this service.
-func (s *Service) ServiceName() string {
-	return "forge.lthn.ai/core/gui/display"
+func (s *Service) loadConfig() {
+	// In-memory defaults. go-config integration is deferred work.
+	if s.configData == nil {
+		s.configData = map[string]map[string]any{
+			"window":  {},
+			"systray": {},
+			"menu":    {},
+		}
+	}
 }

-// ServiceStartup is called by Wails when the app starts.
-func (s *Service) ServiceStartup(ctx context.Context, options application.ServiceOptions) error {
-	return s.Startup(ctx)
+func (s *Service) handleConfigQuery(c *core.Core, q core.Query) (any, bool, error) {
+	switch q.(type) {
+	case window.QueryConfig:
+		return s.configData["window"], true, nil
+	case systray.QueryConfig:
+		return s.configData["systray"], true, nil
+	case menu.QueryConfig:
+		return s.configData["menu"], true, nil
+	default:
+		return nil, false, nil
+	}
 }

-// Startup initialises the display service and sets up sub-managers.
-func (s *Service) Startup(ctx context.Context) error {
-	wailsApp := application.Get()
-	s.app = newWailsApp(wailsApp)
-
-	// Create sub-manager platform adapters
-	s.windows = window.NewManager(window.NewWailsPlatform(wailsApp))
-	s.tray = systray.NewManager(systray.NewWailsPlatform(wailsApp))
-	s.menus = menu.NewManager(menu.NewWailsPlatform(wailsApp))
-
-	s.events = NewWSEventManager(newWailsEventSource(wailsApp))
-	s.events.SetupWindowEventListeners()
-	s.app.Logger().Info("Display service started")
-	s.buildMenu()
-	s.setupTray()
-	return s.OpenWindow()
+func (s *Service) handleConfigTask(c *core.Core, t core.Task) (any, bool, error) {
+	switch t := t.(type) {
+	case window.TaskSaveConfig:
+		s.configData["window"] = t.Value
+		return nil, true, nil
+	case systray.TaskSaveConfig:
+		s.configData["systray"] = t.Value
+		return nil, true, nil
+	case menu.TaskSaveConfig:
+		s.configData["menu"] = t.Value
+		return nil, true, nil
+	default:
+		return nil, false, nil
+	}
 }

 // --- Window Management (delegates to window.Manager) ---
--- a/pkg/display/display_test.go
+++ b/pkg/display/display_test.go
@ -1,6 +1,7 @@
 package display

 import (
+	"context"
 	"testing"

 	"forge.lthn.ai/core/go/pkg/core"
@ -161,13 +162,6 @@ func (mi *displayMockMenuItem) OnClick(fn func()) menu.PlatformMenuItem

 // --- Test helpers ---

-// newTestCore creates a new core instance for testing.
-func newTestCore(t *testing.T) *core.Core {
-	coreInstance, err := core.New()
-	require.NoError(t, err)
-	return coreInstance
-}
-
 // newServiceWithMocks creates a Service with mock sub-managers for testing.
 // Uses a temp directory for state/layout persistence to avoid loading real saved state.
 func newServiceWithMocks(t *testing.T) (*Service, *mockApp, *displayMockWindowPlatform) {
@ -185,6 +179,19 @@ func newServiceWithMocks(t *testing.T) (*Service, *mockApp, *displayMockWindowPl
 	return service, mock, wp
 }

+// newTestDisplayService creates a display service registered with Core for IPC testing.
+func newTestDisplayService(t *testing.T) (*Service, *core.Core) {
+	t.Helper()
+	c, err := core.New(
+		core.WithService(Register(nil)),
+		core.WithServiceLock(),
+	)
+	require.NoError(t, err)
+	require.NoError(t, c.ServiceStartup(context.Background(), nil))
+	svc := core.MustServiceFor[*Service](c, "display")
+	return svc, c
+}
+
 // --- Tests ---

 func TestNew(t *testing.T) {
@ -203,31 +210,56 @@ func TestNew(t *testing.T) {
 	})
 }

-func TestRegister(t *testing.T) {
-	t.Run("registers with core successfully", func(t *testing.T) {
-		coreInstance := newTestCore(t)
-		service, err := Register(coreInstance)
-		require.NoError(t, err)
-		assert.NotNil(t, service, "Register() should return a non-nil service instance")
-	})
+func TestRegisterClosure_Good(t *testing.T) {
+	factory := Register(nil) // nil wailsApp for testing
+	assert.NotNil(t, factory)

-	t.Run("returns Service type", func(t *testing.T) {
-		coreInstance := newTestCore(t)
-		service, err := Register(coreInstance)
-		require.NoError(t, err)
+	c, err := core.New(
+		core.WithService(factory),
+		core.WithServiceLock(),
+	)
+	require.NoError(t, err)
+	require.NoError(t, c.ServiceStartup(context.Background(), nil))

-		displayService, ok := service.(*Service)
-		assert.True(t, ok, "Register() should return *Service type")
-		assert.NotNil(t, displayService.ServiceRuntime, "ServiceRuntime should be initialized")
-	})
+	svc := core.MustServiceFor[*Service](c, "display")
+	assert.NotNil(t, svc)
 }

-func TestServiceName(t *testing.T) {
-	service, err := New()
-	require.NoError(t, err)
+func TestConfigQuery_Good(t *testing.T) {
+	svc, c := newTestDisplayService(t)

-	name := service.ServiceName()
-	assert.Equal(t, "forge.lthn.ai/core/gui/display", name)
+	// Set window config
+	svc.configData["window"] = map[string]any{
+		"default_width": 1024,
+	}
+
+	result, handled, err := c.QUERY(window.QueryConfig{})
+	require.NoError(t, err)
+	assert.True(t, handled)
+	cfg := result.(map[string]any)
+	assert.Equal(t, 1024, cfg["default_width"])
+}
+
+func TestConfigQuery_Bad(t *testing.T) {
+	// No display service — window config query returns handled=false
+	c, err := core.New(core.WithServiceLock())
+	require.NoError(t, err)
+	_, handled, _ := c.QUERY(window.QueryConfig{})
+	assert.False(t, handled)
+}
+
+func TestConfigTask_Good(t *testing.T) {
+	_, c := newTestDisplayService(t)
+
+	newCfg := map[string]any{"default_width": 800}
+	_, handled, err := c.PERFORM(window.TaskSaveConfig{Value: newCfg})
+	require.NoError(t, err)
+	assert.True(t, handled)
+
+	// Verify config was saved
+	result, _, _ := c.QUERY(window.QueryConfig{})
+	cfg := result.(map[string]any)
+	assert.Equal(t, 800, cfg["default_width"])
 }

 func TestOpenWindow_Good(t *testing.T) {
@ -461,8 +493,8 @@ func TestShowEnvironmentDialog_Good(t *testing.T) {

 func TestBuildMenu_Good(t *testing.T) {
 	service, _, _ := newServiceWithMocks(t)
-	coreInstance := newTestCore(t)
-	service.ServiceRuntime = core.NewServiceRuntime[Options](coreInstance, Options{})
+	c, _ := core.New()
+	service.ServiceRuntime = core.NewServiceRuntime[Options](c, Options{})

 	// buildMenu should not panic with mock platforms
 	assert.NotPanics(t, func() {
@ -472,8 +504,8 @@ func TestBuildMenu_Good(t *testing.T) {

 func TestSetupTray_Good(t *testing.T) {
 	service, _, _ := newServiceWithMocks(t)
-	coreInstance := newTestCore(t)
-	service.ServiceRuntime = core.NewServiceRuntime[Options](coreInstance, Options{})
+	c, _ := core.New()
+	service.ServiceRuntime = core.NewServiceRuntime[Options](c, Options{})

 	// setupTray should not panic with mock platforms
 	assert.NotPanics(t, func() {
@ -499,8 +531,8 @@ func TestHandleNewWorkspace_Good(t *testing.T) {

 func TestHandleListWorkspaces_Good(t *testing.T) {
 	service, _, _ := newServiceWithMocks(t)
-	coreInstance := newTestCore(t)
-	service.ServiceRuntime = core.NewServiceRuntime[Options](coreInstance, Options{})
+	c, _ := core.New()
+	service.ServiceRuntime = core.NewServiceRuntime[Options](c, Options{})

 	// handleListWorkspaces should not panic when workspace service is not available
 	assert.NotPanics(t, func() {
@ -565,19 +597,3 @@ func TestGetSavedWindowStates_Good(t *testing.T) {
 	states := service.GetSavedWindowStates()
 	assert.NotNil(t, states)
 }
-
-func TestActionOpenWindow_Good(t *testing.T) {
-	action := ActionOpenWindow{
-		Window: window.Window{
-			Name:   "test",
-			Title:  "Test Window",
-			Width:  800,
-			Height: 600,
-		},
-	}
-
-	assert.Equal(t, "test", action.Name)
-	assert.Equal(t, "Test Window", action.Title)
-	assert.Equal(t, 800, action.Width)
-	assert.Equal(t, 600, action.Height)
-}
--- a/pkg/display/events.go
+++ b/pkg/display/events.go
@ -21,8 +21,10 @@ const (
 	EventWindowResize EventType = "window.resize"
 	EventWindowClose  EventType = "window.close"
 	EventWindowCreate EventType = "window.create"
-	EventThemeChange  EventType = "theme.change"
-	EventScreenChange EventType = "screen.change"
+	EventThemeChange         EventType = "theme.change"
+	EventScreenChange        EventType = "screen.change"
+	EventTrayClick           EventType = "tray.click"
+	EventTrayMenuItemClick   EventType = "tray.menuitem.click"
 )

 // Event represents a display event sent to subscribers.