From f66220634a4289c19f4b6e5398a7d0fbb3ba047a Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Sat, 10 Oct 2026 13:57:16 +0200 Subject: [PATCH 1/3] refactor(up): inject the navigation menu instead of importing cmd/formatter pkg/compose imported cmd/formatter for the interactive keyboard menu of a foreground up, so the library depended on the CLI. Define a NavigationMenu interface (and a factory) in pkg/compose, satisfied by formatter.LogKeyboard, and have the CLI inject it with the new WithNavigationMenu option. The terminal code stays in cmd/formatter. Without the option no menu is displayed. formatter.Feature moves to api.Feature so that both sides share the type, and a depguard rule now forbids pkg/ and internal/ from importing cmd/. Signed-off-by: Nicolas De Loof --- .golangci.yml | 11 +++++++ cmd/formatter/shortcut.go | 10 ++---- cmd/main.go | 4 +++ docs/sdk.md | 1 + pkg/api/feature.go | 26 ++++++++++++++++ pkg/compose/compose.go | 2 ++ pkg/compose/navigation_menu.go | 57 ++++++++++++++++++++++++++++++++++ pkg/compose/up.go | 23 ++++++++++---- pkg/compose/up_test.go | 19 ++++++++++++ 9 files changed, 139 insertions(+), 14 deletions(-) create mode 100644 pkg/api/feature.go create mode 100644 pkg/compose/navigation_menu.go diff --git a/.golangci.yml b/.golangci.yml index cce5d7e520a..09bf18ecda1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -55,6 +55,17 @@ linters: desc: Use "gotest.tools/v3/assert" instead - pkg: github.com/stretchr/testify/suite desc: Do not use + library-not-cli: + # pkg/ is the SDK and internal/ its support code: neither may depend + # on the CLI. A CLI-only component the library needs goes behind an + # interface that cmd/ implements and injects (e.g. WithNavigationMenu). + files: + - "**/pkg/**" + - "**/internal/**" + - "!**/pkg/e2e/**" + deny: + - pkg: github.com/docker/compose/v5/cmd + desc: the library must not depend on the CLI, inject an implementation instead. forbidigo: analyze-types: true forbid: diff --git a/cmd/formatter/shortcut.go b/cmd/formatter/shortcut.go index 054b076aa0f..e931437c746 100644 --- a/cmd/formatter/shortcut.go +++ b/cmd/formatter/shortcut.go @@ -73,13 +73,7 @@ func (ke *KeyboardError) error() string { type KeyboardWatch struct { Watching bool - Watcher Feature -} - -// Feature is an compose feature that can be started/stopped by a menu command -type Feature interface { - Start(context.Context) error - Stop() error + Watcher api.Feature } type KEYBOARD_LOG_LEVEL int @@ -356,7 +350,7 @@ func (lk *LogKeyboard) HandleKeyEvents(ctx context.Context, event keyboard.KeyEv } } -func (lk *LogKeyboard) EnableWatch(enabled bool, watcher Feature) { +func (lk *LogKeyboard) EnableWatch(enabled bool, watcher api.Feature) { lk.Watch = &KeyboardWatch{ Watching: enabled, Watcher: watcher, diff --git a/cmd/main.go b/cmd/main.go index 7d3fab4f5ed..7534f0741cf 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -29,6 +29,7 @@ import ( "github.com/docker/compose/v5/cmd/cmdtrace" "github.com/docker/compose/v5/cmd/compatibility" commands "github.com/docker/compose/v5/cmd/compose" + "github.com/docker/compose/v5/cmd/formatter" "github.com/docker/compose/v5/cmd/prompt" "github.com/docker/compose/v5/internal" "github.com/docker/compose/v5/pkg/compose" @@ -40,6 +41,9 @@ func pluginMain() { backendOptions := &commands.BackendOptions{ Options: []compose.Option{ compose.WithPrompt(prompt.NewPrompt(cli.In(), cli.Out()).Confirm), + compose.WithNavigationMenu(func(isDockerDesktopActive, isLogsViewEnabled bool, signals chan<- os.Signal) compose.NavigationMenu { + return formatter.NewKeyboardManager(isDockerDesktopActive, isLogsViewEnabled, signals) + }), }, } diff --git a/docs/sdk.md b/docs/sdk.md index c0c0c39f3ea..97780dc4db4 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -105,6 +105,7 @@ options allow you to configure I/O streams, concurrency limits, dry-run mode, an - `WithStreams(out, err, in)` - Set all I/O streams at once - `WithMaxConcurrency(int)` - Limit the number of concurrent operations against the Docker API - `WithPrompt(Prompt)` - Customize user confirmation behavior (use `AlwaysOkPrompt()` for non-interactive mode) +- `WithNavigationMenu(NavigationMenuFactory)` - Provide the interactive keyboard menu of a foreground `up` (no menu is displayed without it) - `WithDryRun` - Run operations in dry-run mode without actually applying changes - `WithContextInfo(api.ContextInfo)` - Set custom Docker context information - `WithProxyConfig(map[string]string)` - Configure HTTP proxy settings for builds diff --git a/pkg/api/feature.go b/pkg/api/feature.go new file mode 100644 index 00000000000..d7e965059b9 --- /dev/null +++ b/pkg/api/feature.go @@ -0,0 +1,26 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package api + +import "context" + +// Feature is a compose feature that can be started and stopped on demand, +// e.g. by a command of the interactive menu of `compose up`. +type Feature interface { + Start(context.Context) error + Stop() error +} diff --git a/pkg/compose/compose.go b/pkg/compose/compose.go index 73710bedc0d..32754867ddb 100644 --- a/pkg/compose/compose.go +++ b/pkg/compose/compose.go @@ -281,6 +281,8 @@ type composeService struct { dockerCli command.Cli // prompt is used to interact with user and confirm actions prompt Prompt + // navigationMenu creates the interactive menu of a foreground `up`, if any + navigationMenu NavigationMenuFactory // eventBus collects tasks execution events events api.EventProcessor diff --git a/pkg/compose/navigation_menu.go b/pkg/compose/navigation_menu.go new file mode 100644 index 00000000000..c4efa5cca7d --- /dev/null +++ b/pkg/compose/navigation_menu.go @@ -0,0 +1,57 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "context" + "os" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/eiannone/keyboard" + + "github.com/docker/compose/v5/pkg/api" +) + +// NavigationMenu is the interactive keyboard menu of a foreground `compose up`. +// The terminal rendering is up to the implementation, which the CLI provides +// (see WithNavigationMenu): the library only drives it. +type NavigationMenu interface { + // Decorate returns a LogConsumer that keeps the menu displayed below the logs + Decorate(api.LogConsumer) api.LogConsumer + // EnableWatch makes the menu able to toggle watch, which is initially enabled or not + EnableWatch(enabled bool, watcher api.Feature) + // EnableDetach sets the function run when the user asks to detach + EnableDetach(detach func()) + // HandleKeyEvents reacts to a key typed by the user + HandleKeyEvents(ctx context.Context, event keyboard.KeyEvent, project *types.Project, options api.UpOptions) +} + +// NavigationMenuFactory creates the NavigationMenu of an `up` session. +// isDockerDesktopActive and isLogsViewEnabled tell which Docker Desktop +// integrations the menu can offer, and signals is the channel the menu notifies +// with an interruption when the user asks to stop the application. +type NavigationMenuFactory func(isDockerDesktopActive, isLogsViewEnabled bool, signals chan<- os.Signal) NavigationMenu + +// WithNavigationMenu configures the interactive menu displayed by a foreground +// `up` when api.StartOptions.NavigationMenu is set. Without it, no menu is +// displayed. +func WithNavigationMenu(factory NavigationMenuFactory) Option { + return func(s *composeService) error { + s.navigationMenu = factory + return nil + } +} diff --git a/pkg/compose/up.go b/pkg/compose/up.go index 0df95ad4187..ac5a23eb481 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -36,7 +36,6 @@ import ( "golang.org/x/sync/errgroup" "golang.org/x/sync/semaphore" - "github.com/docker/compose/v5/cmd/formatter" "github.com/docker/compose/v5/internal/desktop" "github.com/docker/compose/v5/internal/tracing" "github.com/docker/compose/v5/pkg/api" @@ -79,7 +78,7 @@ type upSession struct { // drain them before tearing the context down — see the monitor wrapper. logStreams sync.WaitGroup watcher *Watcher - menu *formatter.LogKeyboard + menu NavigationMenu globalCtx context.Context cancel context.CancelFunc // logOpenLimiter bounds concurrent log-attach opens from @@ -238,12 +237,18 @@ func (s *composeService) runInteractiveUp(ctx context.Context, project *types.Pr } // setupNavigationMenu initializes the interactive keyboard menu when enabled. -// It returns a nil menu when the menu is disabled, or when the keyboard can't -// be grabbed — then disabling the option. -func (s *composeService) setupNavigationMenu(ctx context.Context, options *api.UpOptions, signalChan chan os.Signal) (*formatter.LogKeyboard, <-chan keyboard.KeyEvent, error) { +// It returns a nil menu when the menu is disabled, when no menu implementation +// was configured (WithNavigationMenu), or when the keyboard can't be grabbed — +// then disabling the option. +func (s *composeService) setupNavigationMenu(ctx context.Context, options *api.UpOptions, signalChan chan os.Signal) (NavigationMenu, <-chan keyboard.KeyEvent, error) { if !options.Start.NavigationMenu { return nil, nil, nil } + if s.navigationMenu == nil { + logrus.Warn("no navigation menu implementation configured (see WithNavigationMenu), disabling the option") + options.Start.NavigationMenu = false + return nil, nil, nil + } kEvents, err := keyboard.GetKeys(100) if err != nil { logrus.Warnf("could not start menu, an error occurred while starting: %v", err) @@ -257,7 +262,13 @@ func (s *composeService) setupNavigationMenu(ctx context.Context, options *api.U } isLogsViewEnabled := s.isDesktopFeatureActive(ctx, desktop.FeatureLogsTab) tracing.KeyboardMetrics(ctx, options.Start.NavigationMenu, isDockerDesktopActive, isLogsViewEnabled) - return formatter.NewKeyboardManager(isDockerDesktopActive, isLogsViewEnabled, signalChan), kEvents, nil + menu := s.navigationMenu(isDockerDesktopActive, isLogsViewEnabled, signalChan) + if menu == nil { + _ = keyboard.Close() + options.Start.NavigationMenu = false + return nil, nil, nil + } + return menu, kEvents, nil } // appendErr records err for the final report, unless it is nothing more than diff --git a/pkg/compose/up_test.go b/pkg/compose/up_test.go index 19f293c30e5..d0ab677cfb0 100644 --- a/pkg/compose/up_test.go +++ b/pkg/compose/up_test.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "io" + "os" "strings" "testing" "time" @@ -232,3 +233,21 @@ func TestStreamContainerLogs_ConcurrencyIsBounded(t *testing.T) { assert.NilError(t, eg.Wait()) assert.Equal(t, tracker.Peak(), 1, "streamContainerLogs must share --parallel's budget for concurrent log-attach opens") } + +func TestSetupNavigationMenuWithoutImplementation(t *testing.T) { + s := &composeService{} + options := api.UpOptions{Start: api.StartOptions{NavigationMenu: true}} + + menu, events, err := s.setupNavigationMenu(t.Context(), &options, make(chan os.Signal, 1)) + + assert.NilError(t, err) + assert.Assert(t, menu == nil) + assert.Assert(t, events == nil, "the keyboard must not be grabbed when there is no menu to feed") + assert.Assert(t, !options.Start.NavigationMenu) +} + +func TestWithNavigationMenu(t *testing.T) { + s := &composeService{} + assert.NilError(t, WithNavigationMenu(func(bool, bool, chan<- os.Signal) NavigationMenu { return nil })(s)) + assert.Assert(t, s.navigationMenu != nil) +} From 51c8bbaa5f0c46745247f15bd707311508e4b440 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Sat, 10 Oct 2026 18:22:51 +0200 Subject: [PATCH 2/3] refactor(up): let the navigation menu own the keyboard pkg/compose still imported the keyboard library: it grabbed the terminal, forwarded raw key events to the menu and released the terminal on exit. The NavigationMenu now takes care of all of it through Open, Run and Close, so the library no longer knows about keys or the terminal, and the menu interface no longer exposes a third-party event type. As a side effect, key handling no longer shares a goroutine with the signal handling loop of `up`. Signed-off-by: Nicolas De Loof --- cmd/formatter/shortcut.go | 40 +++++++++++++++++- cmd/formatter/shortcut_test.go | 75 ++++++++++++++++++++++++++++++++++ pkg/compose/navigation_menu.go | 16 ++++++-- pkg/compose/up.go | 50 ++++++++++++----------- pkg/compose/up_test.go | 50 ++++++++++++++++++++++- 5 files changed, 199 insertions(+), 32 deletions(-) create mode 100644 cmd/formatter/shortcut_test.go diff --git a/cmd/formatter/shortcut.go b/cmd/formatter/shortcut.go index e931437c746..a098d3866e3 100644 --- a/cmd/formatter/shortcut.go +++ b/cmd/formatter/shortcut.go @@ -23,6 +23,7 @@ import ( "math" "os" "strings" + "sync" "syscall" "time" @@ -92,6 +93,9 @@ type LogKeyboard struct { IsLogsViewEnabled bool logLevel KEYBOARD_LOG_LEVEL signalChannel chan<- os.Signal + keys <-chan keyboard.KeyEvent + closeOnce sync.Once + closeErr error } func NewKeyboardManager(isDockerDesktopActive, isLogsViewEnabled bool, sc chan<- os.Signal) *LogKeyboard { @@ -304,7 +308,39 @@ func (lk *LogKeyboard) ToggleWatch(ctx context.Context, options api.UpOptions) { } } -func (lk *LogKeyboard) HandleKeyEvents(ctx context.Context, event keyboard.KeyEvent, project *types.Project, options api.UpOptions) { +// Open puts the terminal in raw mode; keys are buffered until Run reads them. +func (lk *LogKeyboard) Open() error { + keys, err := keyboard.GetKeys(100) + if err != nil { + return err + } + lk.keys = keys + return nil +} + +// Run handles the keys typed by the user until ctx is done. +func (lk *LogKeyboard) Run(ctx context.Context, project *types.Project, options api.UpOptions) { + if lk.keys == nil { // Open was not called, or failed + return + } + for { + select { + case <-ctx.Done(): + return + case event := <-lk.keys: + lk.handleKeyEvent(ctx, event, project, options) + } + } +} + +// Close restores the terminal. It can be called several times, from several +// goroutines (Ctrl+C is seen both as a key and as a signal). +func (lk *LogKeyboard) Close() error { + lk.closeOnce.Do(func() { lk.closeErr = keyboard.Close() }) + return lk.closeErr +} + +func (lk *LogKeyboard) handleKeyEvent(ctx context.Context, event keyboard.KeyEvent, project *types.Project, options api.UpOptions) { switch kRune := event.Rune; kRune { case 'd': lk.clearNavigationMenu() @@ -335,7 +371,7 @@ func (lk *LogKeyboard) HandleKeyEvents(ctx context.Context, event keyboard.KeyEv } switch key := event.Key; key { case keyboard.KeyCtrlC: - _ = keyboard.Close() + _ = lk.Close() lk.clearNavigationMenu() showCursor() diff --git a/cmd/formatter/shortcut_test.go b/cmd/formatter/shortcut_test.go new file mode 100644 index 00000000000..0f85be07445 --- /dev/null +++ b/cmd/formatter/shortcut_test.go @@ -0,0 +1,75 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package formatter + +import ( + "context" + "os" + "testing" + "time" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/eiannone/keyboard" + "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/api" +) + +func TestRunHandlesKeysUntilContextIsDone(t *testing.T) { + detached := make(chan struct{}) + keys := make(chan keyboard.KeyEvent, 1) + lk := NewKeyboardManager(false, false, make(chan os.Signal, 1)) + lk.keys = keys + lk.EnableDetach(func() { close(detached) }) + + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan struct{}) + go func() { + lk.Run(ctx, &types.Project{}, api.UpOptions{}) + close(done) + }() + + keys <- keyboard.KeyEvent{Rune: 'd'} + select { + case <-detached: + case <-time.After(5 * time.Second): + t.Fatal("the detach key was not handled") + } + + cancel() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("Run must return once its context is done") + } + assert.NilError(t, lk.Close(), "Close must be harmless when the keyboard was never opened") + assert.NilError(t, lk.Close(), "Close must be idempotent") +} + +func TestRunWithoutOpenReturnsImmediately(t *testing.T) { + lk := NewKeyboardManager(false, false, make(chan os.Signal, 1)) + done := make(chan struct{}) + go func() { + lk.Run(t.Context(), &types.Project{}, api.UpOptions{}) + close(done) + }() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("Run must not wait for keys that can't come") + } +} diff --git a/pkg/compose/navigation_menu.go b/pkg/compose/navigation_menu.go index c4efa5cca7d..1921be495b0 100644 --- a/pkg/compose/navigation_menu.go +++ b/pkg/compose/navigation_menu.go @@ -21,7 +21,6 @@ import ( "os" "github.com/compose-spec/compose-go/v2/types" - "github.com/eiannone/keyboard" "github.com/docker/compose/v5/pkg/api" ) @@ -36,11 +35,20 @@ type NavigationMenu interface { EnableWatch(enabled bool, watcher api.Feature) // EnableDetach sets the function run when the user asks to detach EnableDetach(detach func()) - // HandleKeyEvents reacts to a key typed by the user - HandleKeyEvents(ctx context.Context, event keyboard.KeyEvent, project *types.Project, options api.UpOptions) + // Open takes control of the keyboard. Keys typed from then on are kept until + // Run consumes them. An error means the menu can't be used (no terminal, …) + // and the session carries on without it. + Open() error + // Run reacts to the keys typed by the user until ctx is done. It is called + // once, after Open and after the menu has been fully configured. + Run(ctx context.Context, project *types.Project, options api.UpOptions) + // Close gives the keyboard back. It can be called several times, and also + // when Open failed. + Close() error } -// NavigationMenuFactory creates the NavigationMenu of an `up` session. +// NavigationMenuFactory creates the NavigationMenu of an `up` session. It must +// return a nil interface, not a typed nil pointer, when there is no menu. // isDockerDesktopActive and isLogsViewEnabled tell which Docker Desktop // integrations the menu can offer, and signals is the channel the menu notifies // with an interruption when the user asks to stop the application. diff --git a/pkg/compose/up.go b/pkg/compose/up.go index ac5a23eb481..67376e3934f 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -31,7 +31,6 @@ import ( "github.com/compose-spec/compose-go/v2/types" "github.com/containerd/errdefs" "github.com/docker/cli/cli" - "github.com/eiannone/keyboard" "github.com/sirupsen/logrus" "golang.org/x/sync/errgroup" "golang.org/x/sync/semaphore" @@ -104,12 +103,12 @@ func (s *composeService) runInteractiveUp(ctx context.Context, project *types.Pr defer signal.Stop(signalChan) logConsumer := options.Start.Attach - navigationMenu, kEvents, err := s.setupNavigationMenu(ctx, &options, signalChan) + navigationMenu, err := s.setupNavigationMenu(ctx, &options, signalChan) if err != nil { return err } if navigationMenu != nil { - defer keyboard.Close() //nolint:errcheck + defer navigationMenu.Close() //nolint:errcheck logConsumer = navigationMenu.Decorate(logConsumer) } @@ -144,8 +143,14 @@ func (s *composeService) runInteractiveUp(ctx context.Context, project *types.Pr } u.eg.Go(func() error { - return u.runEventLoop(ctx, kEvents) + return u.runEventLoop(ctx) }) + if navigationMenu != nil { + u.eg.Go(func() error { + navigationMenu.Run(globalCtx, project, options) + return nil + }) + } if options.Start.Watch && watcher != nil { if err := watcher.Start(globalCtx); err != nil { @@ -240,35 +245,32 @@ func (s *composeService) runInteractiveUp(ctx context.Context, project *types.Pr // It returns a nil menu when the menu is disabled, when no menu implementation // was configured (WithNavigationMenu), or when the keyboard can't be grabbed — // then disabling the option. -func (s *composeService) setupNavigationMenu(ctx context.Context, options *api.UpOptions, signalChan chan os.Signal) (NavigationMenu, <-chan keyboard.KeyEvent, error) { +func (s *composeService) setupNavigationMenu(ctx context.Context, options *api.UpOptions, signalChan chan os.Signal) (NavigationMenu, error) { if !options.Start.NavigationMenu { - return nil, nil, nil + return nil, nil } if s.navigationMenu == nil { logrus.Warn("no navigation menu implementation configured (see WithNavigationMenu), disabling the option") options.Start.NavigationMenu = false - return nil, nil, nil - } - kEvents, err := keyboard.GetKeys(100) - if err != nil { - logrus.Warnf("could not start menu, an error occurred while starting: %v", err) - options.Start.NavigationMenu = false - return nil, nil, nil + return nil, nil } isDockerDesktopActive, err := s.isDesktopIntegrationActive(ctx) if err != nil { - _ = keyboard.Close() - return nil, nil, err + return nil, err } isLogsViewEnabled := s.isDesktopFeatureActive(ctx, desktop.FeatureLogsTab) - tracing.KeyboardMetrics(ctx, options.Start.NavigationMenu, isDockerDesktopActive, isLogsViewEnabled) menu := s.navigationMenu(isDockerDesktopActive, isLogsViewEnabled, signalChan) if menu == nil { - _ = keyboard.Close() options.Start.NavigationMenu = false - return nil, nil, nil + return nil, nil } - return menu, kEvents, nil + if err := menu.Open(); err != nil { + logrus.Warnf("could not start menu, an error occurred while starting: %v", err) + options.Start.NavigationMenu = false + return nil, nil + } + tracing.KeyboardMetrics(ctx, options.Start.NavigationMenu, isDockerDesktopActive, isLogsViewEnabled) + return menu, nil } // appendErr records err for the final report, unless it is nothing more than @@ -290,10 +292,10 @@ func (u *upSession) appendErr(err error) { u.mu.Unlock() } -// runEventLoop reacts to cancellation, SIGINT/SIGTERM and keyboard input until +// runEventLoop reacts to cancellation and SIGINT/SIGTERM until // the application terminates: a first interruption triggers a graceful stop, // a second one kills the services. -func (u *upSession) runEventLoop(ctx context.Context, kEvents <-chan keyboard.KeyEvent) error { +func (u *upSession) runEventLoop(ctx context.Context) error { first := true gracefulTeardown := func() { first = false @@ -317,14 +319,14 @@ func (u *upSession) runEventLoop(ctx context.Context, kEvents <-chan keyboard.Ke } case <-u.signalChan: if first { - _ = keyboard.Close() + if u.menu != nil { + _ = u.menu.Close() + } gracefulTeardown() break } u.killApplication() return nil - case event := <-kEvents: - u.menu.HandleKeyEvents(u.globalCtx, event, u.project, u.options) } } } diff --git a/pkg/compose/up_test.go b/pkg/compose/up_test.go index d0ab677cfb0..243a6718459 100644 --- a/pkg/compose/up_test.go +++ b/pkg/compose/up_test.go @@ -26,6 +26,7 @@ import ( "testing" "time" + "github.com/compose-spec/compose-go/v2/types" containerType "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" "go.uber.org/mock/gomock" @@ -238,14 +239,59 @@ func TestSetupNavigationMenuWithoutImplementation(t *testing.T) { s := &composeService{} options := api.UpOptions{Start: api.StartOptions{NavigationMenu: true}} - menu, events, err := s.setupNavigationMenu(t.Context(), &options, make(chan os.Signal, 1)) + menu, err := s.setupNavigationMenu(t.Context(), &options, make(chan os.Signal, 1)) assert.NilError(t, err) assert.Assert(t, menu == nil) - assert.Assert(t, events == nil, "the keyboard must not be grabbed when there is no menu to feed") assert.Assert(t, !options.Start.NavigationMenu) } +// fakeMenu records how the session drives a NavigationMenu. +type fakeMenu struct { + openErr error + opened bool +} + +func (m *fakeMenu) Decorate(l api.LogConsumer) api.LogConsumer { return l } +func (m *fakeMenu) EnableWatch(bool, api.Feature) {} +func (m *fakeMenu) EnableDetach(func()) {} +func (m *fakeMenu) Run(context.Context, *types.Project, api.UpOptions) {} +func (m *fakeMenu) Close() error { return nil } +func (m *fakeMenu) Open() error { + m.opened = true + return m.openErr +} + +func TestSetupNavigationMenu(t *testing.T) { + tests := []struct { + name string + menu NavigationMenu + wantMenu bool + }{ + {name: "usable", menu: &fakeMenu{}, wantMenu: true}, + {name: "keyboard can't be grabbed", menu: &fakeMenu{openErr: errors.New("no tty")}}, + {name: "factory without menu"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, apiClient := newTestService(t, WithNavigationMenu(func(bool, bool, chan<- os.Signal) NavigationMenu { + return tt.menu + })) + apiClient.EXPECT().Info(gomock.Any(), gomock.Any()).Return(client.SystemInfoResult{}, nil).AnyTimes() + options := api.UpOptions{Start: api.StartOptions{NavigationMenu: true}} + + menu, err := svc.setupNavigationMenu(t.Context(), &options, make(chan os.Signal, 1)) + + assert.NilError(t, err) + assert.Equal(t, menu != nil, tt.wantMenu) + assert.Equal(t, options.Start.NavigationMenu, tt.wantMenu, "the option must be disabled when there is no usable menu") + if fm, ok := tt.menu.(*fakeMenu); ok { + assert.Assert(t, fm.opened, "the keyboard must be grabbed through the menu") + } + }) + } +} + func TestWithNavigationMenu(t *testing.T) { s := &composeService{} assert.NilError(t, WithNavigationMenu(func(bool, bool, chan<- os.Signal) NavigationMenu { return nil })(s)) From 682c463fdcc51b14340f23a7f19d60a2baaa5850 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Sat, 10 Oct 2026 18:37:52 +0200 Subject: [PATCH 3/3] fix(up): stop the menu loop when the keyboard channel is closed keyboard.Close closes the channel handed out by GetKeys. Run kept reading zero-value events from it until its context was done. Signed-off-by: Nicolas De Loof --- cmd/formatter/shortcut.go | 5 ++++- cmd/formatter/shortcut_test.go | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/cmd/formatter/shortcut.go b/cmd/formatter/shortcut.go index a098d3866e3..d6add09c269 100644 --- a/cmd/formatter/shortcut.go +++ b/cmd/formatter/shortcut.go @@ -327,7 +327,10 @@ func (lk *LogKeyboard) Run(ctx context.Context, project *types.Project, options select { case <-ctx.Done(): return - case event := <-lk.keys: + case event, ok := <-lk.keys: + if !ok { // the keyboard was closed + return + } lk.handleKeyEvent(ctx, event, project, options) } } diff --git a/cmd/formatter/shortcut_test.go b/cmd/formatter/shortcut_test.go index 0f85be07445..ea76ae953fa 100644 --- a/cmd/formatter/shortcut_test.go +++ b/cmd/formatter/shortcut_test.go @@ -60,6 +60,25 @@ func TestRunHandlesKeysUntilContextIsDone(t *testing.T) { assert.NilError(t, lk.Close(), "Close must be idempotent") } +func TestRunReturnsWhenKeyboardIsClosed(t *testing.T) { + keys := make(chan keyboard.KeyEvent) + lk := NewKeyboardManager(false, false, make(chan os.Signal, 1)) + lk.keys = keys + + done := make(chan struct{}) + go func() { + lk.Run(t.Context(), &types.Project{}, api.UpOptions{}) + close(done) + }() + + close(keys) // what keyboard.Close does to the channel it handed out + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("Run must return once the keyboard channel is closed") + } +} + func TestRunWithoutOpenReturnsImmediately(t *testing.T) { lk := NewKeyboardManager(false, false, make(chan os.Signal, 1)) done := make(chan struct{})