From 0ba96454540ae86d7e611cafb4c555ca11dce875 Mon Sep 17 00:00:00 2001 From: Ben Durrans Date: Tue, 3 Mar 2026 17:09:39 +0000 Subject: [PATCH 1/2] fix: server lifecycle context Done by passing a context to the loading of the configured environment. This will prevent interactive terminals being launched when the server shutsdown. Currently targeting the PR branch in GAF. Change before merge. --- application/config/config.go | 36 +++++++ application/server/codeaction_handlers.go | 11 +- application/server/configuration_test.go | 4 +- application/server/execute_command_test.go | 16 +-- application/server/notification_test.go | 4 +- application/server/server.go | 65 ++++++----- application/server/server_test.go | 101 ++++++++++++++---- application/server/trust_test.go | 8 +- .../server/unified_test_api_smoke_test.go | 4 - domain/snyk/scanner/scanner.go | 2 +- go.mod | 2 +- go.sum | 4 +- infrastructure/cli/cli.go | 13 ++- infrastructure/cli/cli_extension_executor.go | 6 +- infrastructure/oss/cli_scanner.go | 4 +- internal/env.go | 5 +- internal/env_test.go | 2 +- internal/sdk/sdk.go | 10 +- internal/testutil/test_setup.go | 4 + 19 files changed, 214 insertions(+), 87 deletions(-) diff --git a/application/config/config.go b/application/config/config.go index 8aeb23d38..d9f45da40 100644 --- a/application/config/config.go +++ b/application/config/config.go @@ -211,6 +211,8 @@ type Config struct { offline bool ws types.Workspace isLSPInitialized bool + serverLifecycleCtx context.Context + serverLifecycleCtxCancelFunc context.CancelFunc cachedOriginalPath string userSettingsPath string lastSetOrganization string // Trimmed raw org value last passed to SetOrganization @@ -1556,6 +1558,40 @@ func (c *Config) SetLSPInitialized(initialized bool) { c.isLSPInitialized = initialized } +func (c *Config) SetServerLifecycleContext(ctx context.Context, cancel context.CancelFunc) { + if ctx == nil { + panic("server lifecycle context must not be nil") + } + if cancel == nil { + panic("server lifecycle cancel func must not be nil") + } + c.m.Lock() + defer c.m.Unlock() + c.serverLifecycleCtx = ctx + c.serverLifecycleCtxCancelFunc = cancel +} + +// ServerLifecycleContext returns the language server lifecycle context to be used by +// background operations. It will be canceled when the language server shuts down. +func (c *Config) ServerLifecycleContext() context.Context { + c.m.RLock() + defer c.m.RUnlock() + if c.serverLifecycleCtx == nil { + panic("server lifecycle context is not initialized") + } + return c.serverLifecycleCtx +} + +func (c *Config) CancelServerLifecycleContext() { + c.m.RLock() + cancel := c.serverLifecycleCtxCancelFunc + c.m.RUnlock() + if cancel == nil { + panic("server lifecycle cancel func is not initialized") + } + cancel() +} + func (c *Config) EmptyToken() bool { return !c.NonEmptyToken() } diff --git a/application/server/codeaction_handlers.go b/application/server/codeaction_handlers.go index 81a771713..cca7cc435 100644 --- a/application/server/codeaction_handlers.go +++ b/application/server/codeaction_handlers.go @@ -57,17 +57,18 @@ func GetCodeActionHandler(c *config.Config) TextDocumentCodeActionHandler { // We share a mutex between all the handler calls to prevent concurrent runs. var mu = &sync.Mutex{} - // This "field" is shared between the handlers to allow for cancellation of previous handler - _, cancel := context.WithCancel(context.Background()) + // This "field" is shared between the handlers to allow for cancellation of previous handler. + // Use a no-op fake cancel func as the initial cancel func, so the first call has something to cancel. + var cancelFunc context.CancelFunc = func() {} logger := c.Logger().With().Str("method", "CodeActionHandler").Logger() return func(paramCtx context.Context, params types.CodeActionParams) ([]types.LSPCodeAction, error) { // We want to avoid concurrent runs of this handler to prevent race condition. var ctx context.Context mu.Lock() - cancel() - ctx, cancel = context.WithCancel(paramCtx) - defer cancel() + cancelFunc() + ctx, cancelFunc = context.WithCancel(paramCtx) + defer cancelFunc() mu.Unlock() // This handler uses debouncing because it is called very often on mouse/caret moves. diff --git a/application/server/configuration_test.go b/application/server/configuration_test.go index fd374bcd6..fb2cfbc8a 100644 --- a/application/server/configuration_test.go +++ b/application/server/configuration_test.go @@ -114,7 +114,7 @@ func Test_WorkspaceDidChangeConfiguration_Push(t *testing.T) { func Test_WorkspaceDidChangeConfiguration_Pull(t *testing.T) { c := testutil.UnitTest(t) - loc, _ := setupCustomServer(t, c, callBackMock) + loc, _ := setupServer(t, c, withCallbackFn(callBackMock)) _, err := loc.Client.Call(t.Context(), "initialize", types.InitializeParams{ Capabilities: types.ClientCapabilities{ @@ -157,7 +157,7 @@ func callBackMock(_ context.Context, request *jrpc2.Request) (any, error) { func Test_WorkspaceDidChangeConfiguration_PullNoCapability(t *testing.T) { c := testutil.UnitTest(t) - loc, jsonRPCRecorder := setupCustomServer(t, c, callBackMock) + loc, jsonRPCRecorder := setupServer(t, c, withCallbackFn(callBackMock)) params := types.DidChangeConfigurationParams{Settings: types.Settings{}} var updated = true diff --git a/application/server/execute_command_test.go b/application/server/execute_command_test.go index b3493272d..c83ab7aaf 100644 --- a/application/server/execute_command_test.go +++ b/application/server/execute_command_test.go @@ -42,7 +42,7 @@ import ( func Test_executeWorkspaceScanCommand_shouldStartWorkspaceScanOnCommandReceipt(t *testing.T) { c := testutil.UnitTest(t) - loc, _ := setupServerWithCustomDI(t, c, false) + loc, _ := setupServer(t, c, withRealDI()) s := &scanner.TestScanner{} c.Workspace().AddFolder(workspace.NewFolder(c, "dummy", "dummy", s, di.HoverService(), di.ScanNotifier(), di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver())) @@ -59,7 +59,7 @@ func Test_executeWorkspaceScanCommand_shouldStartWorkspaceScanOnCommandReceipt(t func Test_executeWorkspaceFolderScanCommand_shouldStartFolderScanOnCommandReceipt(t *testing.T) { c := testutil.UnitTest(t) - loc, _ := setupServerWithCustomDI(t, c, false) + loc, _ := setupServer(t, c, withRealDI()) s := &scanner.TestScanner{} c.Workspace().AddFolder(workspace.NewFolder(c, "dummy", "dummy", s, di.HoverService(), di.ScanNotifier(), di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver())) @@ -76,7 +76,7 @@ func Test_executeWorkspaceFolderScanCommand_shouldStartFolderScanOnCommandReceip func Test_executeWorkspaceFolderScanCommand_shouldNotClearOtherFoldersDiagnostics(t *testing.T) { c := testutil.UnitTest(t) - loc, _ := setupServerWithCustomDI(t, c, false) + loc, _ := setupServer(t, c, withRealDI()) scannerForFolder := scanner.NewTestScanner() scannerForDontClear := scanner.NewTestScanner() @@ -109,7 +109,7 @@ func Test_executeWorkspaceFolderScanCommand_shouldNotClearOtherFoldersDiagnostic func Test_executeWorkspaceScanCommand_shouldAskForTrust(t *testing.T) { c := testutil.UnitTest(t) - loc, jsonRPCRecorder := setupServerWithCustomDI(t, c, false) + loc, jsonRPCRecorder := setupServer(t, c, withRealDI()) s := &scanner.TestScanner{} c.Workspace().AddFolder(workspace.NewFolder(c, "dummy", "dummy", s, di.HoverService(), di.ScanNotifier(), di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver())) @@ -128,7 +128,7 @@ func Test_executeWorkspaceScanCommand_shouldAskForTrust(t *testing.T) { func Test_executeWorkspaceScanCommand_shouldAcceptScanSourceParam(t *testing.T) { c := testutil.UnitTest(t) - loc, jsonRPCRecorder := setupServerWithCustomDI(t, c, false) + loc, jsonRPCRecorder := setupServer(t, c, withRealDI()) s := &scanner.TestScanner{} c.Workspace().AddFolder(workspace.NewFolder(c, "dummy", "dummy", s, di.HoverService(), di.ScanNotifier(), di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver())) @@ -232,7 +232,7 @@ func Test_TrustWorkspaceFolders(t *testing.T) { t.Run("Doesn't mutate trusted folders, if trusted folders disabled", func(t *testing.T) { c := testutil.UnitTest(t) - loc, _ := setupServerWithCustomDI(t, c, false) + loc, _ := setupServer(t, c, withRealDI()) c.Workspace().AddFolder(workspace.NewFolder(c, folderPath1, "dummy", nil, di.HoverService(), di.ScanNotifier(), di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver())) @@ -247,7 +247,7 @@ func Test_TrustWorkspaceFolders(t *testing.T) { t.Run("Updates trusted workspace folders", func(t *testing.T) { c := testutil.UnitTest(t) - loc, _ := setupServerWithCustomDI(t, c, false) + loc, _ := setupServer(t, c, withRealDI()) c.Workspace().AddFolder(workspace.NewFolder(c, folderPath1, "dummy", nil, di.HoverService(), di.ScanNotifier(), di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver())) c.Workspace().AddFolder(workspace.NewFolder(c, folderPath2, "dummy", nil, di.HoverService(), di.ScanNotifier(), di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver())) @@ -266,7 +266,7 @@ func Test_TrustWorkspaceFolders(t *testing.T) { t.Run("Existing trusted workspace folders are not removed", func(t *testing.T) { c := testutil.UnitTest(t) - loc, _ := setupServerWithCustomDI(t, c, false) + loc, _ := setupServer(t, c, withRealDI()) c.Workspace().AddFolder(workspace.NewFolder(c, folderPath1, "dummy", nil, di.HoverService(), di.ScanNotifier(), di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver())) c.SetTrustedFolderFeatureEnabled(true) diff --git a/application/server/notification_test.go b/application/server/notification_test.go index 999577814..834d8022d 100644 --- a/application/server/notification_test.go +++ b/application/server/notification_test.go @@ -264,11 +264,11 @@ func TestShowMessageRequest(t *testing.T) { t.Run("should execute a command when action item is selected", func(t *testing.T) { c := testutil.UnitTest(t) selectedAction := "Open browser" - loc, _ := setupCustomServer(t, c, func(_ context.Context, _ *jrpc2.Request) (any, error) { + loc, _ := setupServer(t, c, withCallbackFn(func(_ context.Context, _ *jrpc2.Request) (any, error) { return types.MessageActionItem{ Title: selectedAction, }, nil - }) + })) _, err := loc.Client.Call(t.Context(), "initialize", nil) if err != nil { t.Fatal(err) diff --git a/application/server/server.go b/application/server/server.go index 2d92973f5..a213c6d7f 100644 --- a/application/server/server.go +++ b/application/server/server.go @@ -62,6 +62,14 @@ import ( func Start(c *config.Config) { var srv *jrpc2.Server + // Create a context that can be used for background operations that will be canceled when the language server shuts down. + lifecycleCtx, lifecycleCancel := context.WithCancel(context.Background()) + c.SetServerLifecycleContext(lifecycleCtx, lifecycleCancel) + // Safety net: if the server exits without processing shutdown or exit, + // still cancel lifecycle-bound background operations. This `defer` will only + // process after `srv.WaitStatus()` returns, which is only after the JRPC server exits. + defer lifecycleCancel() + handlers := handler.Map{} srv = jrpc2.NewServer(handlers, &jrpc2.ServerOptions{ Logger: func(text string) { @@ -97,7 +105,7 @@ func initHandlers(srv *jrpc2.Server, handlers handler.Map, c *config.Config) { handlers["textDocument/didChange"] = textDocumentDidChangeHandler() handlers["textDocument/didClose"] = noOpHandler() handlers[textDocumentDidOpenOperation] = textDocumentDidOpenHandler(c) - handlers[textDocumentDidSaveOperation] = textDocumentDidSaveHandler() + handlers[textDocumentDidSaveOperation] = textDocumentDidSaveHandler(c) handlers["textDocument/hover"] = textDocumentHover() handlers["textDocument/codeAction"] = textDocumentCodeActionHandler() handlers["textDocument/codeLens"] = codeLensHandler() @@ -105,8 +113,8 @@ func initHandlers(srv *jrpc2.Server, handlers handler.Map, c *config.Config) { handlers["textDocument/willSave"] = noOpHandler() handlers["textDocument/willSaveWaitUntil"] = noOpHandler() handlers["codeAction/resolve"] = codeActionResolveHandler(srv) - handlers["shutdown"] = shutdown() - handlers["exit"] = exit(srv) + handlers["shutdown"] = shutdown(c) + handlers["exit"] = exit(srv, c) handlers["workspace/didChangeWorkspaceFolders"] = workspaceDidChangeWorkspaceFoldersHandler(c, srv) handlers["workspace/willDeleteFiles"] = workspaceWillDeleteFilesHandler(c) handlers["workspace/didChangeConfiguration"] = workspaceDidChangeConfiguration(c, srv) @@ -181,23 +189,24 @@ func codeLensHandler() jrpc2.Handler { func workspaceDidChangeWorkspaceFoldersHandler(c *config.Config, srv *jrpc2.Server) jrpc2.Handler { return handler.New(func(ctx context.Context, params types.DidChangeWorkspaceFoldersParams) (any, error) { - // The context provided by the JSON-RPC server is canceled once a new message is being processed, - // so we don't want to propagate it to functions that start background operations - bgCtx := context.Background() logger := c.Logger().With().Str("method", "WorkspaceDidChangeWorkspaceFoldersHandler").Logger() - logger.Info().Msg("RECEIVING") defer logger.Info().Msg("SENDING") + + // The context provided by the JSON-RPC server is canceled once a new message is being processed, + // so we don't want to propagate it to functions that start background operations, + // instead we'll use a background context tied to the language server's lifecycle. + lifecycleCtx := c.ServerLifecycleContext() changedFolders := c.Workspace().ChangeWorkspaceFolders(params) if di.AuthenticationService().IsAuthenticated() { - di.LdxSyncService().RefreshConfigFromLdxSync(bgCtx, c, changedFolders, di.Notifier()) + di.LdxSyncService().RefreshConfigFromLdxSync(lifecycleCtx, c, changedFolders, di.Notifier()) } - command.HandleFolders(c, bgCtx, srv, di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver()) + command.HandleFolders(c, lifecycleCtx, srv, di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver()) for _, f := range changedFolders { if f.IsAutoScanEnabled() { - go f.ScanFolder(ctx) + go f.ScanFolder(lifecycleCtx) } } return nil, nil @@ -443,7 +452,12 @@ func initializedHandler(c *config.Config, srv *jrpc2.Server) handler.Func { logger.Error().Err(err).Msg("Scan initialization error, canceling scan") return nil, nil } - command.HandleFolders(c, context.Background(), srv, di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver()) + + // The context provided by the JSON-RPC server is canceled once a new message is being processed, + // so we don't want to propagate it to functions that start background operations, + // instead we'll use a background context tied to the language server's lifecycle. + lifecycleCtx := c.ServerLifecycleContext() + command.HandleFolders(c, lifecycleCtx, srv, di.Notifier(), di.ScanPersister(), di.ScanStateAggregator(), di.FeatureFlagService(), di.ConfigResolver()) // Check once for expired cache in same thread before triggering a scan. // Start a periodic go routine to check for the expired cache afterwards @@ -453,7 +467,7 @@ func initializedHandler(c *config.Config, srv *jrpc2.Server) handler.Func { autoScanEnabled := c.IsAutoScanEnabled() if autoScanEnabled { logger.Info().Msg("triggering workspace scan after successful initialization") - c.Workspace().ScanWorkspace(context.Background()) + c.Workspace().ScanWorkspace(lifecycleCtx) } else { msg := fmt.Sprintf( "No automatic workspace scan on initialization: autoScanEnabled=%v", @@ -630,12 +644,12 @@ func monitorClientProcess(pid int) time.Duration { return time.Since(start) } -func shutdown() jrpc2.Handler { - return handler.New(func(ctx context.Context) (any, error) { - c := config.CurrentConfig() +func shutdown(c *config.Config) jrpc2.Handler { + return handler.New(func(_ context.Context) (any, error) { logger := c.Logger().With().Str("method", "Shutdown").Logger() logger.Info().Msg("ENTERING") defer logger.Info().Msg("RETURNING") + c.CancelServerLifecycleContext() di.ErrorReporter().FlushErrorReporting() disposeProgressListener() @@ -645,11 +659,13 @@ func shutdown() jrpc2.Handler { }) } -func exit(srv *jrpc2.Server) jrpc2.Handler { +func exit(srv *jrpc2.Server, c *config.Config) jrpc2.Handler { return handler.New(func(_ context.Context) (any, error) { - c := config.CurrentConfig() logger := c.Logger().With().Str("method", "Exit").Logger() logger.Info().Msg("ENTERING") + // Canceling the server's lifecycle context should have been covered by shutdown, + // but we'll do it again, just in case it was skipped. + c.CancelServerLifecycleContext() logger.Info().Msg("Flushing error reporting...") di.ErrorReporter().FlushErrorReporting() logger.Info().Msg("Stopping server...") @@ -704,15 +720,16 @@ func textDocumentDidOpenHandler(c *config.Config) jrpc2.Handler { }) } -func textDocumentDidSaveHandler() jrpc2.Handler { +func textDocumentDidSaveHandler(c *config.Config) jrpc2.Handler { return handler.New(func(_ context.Context, params sglsp.DidSaveTextDocumentParams) (any, error) { - // The context provided by the JSON-RPC server is canceled once a new message is being processed, - // so we don't want to propagate it to functions that start background operations - bgCtx := context.Background() - c := config.CurrentConfig() logger := c.Logger().With().Str("method", "TextDocumentDidSaveHandler").Logger() logger.Debug().Interface("params", params).Msg("Receiving") + // The context provided by the JSON-RPC server is canceled once a new message is being processed, + // so we don't want to propagate it to functions that start background operations, + // instead we'll use a background context tied to the language server's lifecycle. + lifecycleCtx := c.ServerLifecycleContext() + di.FileWatcher().SetFileAsSaved(params.TextDocument.URI) filePath := uri.PathFromUri(params.TextDocument.URI) @@ -728,12 +745,12 @@ func textDocumentDidSaveHandler() jrpc2.Handler { } if folder.IsAutoScanEnabled() && uri.IsDotSnykFile(params.TextDocument.URI) { - go folder.ScanFolder(bgCtx) + go folder.ScanFolder(lifecycleCtx) return nil, nil } if folder.IsAutoScanEnabled() { - go folder.ScanFile(bgCtx, filePath) + go folder.ScanFile(lifecycleCtx, filePath) } else { logger.Warn().Msg("Not scanning, auto-scan is disabled") } diff --git a/application/server/server_test.go b/application/server/server_test.go index fb4421dc4..401d91e00 100644 --- a/application/server/server_test.go +++ b/application/server/server_test.go @@ -82,24 +82,38 @@ func didOpenTextParams(t *testing.T) (sglsp.DidOpenTextDocumentParams, types.Fil return didOpenParams, dirPath } -func setupServer(t *testing.T, c *config.Config) (server.Local, *testsupport.JsonRPCRecorder) { - t.Helper() - return setupCustomServer(t, c, nil) +type setupServerOptions struct { + performLifecycleCleanup bool + callBackFn onCallbackFn + useRealDI bool } -func setupServerWithCustomDI(t *testing.T, c *config.Config, useMocks bool) (server.Local, *testsupport.JsonRPCRecorder) { - t.Helper() - s, jsonRPCRecorder := setupCustomServer(t, c, nil) - if !useMocks { - di.Init() +type setupServerOption func(*setupServerOptions) + +func withSkipOfLSShutdownCleanup() setupServerOption { + return func(opts *setupServerOptions) { + opts.performLifecycleCleanup = false + } +} + +func withCallbackFn(callBackFn onCallbackFn) setupServerOption { + return func(opts *setupServerOptions) { + opts.callBackFn = callBackFn + } +} + +func withRealDI() setupServerOption { + return func(opts *setupServerOptions) { + opts.useRealDI = true } - return s, jsonRPCRecorder } -func setupCustomServer(t *testing.T, c *config.Config, callBackFn onCallbackFn) (server.Local, *testsupport.JsonRPCRecorder) { +func setupServer(t *testing.T, c *config.Config, opts ...setupServerOption) (server.Local, *testsupport.JsonRPCRecorder) { t.Helper() - if c == nil { - c = testutil.UnitTest(t) + + options := setupServerOptions{performLifecycleCleanup: true} + for _, opt := range opts { + opt(&options) } // Ensure SNYK_API endpoint is set in config if environment variable is present @@ -109,17 +123,38 @@ func setupCustomServer(t *testing.T, c *config.Config, callBackFn onCallbackFn) } jsonRPCRecorder := &testsupport.JsonRPCRecorder{} - loc := startServer(c, callBackFn, jsonRPCRecorder) - di.TestInit(t) - cleanupChannels() + loc := startServer(c, options.callBackFn, jsonRPCRecorder) t.Cleanup(func() { - _, _ = loc.Client.Call(context.Background(), "shutdown", nil) + if options.performLifecycleCleanup { + // Shutdown LS + shutdownCtx, shutdownCtxCancel := context.WithTimeout(context.Background(), 30*time.Second) + defer shutdownCtxCancel() + _, err := loc.Client.Call(shutdownCtx, "shutdown", nil) + assert.NoError(t, err, "LS shutdown errored") + assert.NoError(t, shutdownCtx.Err(), "LS shutdown took too long") + + // Exit LS + exitCtx, exitCtxCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer exitCtxCancel() + err = loc.Client.Notify(exitCtx, "exit", nil) + assert.NoError(t, err, "LS exit errored") + assert.NoError(t, exitCtx.Err(), "LS exit took too long") + } + + // Cleanup _ = loc.Close() cleanupChannels() jsonRPCRecorder.ClearCallbacks() jsonRPCRecorder.ClearNotifications() }) + + di.TestInit(t) + cleanupChannels() + if options.useRealDI { + di.Init() + } + return loc, jsonRPCRecorder } @@ -204,6 +239,34 @@ func Test_shutdown_shouldBeServed(t *testing.T) { assert.NotNil(t, rsp) } +func Test_shutdown_shouldCancelServerLifecycleContext(t *testing.T) { + c := testutil.UnitTest(t) + loc, _ := setupServer(t, c) + + _, err := loc.Client.Call(t.Context(), "shutdown", nil) + require.NoError(t, err) + + lifecycleCtx := c.ServerLifecycleContext() + assert.Eventually(t, func() bool { + return lifecycleCtx.Err() != nil + }, time.Second, 10*time.Millisecond) +} + +// Test_exit_shouldCancelServerLifecycleContext tests that the server lifecycle context is canceled when the +// "exit" handler is called as a safety measure. The real relied on cancel should be handled by "shutdown". +func Test_exit_shouldCancelServerLifecycleContext(t *testing.T) { + c := testutil.UnitTest(t) + loc, _ := setupServer(t, c, withSkipOfLSShutdownCleanup()) + + err := loc.Client.Notify(t.Context(), "exit", nil) + require.NoError(t, err) + + lifecycleCtx := c.ServerLifecycleContext() + assert.Eventually(t, func() bool { + return lifecycleCtx.Err() != nil + }, time.Second, 10*time.Millisecond) +} + func Test_initialize_containsServerInfo(t *testing.T) { c := testutil.UnitTest(t) loc, _ := setupServer(t, c) @@ -825,7 +888,7 @@ func Test_textDocumentDidSaveHandler_shouldTriggerScanForDotSnykFile(t *testing. sendFileSavedMessage(t, c, snykFilePath, folderPath, loc) - // Wait for $/snyk.scan notification + // Wait for any $/snyk.scan notification assert.Eventually( t, checkForSnykScan(t, jsonRPCRecorder), @@ -1018,7 +1081,7 @@ func Test_workspaceDidChangeWorkspaceFolders_CallsRefreshConfigFromLdxSync(t *te c.SetAuthenticationMethod(types.FakeAuthentication) // Setup server - loc, _ := setupServerWithCustomDI(t, c, false) + loc, _ := setupServer(t, c, withRealDI()) // Setup mock LdxSyncService AFTER setupServer to avoid it being overwritten by di.TestInit ctrl := gomock.NewController(t) @@ -1089,7 +1152,7 @@ func Test_initialized_CallsRefreshConfigFromLdxSync(t *testing.T) { } // Setup server - loc, _ := setupServerWithCustomDI(t, c, false) + loc, _ := setupServer(t, c, withRealDI()) // Setup mock LdxSyncService AFTER setupServer to avoid it being overwritten by di.TestInit ctrl := gomock.NewController(t) diff --git a/application/server/trust_test.go b/application/server/trust_test.go index 80091b9fd..fb33c37d2 100644 --- a/application/server/trust_test.go +++ b/application/server/trust_test.go @@ -70,11 +70,11 @@ func Test_handleUntrustedFolders_shouldNotTriggerTrustRequestWhenAlreadyRequesti func Test_handleUntrustedFolders_shouldTriggerTrustRequestAndScanAfterConfirmation(t *testing.T) { c := testutil.UnitTest(t) - loc, jsonRPCRecorder := setupCustomServer(t, c, func(_ context.Context, _ *jrpc2.Request) (any, error) { + loc, jsonRPCRecorder := setupServer(t, c, withCallbackFn(func(_ context.Context, _ *jrpc2.Request) (any, error) { return types.MessageActionItem{ Title: command.DoTrust, }, nil - }) + })) registerNotifier(c, loc.Server) w := c.Workspace() @@ -93,11 +93,11 @@ func Test_handleUntrustedFolders_shouldTriggerTrustRequestAndScanAfterConfirmati func Test_handleUntrustedFolders_shouldTriggerTrustRequestAndNotScanAfterNegativeConfirmation(t *testing.T) { c := testutil.UnitTest(t) - loc, _ := setupCustomServer(t, c, func(_ context.Context, _ *jrpc2.Request) (any, error) { + loc, _ := setupServer(t, c, withCallbackFn(func(_ context.Context, _ *jrpc2.Request) (any, error) { return types.MessageActionItem{ Title: command.DontTrust, }, nil - }) + })) registerNotifier(c, loc.Server) w := c.Workspace() sc := &scanner.TestScanner{} diff --git a/application/server/unified_test_api_smoke_test.go b/application/server/unified_test_api_smoke_test.go index a3db46182..9ed905afc 100644 --- a/application/server/unified_test_api_smoke_test.go +++ b/application/server/unified_test_api_smoke_test.go @@ -89,10 +89,6 @@ func runOSSComparisonTest(t *testing.T, unifiedScan bool, dir string) []types.Di t.Helper() c, loc, jsonRPCRecorder := setupOSSComparisonTest(t) - defer func() { - _ = loc.Client.Close() - loc.Server.Stop() - }() // ----------------------------------------- // setup test repo diff --git a/domain/snyk/scanner/scanner.go b/domain/snyk/scanner/scanner.go index c5d3a9041..b9e91da1d 100644 --- a/domain/snyk/scanner/scanner.go +++ b/domain/snyk/scanner/scanner.go @@ -298,7 +298,7 @@ func (sc *DelegatingConcurrentScanner) Scan(ctx context.Context, pathToScan type go func() { defer referenceBranchScanWaitGroup.Done() isSingleFileScan := pathToScan != folderPath - scanTypeCtx := ctx2.NewContextWithDeltaScanType(ctx2.Clone(ctx, context.Background()), ctx2.Reference) + scanTypeCtx := ctx2.NewContextWithDeltaScanType(ctx, ctx2.Reference) refScanCtx, refLogger := sc.enrichContextAndLogger(scanTypeCtx, scanLogger, workspaceFolderConfig, folderPath, pathToScan) // only trigger a base scan if we are scanning an actual working directory. It could also be a diff --git a/go.mod b/go.mod index c9abd0ed9..84f23f80b 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( github.com/snyk/cli-extension-secrets v0.0.0-20260210120918-669d47e834cf github.com/snyk/code-client-go v1.26.1 github.com/snyk/error-catalog-golang-public v0.0.0-20260205094614-116c03822905 - github.com/snyk/go-application-framework v0.0.0-20260211155351-c4fb58433d93 + github.com/snyk/go-application-framework v0.0.0-20260305101054-96ae0163d6f9 github.com/snyk/studio-mcp v1.6.1 github.com/sourcegraph/go-lsp v0.0.0-20240223163137-f80c5dd31dfd github.com/spf13/pflag v1.0.10 diff --git a/go.sum b/go.sum index 68602a0e4..0001836c6 100644 --- a/go.sum +++ b/go.sum @@ -313,8 +313,8 @@ github.com/snyk/code-client-go v1.26.1 h1:09g3z462ZxSmXxuH2s13DbKjUPH5ve20af2+RV github.com/snyk/code-client-go v1.26.1/go.mod h1:0NcZZHB48Sr4UAucEH2H10HwV7gjI2Ue0c+FxPWaTNo= github.com/snyk/error-catalog-golang-public v0.0.0-20260205094614-116c03822905 h1:pUe6iOWHEOFY0t4u4ssXeTqpMmZBu1xq06VBFI9zUik= github.com/snyk/error-catalog-golang-public v0.0.0-20260205094614-116c03822905/go.mod h1:Ytttq7Pw4vOCu9NtRQaOeDU2dhBYUyNBe6kX4+nIIQ4= -github.com/snyk/go-application-framework v0.0.0-20260211155351-c4fb58433d93 h1:LfhYf3LpiekqfuLVT2/qnuDiKpWmYevs8ZK1smOSyAA= -github.com/snyk/go-application-framework v0.0.0-20260211155351-c4fb58433d93/go.mod h1:6MuCxSVGYNY3gfNKPZc5oMuy5/Q+yxbLxKnVtOMSB8Y= +github.com/snyk/go-application-framework v0.0.0-20260305101054-96ae0163d6f9 h1:aHwif/gZHALiCHMcLtnPvmA0KWRJTdMvaLUlNrV+Vlk= +github.com/snyk/go-application-framework v0.0.0-20260305101054-96ae0163d6f9/go.mod h1:6MuCxSVGYNY3gfNKPZc5oMuy5/Q+yxbLxKnVtOMSB8Y= github.com/snyk/go-httpauth v0.0.0-20231117135515-eb445fea7530 h1:s9PHNkL6ueYRiAKNfd8OVxlUOqU3qY0VDbgCD1f6WQY= github.com/snyk/go-httpauth v0.0.0-20231117135515-eb445fea7530/go.mod h1:88KbbvGYlmLgee4OcQ19yr0bNpXpOr2kciOthaSzCAg= github.com/snyk/studio-mcp v1.6.1 h1:rk+3eZDt9hVcdnmCzvK78qzuBzbxAHBf862ABc38t+I= diff --git a/infrastructure/cli/cli.go b/infrastructure/cli/cli.go index 0cf265284..4428afb35 100644 --- a/infrastructure/cli/cli.go +++ b/infrastructure/cli/cli.go @@ -67,15 +67,15 @@ type Executor interface { } func (c *SnykCli) Execute(ctx context.Context, cmd []string, workingDir types.FilePath, env gotenv.Env) (resp []byte, err error) { - method := "SnykCli.Execute" - c.c.Logger().Debug().Str("method", method).Interface("cmd", cmd).Str("workingDir", string(workingDir)).Msg("calling Snyk CLI") + logger := c.c.Logger().With().Str("method", "SnykCli.Execute").Logger() + logger.Debug().Interface("cmd", cmd).Str("workingDir", string(workingDir)).Msg("calling Snyk CLI") // set deadline to handle CLI hanging when obtaining semaphore ctx, cancel := context.WithTimeout(ctx, c.cliTimeout) defer cancel() output, err := c.doExecute(ctx, cmd, workingDir, env) - c.c.Logger().Trace().Str("method", method).Str("response", string(output)) + logger.Trace().Str("response", string(output)).Err(err).Msg("Snyk CLI executed") return output, err } @@ -115,8 +115,13 @@ func (c *SnykCli) getCommand(cmd []string, workingDir types.FilePath, ctx contex } else { cloneConfig := c.c.Engine().GetConfiguration().Clone() cloneConfig.Set(configuration.WORKING_DIRECTORY, workingDir) + // this is not thread-safe - envvars.LoadConfiguredEnvironment(cloneConfig.GetStringSlice(configuration.CUSTOM_CONFIG_FILES), string(workingDir)) + envvars.LoadConfiguredEnvironmentWithOptions( + envvars.WithContext(ctx), + envvars.WithLogger(c.c.Logger()), + envvars.WithCustomConfigFiles(cloneConfig.GetStringSlice(configuration.CUSTOM_CONFIG_FILES), string(workingDir)), + ) envvars.UpdatePath(c.c.GetUserSettingsPath(), true) // prioritize the user specified PATH over their SHELL's baseEnv = os.Environ() } diff --git a/infrastructure/cli/cli_extension_executor.go b/infrastructure/cli/cli_extension_executor.go index 7051bf6d3..181ea1220 100644 --- a/infrastructure/cli/cli_extension_executor.go +++ b/infrastructure/cli/cli_extension_executor.go @@ -115,7 +115,11 @@ func (c ExtensionExecutor) doExecute(ctx context.Context, cmd []string, workingD } legacyCLIConfig.Set(configuration.RAW_CMD_ARGS, cmd[1:]) - envvars.LoadConfiguredEnvironment(legacyCLIConfig.GetStringSlice(configuration.CUSTOM_CONFIG_FILES), string(workingDir)) + envvars.LoadConfiguredEnvironmentWithOptions( + envvars.WithContext(ctx), + envvars.WithLogger(c.c.Logger()), + envvars.WithCustomConfigFiles(legacyCLIConfig.GetStringSlice(configuration.CUSTOM_CONFIG_FILES), string(workingDir)), + ) envvars.UpdatePath(c.c.GetUserSettingsPath(), true) // prioritize the user specified PATH over their SHELL's data, err := engine.InvokeWithConfig(legacyCLI, legacyCLIConfig) diff --git a/infrastructure/oss/cli_scanner.go b/infrastructure/oss/cli_scanner.go index f0257e8e1..fdf4362f9 100644 --- a/infrastructure/oss/cli_scanner.go +++ b/infrastructure/oss/cli_scanner.go @@ -598,8 +598,8 @@ func (cliScanner *CLIScanner) scheduleRefreshScan(ctx context.Context, path type cliScanner.scheduledScan = timer cliScanner.scheduledScanMtx.Unlock() - // decouple scheduled scan from session but keep context values - newCtx := ctx2.Clone(ctx, context.Background()) + // Decouple scheduled scan context from current scan context but keep context values + newCtx := ctx2.Clone(ctx, cliScanner.config.ServerLifecycleContext()) go func() { select { diff --git a/internal/env.go b/internal/env.go index bbe89bba7..6947f9709 100644 --- a/internal/env.go +++ b/internal/env.go @@ -2,6 +2,7 @@ package env import ( + "context" "maps" "os" "strings" @@ -18,10 +19,10 @@ import ( // The custom config files override the OS environment variables. // The userSettingsPath is used to prioritize the user specified PATH over their SHELL's PATH. // Config files can be specified in the configuration using the `configuration.CUSTOM_CONFIG_FILES` key. -func GetEnvFromSystemAndConfiguration(cfg configuration.Configuration, userSettingsPath string, logger *zerolog.Logger) gotenv.Env { +func GetEnvFromSystemAndConfiguration(ctx context.Context, cfg configuration.Configuration, userSettingsPath string, logger *zerolog.Logger) gotenv.Env { // load the env from shell, but don't load custom config files, // as we don't want to load the dir-specific files into the global environment - envvars.LoadConfiguredEnvironment([]string{}, "") + envvars.LoadConfiguredEnvironmentWithOptions(envvars.WithContext(ctx), envvars.WithLogger(logger)) // prioritize the user specified PATH over their SHELL's envvars.UpdatePath(userSettingsPath, true) diff --git a/internal/env_test.go b/internal/env_test.go index ba9fd13c8..41523db4a 100644 --- a/internal/env_test.go +++ b/internal/env_test.go @@ -44,7 +44,7 @@ func TestGetEnvFromSystemAndConfiguration_CustomConfigOverridesOS(t *testing.T) require.NoError(t, os.WriteFile(customConfigFile, []byte("FOO=from_file\nBAR=only_file\n"), 0660)) cfg.Set(configuration.CUSTOM_CONFIG_FILES, []string{customConfigFile}) - env := GetEnvFromSystemAndConfiguration(cfg, t.TempDir(), &logger) + env := GetEnvFromSystemAndConfiguration(t.Context(), cfg, t.TempDir(), &logger) assert.Equal(t, "from_file", env["FOO"]) assert.Equal(t, "only_file", env["BAR"]) diff --git a/internal/sdk/sdk.go b/internal/sdk/sdk.go index 8194c0f0f..ed4ce4cd7 100644 --- a/internal/sdk/sdk.go +++ b/internal/sdk/sdk.go @@ -39,7 +39,7 @@ func UpdateEnvironmentAndReturnAdditionalParams(c *config.Config, sdks []types.L var additionalParameters []string // env update - env := env.GetEnvFromSystemAndConfiguration(c.Engine().GetConfiguration(), c.GetUserSettingsPath(), &logger) + envVars := env.GetEnvFromSystemAndConfiguration(c.ServerLifecycleContext(), c.Engine().GetConfiguration(), c.GetUserSettingsPath(), c.Logger()) // update process environment with sdk info for i := 0; i < len(sdks); i++ { @@ -48,18 +48,18 @@ func UpdateEnvironmentAndReturnAdditionalParams(c *config.Config, sdks []types.L pathExt := filepath.Join(path, "bin") switch { case strings.Contains(strings.ToLower(sdk.Type), "java"): - env["JAVA_HOME"] = path + envVars["JAVA_HOME"] = path case strings.Contains(strings.ToLower(sdk.Type), "python"): pathExt = filepath.Dir(path) additionalParameters = append(additionalParameters, "--command="+path) case strings.Contains(strings.ToLower(sdk.Type), "go"): - env["GOROOT"] = path + envVars["GOROOT"] = path } - env[pathEnvVarName] = getPath(pathExt, true) + envVars[pathEnvVarName] = getPath(pathExt, true) logger.Debug().Msg("prepended " + pathExt) } - return additionalParameters, env + return additionalParameters, envVars } // UpdatePath prepends or appends the extension to the current path. diff --git a/internal/testutil/test_setup.go b/internal/testutil/test_setup.go index 8facb7989..1a315038d 100644 --- a/internal/testutil/test_setup.go +++ b/internal/testutil/test_setup.go @@ -67,6 +67,8 @@ func UnitTest(t *testing.T) *config.Config { func UnitTestWithCtx(t *testing.T) (*config.Config, context.Context) { t.Helper() c := config.New(config.WithBinarySearchPaths([]string{})) + lifecycleCtx, lifecycleCancel := context.WithCancel(t.Context()) + c.SetServerLifecycleContext(lifecycleCtx, lifecycleCancel) err := c.WaitForDefaultEnv(t.Context()) if err != nil { t.Fatal(err) @@ -156,6 +158,8 @@ func prepareTestHelper(t *testing.T, envVar string, tokenSecretName string) *con } c := config.New(config.WithBinarySearchPaths([]string{})) + lifecycleCtx, lifecycleCancel := context.WithCancel(t.Context()) + c.SetServerLifecycleContext(lifecycleCtx, lifecycleCancel) err := c.WaitForDefaultEnv(t.Context()) if err != nil { t.Fatal(err) From bc5eeeed97a3249fc71fa5f25319f24bc3ce4120 Mon Sep 17 00:00:00 2001 From: Ben Durrans Date: Tue, 10 Mar 2026 15:06:38 +0000 Subject: [PATCH 2/2] refactor: pass absolute config file paths --- go.mod | 2 +- go.sum | 4 ++-- infrastructure/cli/cli.go | 4 +++- infrastructure/cli/cli_extension_executor.go | 4 +++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 84f23f80b..c2f8a8445 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( github.com/snyk/cli-extension-secrets v0.0.0-20260210120918-669d47e834cf github.com/snyk/code-client-go v1.26.1 github.com/snyk/error-catalog-golang-public v0.0.0-20260205094614-116c03822905 - github.com/snyk/go-application-framework v0.0.0-20260305101054-96ae0163d6f9 + github.com/snyk/go-application-framework v0.0.0-20260310143923-dbfe2da7e2b5 github.com/snyk/studio-mcp v1.6.1 github.com/sourcegraph/go-lsp v0.0.0-20240223163137-f80c5dd31dfd github.com/spf13/pflag v1.0.10 diff --git a/go.sum b/go.sum index 0001836c6..0ce7940b5 100644 --- a/go.sum +++ b/go.sum @@ -313,8 +313,8 @@ github.com/snyk/code-client-go v1.26.1 h1:09g3z462ZxSmXxuH2s13DbKjUPH5ve20af2+RV github.com/snyk/code-client-go v1.26.1/go.mod h1:0NcZZHB48Sr4UAucEH2H10HwV7gjI2Ue0c+FxPWaTNo= github.com/snyk/error-catalog-golang-public v0.0.0-20260205094614-116c03822905 h1:pUe6iOWHEOFY0t4u4ssXeTqpMmZBu1xq06VBFI9zUik= github.com/snyk/error-catalog-golang-public v0.0.0-20260205094614-116c03822905/go.mod h1:Ytttq7Pw4vOCu9NtRQaOeDU2dhBYUyNBe6kX4+nIIQ4= -github.com/snyk/go-application-framework v0.0.0-20260305101054-96ae0163d6f9 h1:aHwif/gZHALiCHMcLtnPvmA0KWRJTdMvaLUlNrV+Vlk= -github.com/snyk/go-application-framework v0.0.0-20260305101054-96ae0163d6f9/go.mod h1:6MuCxSVGYNY3gfNKPZc5oMuy5/Q+yxbLxKnVtOMSB8Y= +github.com/snyk/go-application-framework v0.0.0-20260310143923-dbfe2da7e2b5 h1:nxz9NzjyTLVkPHRrEpLX3YFv1Ys0LFHr73f5GohDJoI= +github.com/snyk/go-application-framework v0.0.0-20260310143923-dbfe2da7e2b5/go.mod h1:6MuCxSVGYNY3gfNKPZc5oMuy5/Q+yxbLxKnVtOMSB8Y= github.com/snyk/go-httpauth v0.0.0-20231117135515-eb445fea7530 h1:s9PHNkL6ueYRiAKNfd8OVxlUOqU3qY0VDbgCD1f6WQY= github.com/snyk/go-httpauth v0.0.0-20231117135515-eb445fea7530/go.mod h1:88KbbvGYlmLgee4OcQ19yr0bNpXpOr2kciOthaSzCAg= github.com/snyk/studio-mcp v1.6.1 h1:rk+3eZDt9hVcdnmCzvK78qzuBzbxAHBf862ABc38t+I= diff --git a/infrastructure/cli/cli.go b/infrastructure/cli/cli.go index 4428afb35..e5ce5bc14 100644 --- a/infrastructure/cli/cli.go +++ b/infrastructure/cli/cli.go @@ -32,6 +32,7 @@ import ( "github.com/snyk/go-application-framework/pkg/configuration" "github.com/snyk/go-application-framework/pkg/envvars" + "github.com/snyk/go-application-framework/pkg/utils" "github.com/snyk/snyk-ls/application/config" noti "github.com/snyk/snyk-ls/internal/notification" @@ -116,11 +117,12 @@ func (c *SnykCli) getCommand(cmd []string, workingDir types.FilePath, ctx contex cloneConfig := c.c.Engine().GetConfiguration().Clone() cloneConfig.Set(configuration.WORKING_DIRECTORY, workingDir) + configFiles := utils.MakeRelativePathsAbsolute(string(workingDir), cloneConfig.GetStringSlice(configuration.CUSTOM_CONFIG_FILES)) // this is not thread-safe envvars.LoadConfiguredEnvironmentWithOptions( envvars.WithContext(ctx), envvars.WithLogger(c.c.Logger()), - envvars.WithCustomConfigFiles(cloneConfig.GetStringSlice(configuration.CUSTOM_CONFIG_FILES), string(workingDir)), + envvars.WithCustomConfigFiles(configFiles), ) envvars.UpdatePath(c.c.GetUserSettingsPath(), true) // prioritize the user specified PATH over their SHELL's baseEnv = os.Environ() diff --git a/infrastructure/cli/cli_extension_executor.go b/infrastructure/cli/cli_extension_executor.go index 181ea1220..607caa412 100644 --- a/infrastructure/cli/cli_extension_executor.go +++ b/infrastructure/cli/cli_extension_executor.go @@ -25,6 +25,7 @@ import ( "github.com/snyk/go-application-framework/pkg/configuration" "github.com/snyk/go-application-framework/pkg/envvars" + "github.com/snyk/go-application-framework/pkg/utils" "github.com/snyk/go-application-framework/pkg/workflow" "github.com/subosito/gotenv" @@ -115,10 +116,11 @@ func (c ExtensionExecutor) doExecute(ctx context.Context, cmd []string, workingD } legacyCLIConfig.Set(configuration.RAW_CMD_ARGS, cmd[1:]) + configFiles := utils.MakeRelativePathsAbsolute(string(workingDir), legacyCLIConfig.GetStringSlice(configuration.CUSTOM_CONFIG_FILES)) envvars.LoadConfiguredEnvironmentWithOptions( envvars.WithContext(ctx), envvars.WithLogger(c.c.Logger()), - envvars.WithCustomConfigFiles(legacyCLIConfig.GetStringSlice(configuration.CUSTOM_CONFIG_FILES), string(workingDir)), + envvars.WithCustomConfigFiles(configFiles), ) envvars.UpdatePath(c.c.GetUserSettingsPath(), true) // prioritize the user specified PATH over their SHELL's