From 65ff59e898d9e07535ba9ddcc1dd4ec78bf9b5eb Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Fri, 5 Jun 2026 16:47:42 +0200 Subject: [PATCH 01/11] fix: reset summary panel on stop-scan [IDE-1035] When the user pressed the stop-scan button (LSP window/workDoneProgress/cancel) the summary panel kept showing the last scan's counts. The org-change path already had a helper that reinitialises the panel to its initial state; this commit wires the same reset into the stop-scan handler. - New resetSummaryPanelOnStopScan helper delegates to the existing resetSummaryPanelForOrgChange with the current workspace folders. - windowWorkDoneProgressCancelHandler now invokes it after progress.Cancel. - Unit tests cover the happy path, nil-aggregator safety, and the no-folders case. --- application/server/configuration.go | 9 ++++++ application/server/configuration_test.go | 39 ++++++++++++++++++++++++ application/server/server.go | 6 ++-- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/application/server/configuration.go b/application/server/configuration.go index 063a5f120..076eee1fd 100644 --- a/application/server/configuration.go +++ b/application/server/configuration.go @@ -876,6 +876,15 @@ func resetSummaryPanelForOrgChange(scanAgg scanstates.Aggregator, folderPaths [] scanAgg.Init(folderPaths) } +// resetSummaryPanelOnStopScan resets the summary panel to its initial state +// when the user cancels a scan via the IDE stop-scan button (IDE-1035). +// It resolves current workspace folder paths from conf and delegates to +// resetSummaryPanelForOrgChange, which guards against nil aggregator and +// empty folder list. +func resetSummaryPanelOnStopScan(scanAgg scanstates.Aggregator, conf configuration.Configuration) { + resetSummaryPanelForOrgChange(scanAgg, workspaceFolderPaths(conf)) +} + func workspaceFolderPaths(conf configuration.Configuration) []types.FilePath { ws := config.GetWorkspace(conf) if ws == nil { diff --git a/application/server/configuration_test.go b/application/server/configuration_test.go index 611deadb0..b48ec1aeb 100644 --- a/application/server/configuration_test.go +++ b/application/server/configuration_test.go @@ -2810,3 +2810,42 @@ func TestApplyUserSettingsPath_IgnoresUnchanged(t *testing.T) { assert.Equal(t, "/original", types.GetGlobalString(conf, types.SettingUserSettingsPath)) } + +// IDE-1035: when the user presses the stop-scan button the IDE sends a +// window/workDoneProgress/cancel notification. The summary panel must be reset +// to its initial state so it no longer shows the counts from the cancelled scan. +func TestResetSummaryPanelOnStopScan_CallsInit(t *testing.T) { + engine, _ := testutil.UnitTestWithEngine(t) + conf := engine.GetConfiguration() + + agg := &initRecordingAggregator{} + + // Seed a folder so workspaceFolderPaths returns something non-empty. + tmpDir := types.FilePath(t.TempDir()) + _, _ = workspaceutil.SetupWorkspace(t, engine, tmpDir) + folders := config.GetWorkspace(conf).Folders() + require.NotEmpty(t, folders, "precondition: workspace must contain at least one folder") + + resetSummaryPanelOnStopScan(agg, conf) + + assert.Len(t, agg.initCalls, 1, "Init must be called once to reset the summary panel") +} + +func TestResetSummaryPanelOnStopScan_NilAgg_DoesNotPanic(t *testing.T) { + engine, _ := testutil.UnitTestWithEngine(t) + conf := engine.GetConfiguration() + assert.NotPanics(t, func() { + resetSummaryPanelOnStopScan(nil, conf) + }) +} + +func TestResetSummaryPanelOnStopScan_NoFolders_DoesNotCallInit(t *testing.T) { + engine, _ := testutil.UnitTestWithEngine(t) + conf := engine.GetConfiguration() + // workspace has no folders — Init must not be called + agg := &initRecordingAggregator{} + assert.NotPanics(t, func() { + resetSummaryPanelOnStopScan(agg, conf) + }) + assert.Empty(t, agg.initCalls, "Init must not be called when there are no workspace folders") +} diff --git a/application/server/server.go b/application/server/server.go index 57be89275..1a73591f7 100644 --- a/application/server/server.go +++ b/application/server/server.go @@ -279,7 +279,7 @@ func initHandlers(srv *jrpc2.Server, handlers handler.Map, conf configuration.Co handlers["workspace/didChangeWorkspaceFolders"] = enrich(workspaceDidChangeWorkspaceFoldersHandler(conf, engine, srv)) handlers["workspace/willDeleteFiles"] = enrich(workspaceWillDeleteFilesHandler(conf)) handlers["workspace/didChangeConfiguration"] = enrich(workspaceDidChangeConfiguration(conf, srv)) - handlers["window/workDoneProgress/cancel"] = enrich(windowWorkDoneProgressCancelHandler()) + handlers["window/workDoneProgress/cancel"] = enrich(windowWorkDoneProgressCancelHandler(conf)) handlers["workspace/executeCommand"] = enrich(executeCommandHandler(srv)) handlers["$/cancelRequest"] = cancelRequestHandler(srv) } @@ -1176,10 +1176,12 @@ func textDocumentHover() jrpc2.Handler { }) } -func windowWorkDoneProgressCancelHandler() jrpc2.Handler { +func windowWorkDoneProgressCancelHandler(conf configuration.Configuration) jrpc2.Handler { return handler.New(func(ctx context.Context, params types.WorkdoneProgressCancelParams) (any, error) { ctx2.LoggerFromContext(ctx).Debug().Str("method", "WindowWorkDoneProgressCancelHandler").Interface("params", params).Msg("RECEIVING") progress.Cancel(params.Token) + scanAgg, _ := scanStateAggregatorFromContext(ctx) + resetSummaryPanelOnStopScan(scanAgg, conf) return nil, nil }) } From 89251fbe8dfd3154c1c411145e67f18e13b8b81c Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Fri, 5 Jun 2026 18:01:30 +0200 Subject: [PATCH 02/11] =?UTF-8?q?fix:=20correct=20misspelling=20cancelled?= =?UTF-8?q?=20=E2=86=92=20canceled=20in=20test=20comment=20[IDE-1035]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lint catch: golangci-lint misspell wants the American spelling. --- application/server/configuration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/server/configuration_test.go b/application/server/configuration_test.go index b48ec1aeb..62f49a1c1 100644 --- a/application/server/configuration_test.go +++ b/application/server/configuration_test.go @@ -2813,7 +2813,7 @@ func TestApplyUserSettingsPath_IgnoresUnchanged(t *testing.T) { // IDE-1035: when the user presses the stop-scan button the IDE sends a // window/workDoneProgress/cancel notification. The summary panel must be reset -// to its initial state so it no longer shows the counts from the cancelled scan. +// to its initial state so it no longer shows the counts from the canceled scan. func TestResetSummaryPanelOnStopScan_CallsInit(t *testing.T) { engine, _ := testutil.UnitTestWithEngine(t) conf := engine.GetConfiguration() From b6005edbd3845a9ee3d15ee2d5300ee550fa7bb8 Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Mon, 8 Jun 2026 15:24:53 +0200 Subject: [PATCH 03/11] fix(server): gate stop-scan reset to scan tokens; converge after goroutines exit [IDE-1035] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #1324 review (D, E, F, G): D) The window/workDoneProgress/cancel handler reset the summary panel for every cancel, including CLI install/download trackers, wiping valid scan results. Introduce progress.NewScanTracker / IsScanToken so the handler can gate the reset on the token being a scan token. Update Code, IaC, OSS, and the code-client tracker factory to use the new constructor. E) The reset raced in-flight scan goroutines: progress.Cancel only signals over a channel, and a late SetScanDone could land after the synchronous Init reset, re-populating the panel. Move the reset into the scan lifecycle: DelegatingConcurrentScanner.RegisterCancelCallback stores a per-folder reset fn that Scan() consumes (LoadAndDelete) after both wait groups return — guaranteeing SetScanDone has already happened. New outcome-level test TestScan_CancelCallback_CalledAfterGoroutinesFinish asserts SetScanDone appears before the reset (Init) in the call sequence. F) The cancel handler discarded the `ok` from scanStateAggregatorFromContext, silently skipping the reset if the aggregator was missing. Log a Warn so wiring failures are observable (panic would be too aggressive for an LSP notification handler). G) resetSummaryPanelForOrgChange is now shared by org-change and stop-scan. Rename to resetSummaryPanel (and update callers/comments) so the name matches the responsibility. Tests: - TestIsScanToken_{ScanTracker,PlainTracker,Unknown,AfterCancel}_* - TestCancelProgress_NonScanToken_DoesNotResetAggregator - TestScan_CancelCallback_CalledAfterGoroutinesFinish - Renamed TestResetSummaryPanel_* (G) --- application/server/configuration.go | 22 ++-- application/server/configuration_test.go | 23 ++-- application/server/notification_test.go | 34 ++++++ application/server/server.go | 41 ++++++- domain/snyk/scanner/scanner.go | 27 +++++ domain/snyk/scanner/scanner_test.go | 132 +++++++++++++++++++++++ infrastructure/code/code.go | 2 +- infrastructure/code/code_tracker.go | 2 +- infrastructure/iac/iac.go | 2 +- infrastructure/oss/cli_scanner.go | 2 +- internal/progress/progress.go | 44 ++++++++ internal/progress/progress_test.go | 26 +++++ 12 files changed, 329 insertions(+), 28 deletions(-) diff --git a/application/server/configuration.go b/application/server/configuration.go index 076eee1fd..1c062e774 100644 --- a/application/server/configuration.go +++ b/application/server/configuration.go @@ -371,7 +371,7 @@ func displayNameFor(fm workflow.ConfigurationOptionsMetaData, name string) strin // processConfigSettings writes incoming settings to configuration and applies side effects. // This replaces the old writeSettings + update* functions. // Returns globalOrgChanged so the caller (processFolderConfigs) can fold the -// global reset into the single resetSummaryPanelForOrgChange call that covers +// global reset into the single resetSummaryPanel call that covers // per-folder org changes, plus the names of any machine-scope settings rejected // for being locked. The caller is responsible for emitting a single // deduplicated locked-fields notification for the triggering event (see @@ -453,7 +453,7 @@ func hasFilterChangesInLspConfig(lspConfig *types.LspFolderConfig) bool { // processFolderConfigs handles the folder configuration portion of incoming settings. // When globalOrgChanged is true, every workspace folder is treated as having an // org change so the Summary Panel reset is folded into the single -// resetSummaryPanelForOrgChange call below (avoiding the double-flash that used +// resetSummaryPanel call below (avoiding the double-flash that used // to occur when applyOrganization reset separately from processFolderConfigs). // // Returns the union of all folder-scope setting names rejected for being locked @@ -528,7 +528,7 @@ func processFolderConfigs(ctx context.Context, conf configuration.Configuration, for p := range orgChangedFolderPaths { paths = append(paths, p) } - resetSummaryPanelForOrgChange(mustScanStateAggregatorFromContext(ctx), paths) + resetSummaryPanel(mustScanStateAggregatorFromContext(ctx), paths) } sendFolderConfigUpdateIfNeeded(conf, engine, logger, notifier, processedConfigs, len(changedConfigs) > 0, triggerSource, configResolver) @@ -849,7 +849,7 @@ func applyAutoScan(conf configuration.Configuration, settings map[string]*types. // applyOrganization persists the global org change and emits analytics. // Returns true when the global org actually changed and the LSP is initialized // so the caller (processConfigSettings → processFolderConfigs) can union the -// affected workspace folders into the single resetSummaryPanelForOrgChange call. +// affected workspace folders into the single resetSummaryPanel call. func applyOrganization(conf configuration.Configuration, engine workflow.Engine, logger *zerolog.Logger, settings map[string]*types.ConfigSetting, triggerSource analytics.TriggerSource, configResolver types.ConfigResolverInterface) bool { v, ok := settingStr(settings, types.SettingOrganization) if !ok { @@ -866,10 +866,11 @@ func applyOrganization(conf configuration.Configuration, engine workflow.Engine, return true } -// resetSummaryPanelForOrgChange clears scan state for the given folders so the -// Summary Panel returns to its initial "no scans yet" state. The aggregator's -// Init re-emits a fresh snapshot, which IDE clients render as the empty summary panel. -func resetSummaryPanelForOrgChange(scanAgg scanstates.Aggregator, folderPaths []types.FilePath) { +// resetSummaryPanel clears scan state for the given folders so the Summary +// Panel returns to its initial "no scans yet" state. The aggregator's Init +// re-emits a fresh snapshot, which IDE clients render as the empty summary +// panel. Used by both org-change and stop-scan paths. +func resetSummaryPanel(scanAgg scanstates.Aggregator, folderPaths []types.FilePath) { if scanAgg == nil || len(folderPaths) == 0 { return } @@ -879,10 +880,9 @@ func resetSummaryPanelForOrgChange(scanAgg scanstates.Aggregator, folderPaths [] // resetSummaryPanelOnStopScan resets the summary panel to its initial state // when the user cancels a scan via the IDE stop-scan button (IDE-1035). // It resolves current workspace folder paths from conf and delegates to -// resetSummaryPanelForOrgChange, which guards against nil aggregator and -// empty folder list. +// resetSummaryPanel, which guards against nil aggregator and empty folder list. func resetSummaryPanelOnStopScan(scanAgg scanstates.Aggregator, conf configuration.Configuration) { - resetSummaryPanelForOrgChange(scanAgg, workspaceFolderPaths(conf)) + resetSummaryPanel(scanAgg, workspaceFolderPaths(conf)) } func workspaceFolderPaths(conf configuration.Configuration) []types.FilePath { diff --git a/application/server/configuration_test.go b/application/server/configuration_test.go index 62f49a1c1..6f2af893a 100644 --- a/application/server/configuration_test.go +++ b/application/server/configuration_test.go @@ -2436,34 +2436,37 @@ func Test_updateFolderConfig_PreferredOrgChange_ResetsSummaryPanelOnOrgChange(t } // initRecordingAggregator wraps NoopStateAggregator to count Init calls so we -// can assert that the guard branches in resetSummaryPanelForOrgChange do not +// can assert that the guard branches in resetSummaryPanel do not // invoke Init when there are no folder paths to reset. type initRecordingAggregator struct { scanstates.NoopStateAggregator + mu sync.Mutex initCalls [][]types.FilePath } func (r *initRecordingAggregator) Init(folders []types.FilePath) { cp := make([]types.FilePath, len(folders)) copy(cp, folders) + r.mu.Lock() + defer r.mu.Unlock() r.initCalls = append(r.initCalls, cp) } -// IDE-1969: resetSummaryPanelForOrgChange has two early-return guards (nil -// aggregator, empty folder paths). Both must be safe — the nil case happens -// before di.Init wires the aggregator, and the empty case happens when an -// org change is reported for a workspace with no folders. -func TestResetSummaryPanelForOrgChange_NilAgg_DoesNotPanic(t *testing.T) { +// IDE-1969: resetSummaryPanel has two early-return guards (nil aggregator, +// empty folder paths). Both must be safe — the nil case happens before +// di.Init wires the aggregator, and the empty case happens when an org change +// is reported for a workspace with no folders. +func TestResetSummaryPanel_NilAgg_DoesNotPanic(t *testing.T) { assert.NotPanics(t, func() { - resetSummaryPanelForOrgChange(nil, []types.FilePath{"/some/folder"}) + resetSummaryPanel(nil, []types.FilePath{"/some/folder"}) }) } -func TestResetSummaryPanelForOrgChange_EmptyFolderPaths_DoesNotCallInit(t *testing.T) { +func TestResetSummaryPanel_EmptyFolderPaths_DoesNotCallInit(t *testing.T) { agg := &initRecordingAggregator{} assert.NotPanics(t, func() { - resetSummaryPanelForOrgChange(agg, nil) - resetSummaryPanelForOrgChange(agg, []types.FilePath{}) + resetSummaryPanel(agg, nil) + resetSummaryPanel(agg, []types.FilePath{}) }) assert.Empty(t, agg.initCalls, "Init must not be called when folderPaths is empty") } diff --git a/application/server/notification_test.go b/application/server/notification_test.go index 8c404a5ce..c319edfd4 100644 --- a/application/server/notification_test.go +++ b/application/server/notification_test.go @@ -27,12 +27,14 @@ import ( "github.com/creachadair/jrpc2" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/snyk/snyk-ls/application/di" "github.com/snyk/snyk-ls/domain/ide/command" "github.com/snyk/snyk-ls/internal/data_structure" "github.com/snyk/snyk-ls/internal/progress" "github.com/snyk/snyk-ls/internal/testutil" + "github.com/snyk/snyk-ls/internal/testutil/workspaceutil" "github.com/snyk/snyk-ls/internal/types" "github.com/snyk/snyk-ls/internal/types/mock_types" ) @@ -161,6 +163,38 @@ func TestCancelProgress(t *testing.T) { }, time.Second*5, time.Millisecond) } +// IDE-1035 (D): window/workDoneProgress/cancel for a non-scan token (e.g. a +// download tracker) must NOT reset the summary panel. The positive case (scan +// token → panel is eventually reset) is exercised by +// TestScan_CancelCallback_CalledAfterGoroutinesFinish in the scanner package, +// which verifies the reset happens only after scan goroutines finish writing. +func TestCancelProgress_NonScanToken_DoesNotResetAggregator(t *testing.T) { + engine, tokenService := testutil.UnitTestWithEngine(t) + + agg := &initRecordingAggregator{} + loc, _, _ := setupServer(t, engine, tokenService, WithDeps(di.Dependencies{ScanStateAggregator: agg})) + + _, err := loc.Client.Call(t.Context(), "initialize", nil) + require.NoError(t, err) + + // Seed a workspace folder. + tmpDir := types.FilePath(t.TempDir()) + _, _ = workspaceutil.SetupWorkspace(t, engine, tmpDir) + + // Create a plain (non-scan) tracker, e.g. for a download, and cancel it. + plainTracker := progress.NewTracker(true, engine.GetLogger()) + cancelParams := types.WorkdoneProgressCancelParams{Token: plainTracker.GetToken()} + _, err = loc.Client.Call(t.Context(), "window/workDoneProgress/cancel", cancelParams) + require.NoError(t, err) + + // Give the handler time to execute; Init must NOT be called. + assert.Never(t, func() bool { + agg.mu.Lock() + defer agg.mu.Unlock() + return len(agg.initCalls) > 0 + }, 300*time.Millisecond, time.Millisecond, "Init must NOT be called when a non-scan token is cancelled") +} + func Test_NotifierShouldSendNotificationToClient(t *testing.T) { engine, tokenService := testutil.UnitTestWithEngine(t) loc, jsonRPCRecorder, _ := setupServer(t, engine, tokenService) diff --git a/application/server/server.go b/application/server/server.go index 1a73591f7..2d6a13c51 100644 --- a/application/server/server.go +++ b/application/server/server.go @@ -1178,10 +1178,45 @@ func textDocumentHover() jrpc2.Handler { func windowWorkDoneProgressCancelHandler(conf configuration.Configuration) jrpc2.Handler { return handler.New(func(ctx context.Context, params types.WorkdoneProgressCancelParams) (any, error) { - ctx2.LoggerFromContext(ctx).Debug().Str("method", "WindowWorkDoneProgressCancelHandler").Interface("params", params).Msg("RECEIVING") + logger := ctx2.LoggerFromContext(ctx) + logger.Debug().Str("method", "WindowWorkDoneProgressCancelHandler").Interface("params", params).Msg("RECEIVING") + + // Only reset the summary panel for scan cancellations. Generic progress + // tokens (e.g. CLI download/install) must not wipe valid scan results. + isScan := progress.IsScanToken(params.Token) progress.Cancel(params.Token) - scanAgg, _ := scanStateAggregatorFromContext(ctx) - resetSummaryPanelOnStopScan(scanAgg, conf) + + if isScan { + scanAgg, ok := scanStateAggregatorFromContext(ctx) + if !ok || scanAgg == nil { + logger.Warn().Str("method", "WindowWorkDoneProgressCancelHandler"). + Msg("scan state aggregator not found in context; summary panel will not be reset") + return nil, nil + } + // Register the reset callback on the scanner so it fires AFTER all + // per-product goroutines have finished their SetScanDone writes (E, + // IDE-1035). The reset is keyed to each workspace folder so that + // concurrent folder scans reset independently. + sc, scOk := scannerFromContext(ctx) + if scOk { + if dcs, ok := sc.(*scanner2.DelegatingConcurrentScanner); ok { + folderPaths := workspaceFolderPaths(conf) + for _, fp := range folderPaths { + fp := fp + dcs.RegisterCancelCallback(fp, func() { + resetSummaryPanel(scanAgg, []types.FilePath{fp}) + }) + } + return nil, nil + } + } + // Fallback: scanner not available in context — reset synchronously. + // This path is only hit in tests or during early startup where the + // scanner is not yet wired into the context. + logger.Debug().Str("method", "WindowWorkDoneProgressCancelHandler"). + Msg("scanner not in context; resetting summary panel synchronously") + resetSummaryPanel(scanAgg, workspaceFolderPaths(conf)) + } return nil, nil }) } diff --git a/domain/snyk/scanner/scanner.go b/domain/snyk/scanner/scanner.go index 6dcdc4440..4f4f25e03 100644 --- a/domain/snyk/scanner/scanner.go +++ b/domain/snyk/scanner/scanner.go @@ -38,6 +38,12 @@ import ( "github.com/snyk/snyk-ls/internal/vcs" ) +// cancelCallbacks maps folderPath → reset function registered by the LSP +// cancel handler (IDE-1035). Callbacks are consumed (and removed) by Scan() +// after all per-product goroutines have finished, ensuring the reset happens +// only once and only after all SetScanDone writes are complete. +type cancelCallbackMap = sync.Map + var ( _ Scanner = (*DelegatingConcurrentScanner)(nil) _ snyk.InlineValueProvider = (*DelegatingConcurrentScanner)(nil) @@ -63,6 +69,18 @@ type DelegatingConcurrentScanner struct { scanStateAggregator scanstates.Aggregator snykApiClient snyk_api.SnykApiClient configResolver types.ConfigResolverInterface + // cancelCallbacks stores per-folder reset functions registered by the LSP + // cancel handler. Consumed by Scan() after all goroutines exit (IDE-1035). + cancelCallbacks cancelCallbackMap +} + +// RegisterCancelCallback stores fn to be called by Scan() for folderPath once +// all per-product goroutines have exited after a user-initiated cancellation. +// Only one callback per folder is kept; a second call for the same folder +// overwrites the previous one. This is intentional: the handler is only ever +// called once per active cancel notification. +func (sc *DelegatingConcurrentScanner) RegisterCancelCallback(folderPath types.FilePath, fn func()) { + sc.cancelCallbacks.Store(folderPath, fn) } func (sc *DelegatingConcurrentScanner) Issue(key string) types.Issue { @@ -358,6 +376,15 @@ func (sc *DelegatingConcurrentScanner) Scan(ctx context.Context, pathToScan type waitGroup.Wait() referenceBranchScanWaitGroup.Wait() + // After all per-product goroutines have exited (including their SetScanDone + // writes), invoke any cancel callback registered by the LSP cancel handler + // for this folder (IDE-1035). Consuming the callback here — after the + // WaitGroups — guarantees the aggregator reset cannot race with in-flight + // SetScanDone calls. + if fn, loaded := sc.cancelCallbacks.LoadAndDelete(folderPath); loaded { + fn.(func())() + } + defer func() { if postActionFunc != nil { postActionFunc() diff --git a/domain/snyk/scanner/scanner_test.go b/domain/snyk/scanner/scanner_test.go index 46857fd1d..71c2e30ec 100644 --- a/domain/snyk/scanner/scanner_test.go +++ b/domain/snyk/scanner/scanner_test.go @@ -18,6 +18,7 @@ package scanner import ( "context" + "sync" "testing" "time" @@ -222,6 +223,137 @@ func Test_ScanStarted_TokenChanged_ScanCancelled(t *testing.T) { assert.True(t, wasCanceled, "Scan should have been canceled when token changed") } +// IDE-1035 (E): Outcome-level test — after user-initiated stop-scan during an +// in-flight scan, the cancel callback registered via RegisterCancelCallback +// must run AFTER all per-product goroutines have finished writing to the +// aggregator, so the final snapshot is the initial (NotStarted) state. +func TestScan_CancelCallback_CalledAfterGoroutinesFinish(t *testing.T) { + engine, tokenService := testutil.UnitTestWithEngine(t) + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + tokenService.SetToken(engine.GetConfiguration(), uuid.New().String()) + + scanStarted := make(chan struct{}) + + mockProductScanner := mock_types.NewMockProductScanner(ctrl) + mockProductScanner.EXPECT().Product().Return(product.ProductOpenSource).AnyTimes() + mockProductScanner.EXPECT().IsEnabledForFolder(gomock.Any()).Return(true).AnyTimes() + mockProductScanner.EXPECT().Scan(gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, _ types.FilePath) ([]types.Issue, error) { + close(scanStarted) + // Block until context is cancelled (simulates in-flight cancellation). + <-ctx.Done() + return []types.Issue{}, nil + }).Times(1) + + // Track call order to verify SetScanDone precedes the cancel callback. + type callRecord struct{ kind string } + var callsMu sync.Mutex + var calls []callRecord + + agg := &recordingOrderAggregator{ + SetScanDoneFn: func() { + callsMu.Lock() + calls = append(calls, callRecord{"SetScanDone"}) + callsMu.Unlock() + }, + } + + resolver := defaultResolver(t, engine) + sc, _ := setupScannerWithResolverAndAgg(t, engine, tokenService, resolver, agg, mockProductScanner) + + folderPath := types.FilePath(t.TempDir()) + resetCalled := make(chan struct{}, 1) + sc.(*DelegatingConcurrentScanner).RegisterCancelCallback(folderPath, func() { + callsMu.Lock() + calls = append(calls, callRecord{"Init"}) + callsMu.Unlock() + resetCalled <- struct{}{} + }) + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + done := make(chan struct{}) + go func() { + fc := &types.FolderConfig{FolderPath: folderPath} + scanCtx := ctx2.NewContextWithFolderConfig(ctx, fc) + sc.Scan(scanCtx, folderPath, types.NoopResultProcessor, nil) + close(done) + }() + + // Wait for scan to start, then cancel the outer context. + select { + case <-scanStarted: + case <-time.After(5 * time.Second): + t.Fatal("scan did not start in time") + } + cancel() + + // Wait for Scan() to return. + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("Scan did not return in time after cancel") + } + + // The cancel callback must have been called. + select { + case <-resetCalled: + case <-time.After(time.Second): + t.Fatal("cancel callback was not called after Scan returned") + } + + // Verify order: SetScanDone must appear before Init. + callsMu.Lock() + defer callsMu.Unlock() + require.GreaterOrEqual(t, len(calls), 2, "expected at least SetScanDone then Init") + // Find Init call position + var initIdx, setDoneIdx int = -1, -1 + for i, c := range calls { + if c.kind == "SetScanDone" && setDoneIdx == -1 { + setDoneIdx = i + } + if c.kind == "Init" { + initIdx = i + } + } + require.NotEqual(t, -1, setDoneIdx, "SetScanDone must be called") + require.NotEqual(t, -1, initIdx, "Init must be called") + assert.Less(t, setDoneIdx, initIdx, "SetScanDone must be called before Init (no late writes after reset)") +} + +// recordingOrderAggregator wraps NoopStateAggregator and calls SetScanDoneFn on SetScanDone. +type recordingOrderAggregator struct { + scanstates.NoopStateAggregator + SetScanDoneFn func() +} + +func (r *recordingOrderAggregator) SetScanDone(fp types.FilePath, p product.Product, ref bool, err error) { + r.NoopStateAggregator.SetScanDone(fp, p, ref, err) + if r.SetScanDoneFn != nil { + r.SetScanDoneFn() + } +} + +func setupScannerWithResolverAndAgg(t *testing.T, engine workflow.Engine, tokenService types.TokenService, configResolver types.ConfigResolverInterface, agg scanstates.Aggregator, testProductScanners ...types.ProductScanner) ( + sc Scanner, + scanNotifier ScanNotifier, +) { + t.Helper() + scanNotifier = NewMockScanNotifier() + notifier := notification.NewNotifier() + apiClient := &snyk_api.FakeApiClient{CodeEnabled: false} + persister := persistence.NewNopScanPersister() + er := error_reporting.NewTestErrorReporter(engine) + authenticationProvider := authentication.NewFakeCliAuthenticationProvider(engine) + authenticationProvider.IsAuthenticated = true + authenticationService := authentication.NewAuthenticationService(engine, tokenService, authenticationProvider, er, notifier, configResolver) + sc = NewDelegatingScanner(engine, tokenService, initialize.NewDelegatingInitializer(), performance.NewInstrumentor(), scanNotifier, apiClient, authenticationService, notifier, persister, agg, configResolver, testProductScanners...) + return sc, scanNotifier +} + func TestScan_whenProductScannerEnabled_SendsInProgress(t *testing.T) { engine, tokenService := testutil.UnitTestWithEngine(t) ctrl := gomock.NewController(t) diff --git a/infrastructure/code/code.go b/infrastructure/code/code.go index e8dd5b1fe..0e7d9521b 100644 --- a/infrastructure/code/code.go +++ b/infrastructure/code/code.go @@ -264,7 +264,7 @@ func internalScan(ctx context.Context, sc *Scanner, folderPath types.FilePath, l Int("fileCount", len(filesToBeScanned)). Msg("Code scanner: files to be scanned") - t := progress.NewTracker(true, sc.engine.GetLogger()) + t := progress.NewScanTracker(true, sc.engine.GetLogger()) go func() { t.CancelOrDone(cancel, ctx.Done()) }() t.BeginWithMessage(string("Snyk Code: scanning "+folderPath), "starting scan") diff --git a/infrastructure/code/code_tracker.go b/infrastructure/code/code_tracker.go index cf941fbde..1b02db09b 100644 --- a/infrastructure/code/code_tracker.go +++ b/infrastructure/code/code_tracker.go @@ -37,7 +37,7 @@ func NewCodeTrackerFactory(logger *zerolog.Logger) codeClientScan.TrackerFactory } func (t trackerFactory) GenerateTracker() codeClientScan.Tracker { - newTracker := progress.NewTracker(true, t.logger) + newTracker := progress.NewScanTracker(true, t.logger) return newCodeTracker(newTracker.GetChannel(), newTracker.GetCancelChannel()) } diff --git a/infrastructure/iac/iac.go b/infrastructure/iac/iac.go index 33e87464d..c318142fb 100644 --- a/infrastructure/iac/iac.go +++ b/infrastructure/iac/iac.go @@ -154,7 +154,7 @@ func (iac *Scanner) Scan(ctx context.Context, pathToScan types.FilePath) (issues logger.Debug().Msg("IaC scan skipped: path is not a supported IaC file or directory") return []types.Issue{}, nil } - p := progress.NewTracker(true, iac.logger) + p := progress.NewScanTracker(true, iac.logger) go func() { p.CancelOrDone(cancel, ctx.Done()) }() p.BeginUnquantifiableLength("Scanning for Snyk IaC issues", string(pathToScan)) defer p.EndWithMessage("Snyk Iac Scan completed.") diff --git a/infrastructure/oss/cli_scanner.go b/infrastructure/oss/cli_scanner.go index 3d17136d1..32b2103a7 100644 --- a/infrastructure/oss/cli_scanner.go +++ b/infrastructure/oss/cli_scanner.go @@ -248,7 +248,7 @@ func (cliScanner *CLIScanner) scanInternal(ctx context.Context, commandFunc func ctx, cancel := context.WithCancel(s.Context()) defer cancel() - p := progress.NewTracker(true, cliScanner.engine.GetLogger()) + p := progress.NewScanTracker(true, cliScanner.engine.GetLogger()) go func() { p.CancelOrDone(cancel, ctx.Done()) }() p.BeginUnquantifiableLength("Scanning for Snyk Open Source issues", string(path)) defer p.EndWithMessage("Snyk Open Source scan completed.") diff --git a/internal/progress/progress.go b/internal/progress/progress.go index 15d47f03f..8bc75ade3 100644 --- a/internal/progress/progress.go +++ b/internal/progress/progress.go @@ -33,6 +33,15 @@ import ( var trackersMutex sync.RWMutex var trackers = make(map[types.ProgressToken]*Tracker) var ToServerProgressChannel = make(chan types.ProgressParams, 1000) + +// scanTokensMutex guards scanTokens. +var scanTokensMutex sync.RWMutex + +// scanTokens is the set of progress tokens that belong to scan operations. +// Tokens created via NewScanTracker are recorded here so that the +// window/workDoneProgress/cancel handler can distinguish scan cancellations +// from download/install cancellations (IDE-1035). +var scanTokens = make(map[types.ProgressToken]struct{}) var _ ui.ProgressBar = (*Tracker)(nil) type Tracker struct { @@ -79,6 +88,27 @@ func NewTracker(cancellable bool, logger *zerolog.Logger) *Tracker { return t } +// NewScanTracker creates a progress tracker and registers its token as a scan +// token so that IsScanToken returns true for it. Use this for scan operations +// (OSS, Code, IaC) instead of NewTracker. Download/install operations must +// continue to use NewTracker so their cancel events do not reset the summary +// panel (IDE-1035). +func NewScanTracker(cancellable bool, logger *zerolog.Logger) *Tracker { + t := NewTracker(cancellable, logger) + scanTokensMutex.Lock() + scanTokens[t.token] = struct{}{} + scanTokensMutex.Unlock() + return t +} + +// IsScanToken reports whether token was created via NewScanTracker. +func IsScanToken(token types.ProgressToken) bool { + scanTokensMutex.RLock() + _, ok := scanTokens[token] + scanTokensMutex.RUnlock() + return ok +} + func (t *Tracker) GetChannel() chan types.ProgressParams { return t.channel } @@ -236,6 +266,9 @@ func (t *Tracker) deleteTracker() { trackersMutex.Lock() delete(trackers, t.token) trackersMutex.Unlock() + scanTokensMutex.Lock() + delete(scanTokens, t.token) + scanTokensMutex.Unlock() } func (t *Tracker) GetToken() types.ProgressToken { @@ -300,6 +333,12 @@ func CleanupChannels() { for token := range tempTrackers { Cancel(token) } + + // Clear any remaining scan tokens that may have been deregistered from the + // tracker registry already (e.g. via deleteTracker) but not yet cleaned up. + scanTokensMutex.Lock() + scanTokens = make(map[types.ProgressToken]struct{}) + scanTokensMutex.Unlock() } func (t *Tracker) IsCanceled() bool { @@ -315,6 +354,11 @@ func Cancel(token types.ProgressToken) { delete(trackers, token) close(t.cancelChannel) } + // Remove from scan-token set regardless of whether a tracker was found, + // so IsScanToken is consistent after cancellation. + scanTokensMutex.Lock() + delete(scanTokens, token) + scanTokensMutex.Unlock() } func IsCanceled(token types.ProgressToken) bool { diff --git a/internal/progress/progress_test.go b/internal/progress/progress_test.go index 68ec3b051..98cc7c64c 100644 --- a/internal/progress/progress_test.go +++ b/internal/progress/progress_test.go @@ -86,6 +86,32 @@ func TestEndProgress(t *testing.T) { assert.Equal(t, output, <-channel) } +// IDE-1035 (D): NewScanTracker must register the token so IsScanToken returns +// true; a plain NewTracker must NOT be classified as a scan token. +func TestIsScanToken_ScanTracker_ReturnsTrue(t *testing.T) { + logger := zerolog.Nop() + tr := NewScanTracker(true, &logger) + assert.True(t, IsScanToken(tr.GetToken()), "NewScanTracker token must be recognised as a scan token") +} + +func TestIsScanToken_PlainTracker_ReturnsFalse(t *testing.T) { + logger := zerolog.Nop() + tr := NewTracker(true, &logger) + assert.False(t, IsScanToken(tr.GetToken()), "NewTracker token must NOT be recognised as a scan token") +} + +func TestIsScanToken_UnknownToken_ReturnsFalse(t *testing.T) { + assert.False(t, IsScanToken("unknown-token"), "unknown token must NOT be recognised as a scan token") +} + +func TestIsScanToken_AfterCancel_ReturnsFalse(t *testing.T) { + logger := zerolog.Nop() + tr := NewScanTracker(true, &logger) + token := tr.GetToken() + Cancel(token) + assert.False(t, IsScanToken(token), "canceled scan token must no longer be recognised as a scan token") +} + func TestEndProgressTwice(t *testing.T) { output := types.ProgressParams{ Value: types.WorkDoneProgressEnd{ From 7dc9f44f825f0fa1cf7d3b24792171317ace3191 Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Mon, 8 Jun 2026 19:36:23 +0200 Subject: [PATCH 04/11] style(server,scanner): address golangci-lint feedback [IDE-1035] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - server.go: drop pre-Go-1.22 `fp := fp` (copyloopvar) - scanner.go: extract consumeCancelCallback so Scan's cyclomatic complexity stays under 15 (gocyclo); checked type assertion instead of `fn.(func())()` (forcetypeassert) - scanner.go: drop var-decl-with-type (`var a, b int = -1, -1` → `a, b := -1, -1`) in the new outcome test (staticcheck QF1011) - notification_test.go, scanner_test.go, progress_test.go: cancelled→canceled, recognised→recognized (misspell, repo prefers en_US) --- application/server/notification_test.go | 2 +- application/server/server.go | 1 - domain/snyk/scanner/scanner.go | 19 ++++++++++++++++--- domain/snyk/scanner/scanner_test.go | 4 ++-- internal/progress/progress_test.go | 8 ++++---- 5 files changed, 23 insertions(+), 11 deletions(-) diff --git a/application/server/notification_test.go b/application/server/notification_test.go index c319edfd4..9aad02c30 100644 --- a/application/server/notification_test.go +++ b/application/server/notification_test.go @@ -192,7 +192,7 @@ func TestCancelProgress_NonScanToken_DoesNotResetAggregator(t *testing.T) { agg.mu.Lock() defer agg.mu.Unlock() return len(agg.initCalls) > 0 - }, 300*time.Millisecond, time.Millisecond, "Init must NOT be called when a non-scan token is cancelled") + }, 300*time.Millisecond, time.Millisecond, "Init must NOT be called when a non-scan token is canceled") } func Test_NotifierShouldSendNotificationToClient(t *testing.T) { diff --git a/application/server/server.go b/application/server/server.go index 2d6a13c51..b96ba5f7a 100644 --- a/application/server/server.go +++ b/application/server/server.go @@ -1202,7 +1202,6 @@ func windowWorkDoneProgressCancelHandler(conf configuration.Configuration) jrpc2 if dcs, ok := sc.(*scanner2.DelegatingConcurrentScanner); ok { folderPaths := workspaceFolderPaths(conf) for _, fp := range folderPaths { - fp := fp dcs.RegisterCancelCallback(fp, func() { resetSummaryPanel(scanAgg, []types.FilePath{fp}) }) diff --git a/domain/snyk/scanner/scanner.go b/domain/snyk/scanner/scanner.go index 4f4f25e03..403d193af 100644 --- a/domain/snyk/scanner/scanner.go +++ b/domain/snyk/scanner/scanner.go @@ -83,6 +83,21 @@ func (sc *DelegatingConcurrentScanner) RegisterCancelCallback(folderPath types.F sc.cancelCallbacks.Store(folderPath, fn) } +// consumeCancelCallback fires and removes any cancel callback registered for +// folderPath. Called by Scan() after both WaitGroups return so the callback +// runs only after all SetScanDone writes have completed (IDE-1035). +func (sc *DelegatingConcurrentScanner) consumeCancelCallback(folderPath types.FilePath) { + v, loaded := sc.cancelCallbacks.LoadAndDelete(folderPath) + if !loaded { + return + } + fn, ok := v.(func()) + if !ok { + return + } + fn() +} + func (sc *DelegatingConcurrentScanner) Issue(key string) types.Issue { for _, scanner := range sc.scanners { if s, ok := scanner.(snyk.IssueProvider); ok { @@ -381,9 +396,7 @@ func (sc *DelegatingConcurrentScanner) Scan(ctx context.Context, pathToScan type // for this folder (IDE-1035). Consuming the callback here — after the // WaitGroups — guarantees the aggregator reset cannot race with in-flight // SetScanDone calls. - if fn, loaded := sc.cancelCallbacks.LoadAndDelete(folderPath); loaded { - fn.(func())() - } + sc.consumeCancelCallback(folderPath) defer func() { if postActionFunc != nil { diff --git a/domain/snyk/scanner/scanner_test.go b/domain/snyk/scanner/scanner_test.go index 71c2e30ec..61684a799 100644 --- a/domain/snyk/scanner/scanner_test.go +++ b/domain/snyk/scanner/scanner_test.go @@ -242,7 +242,7 @@ func TestScan_CancelCallback_CalledAfterGoroutinesFinish(t *testing.T) { mockProductScanner.EXPECT().Scan(gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, _ types.FilePath) ([]types.Issue, error) { close(scanStarted) - // Block until context is cancelled (simulates in-flight cancellation). + // Block until context is canceled (simulates in-flight cancellation). <-ctx.Done() return []types.Issue{}, nil }).Times(1) @@ -310,7 +310,7 @@ func TestScan_CancelCallback_CalledAfterGoroutinesFinish(t *testing.T) { defer callsMu.Unlock() require.GreaterOrEqual(t, len(calls), 2, "expected at least SetScanDone then Init") // Find Init call position - var initIdx, setDoneIdx int = -1, -1 + initIdx, setDoneIdx := -1, -1 for i, c := range calls { if c.kind == "SetScanDone" && setDoneIdx == -1 { setDoneIdx = i diff --git a/internal/progress/progress_test.go b/internal/progress/progress_test.go index 98cc7c64c..7ac05e4cf 100644 --- a/internal/progress/progress_test.go +++ b/internal/progress/progress_test.go @@ -91,17 +91,17 @@ func TestEndProgress(t *testing.T) { func TestIsScanToken_ScanTracker_ReturnsTrue(t *testing.T) { logger := zerolog.Nop() tr := NewScanTracker(true, &logger) - assert.True(t, IsScanToken(tr.GetToken()), "NewScanTracker token must be recognised as a scan token") + assert.True(t, IsScanToken(tr.GetToken()), "NewScanTracker token must be recognized as a scan token") } func TestIsScanToken_PlainTracker_ReturnsFalse(t *testing.T) { logger := zerolog.Nop() tr := NewTracker(true, &logger) - assert.False(t, IsScanToken(tr.GetToken()), "NewTracker token must NOT be recognised as a scan token") + assert.False(t, IsScanToken(tr.GetToken()), "NewTracker token must NOT be recognized as a scan token") } func TestIsScanToken_UnknownToken_ReturnsFalse(t *testing.T) { - assert.False(t, IsScanToken("unknown-token"), "unknown token must NOT be recognised as a scan token") + assert.False(t, IsScanToken("unknown-token"), "unknown token must NOT be recognized as a scan token") } func TestIsScanToken_AfterCancel_ReturnsFalse(t *testing.T) { @@ -109,7 +109,7 @@ func TestIsScanToken_AfterCancel_ReturnsFalse(t *testing.T) { tr := NewScanTracker(true, &logger) token := tr.GetToken() Cancel(token) - assert.False(t, IsScanToken(token), "canceled scan token must no longer be recognised as a scan token") + assert.False(t, IsScanToken(token), "canceled scan token must no longer be recognized as a scan token") } func TestEndProgressTwice(t *testing.T) { From 9c73dc20c353715c677617b3c8470a800649be77 Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Wed, 10 Jun 2026 13:55:52 +0200 Subject: [PATCH 05/11] fix(server): defer progress.Cancel so RegisterCancelCallback registers first [IDE-1035] Defer the cancel call in windowWorkDoneProgressCancelHandler so the RegisterCancelCallback loop runs before scan goroutines observe the cancel and drain through consumeCancelCallback. Otherwise the reset can be silently dropped and a stale callback fires on the folder's next scan. Co-Authored-By: Claude Opus 4.7 (1M context) --- application/server/server.go | 64 ++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/application/server/server.go b/application/server/server.go index b96ba5f7a..766c64b5b 100644 --- a/application/server/server.go +++ b/application/server/server.go @@ -1184,38 +1184,46 @@ func windowWorkDoneProgressCancelHandler(conf configuration.Configuration) jrpc2 // Only reset the summary panel for scan cancellations. Generic progress // tokens (e.g. CLI download/install) must not wipe valid scan results. isScan := progress.IsScanToken(params.Token) - progress.Cancel(params.Token) - if isScan { - scanAgg, ok := scanStateAggregatorFromContext(ctx) - if !ok || scanAgg == nil { - logger.Warn().Str("method", "WindowWorkDoneProgressCancelHandler"). - Msg("scan state aggregator not found in context; summary panel will not be reset") - return nil, nil - } - // Register the reset callback on the scanner so it fires AFTER all - // per-product goroutines have finished their SetScanDone writes (E, - // IDE-1035). The reset is keyed to each workspace folder so that - // concurrent folder scans reset independently. - sc, scOk := scannerFromContext(ctx) - if scOk { - if dcs, ok := sc.(*scanner2.DelegatingConcurrentScanner); ok { - folderPaths := workspaceFolderPaths(conf) - for _, fp := range folderPaths { - dcs.RegisterCancelCallback(fp, func() { - resetSummaryPanel(scanAgg, []types.FilePath{fp}) - }) - } - return nil, nil + // Defer the cancel so any RegisterCancelCallback below is in place + // before scan goroutines observe cancellation and drain through + // consumeCancelCallback (scanner.go). Otherwise the reset can be + // silently dropped, and a stale callback fires on the folder's + // next scan (IDE-1035 register-vs-consume race). + defer progress.Cancel(params.Token) + + if !isScan { + return nil, nil + } + + scanAgg, ok := scanStateAggregatorFromContext(ctx) + if !ok || scanAgg == nil { + logger.Warn().Str("method", "WindowWorkDoneProgressCancelHandler"). + Msg("scan state aggregator not found in context; summary panel will not be reset") + return nil, nil + } + // Register the reset callback on the scanner so it fires AFTER all + // per-product goroutines have finished their SetScanDone writes (E, + // IDE-1035). The reset is keyed to each workspace folder so that + // concurrent folder scans reset independently. + sc, scOk := scannerFromContext(ctx) + if scOk { + if dcs, ok := sc.(*scanner2.DelegatingConcurrentScanner); ok { + folderPaths := workspaceFolderPaths(conf) + for _, fp := range folderPaths { + dcs.RegisterCancelCallback(fp, func() { + resetSummaryPanel(scanAgg, []types.FilePath{fp}) + }) } + return nil, nil } - // Fallback: scanner not available in context — reset synchronously. - // This path is only hit in tests or during early startup where the - // scanner is not yet wired into the context. - logger.Debug().Str("method", "WindowWorkDoneProgressCancelHandler"). - Msg("scanner not in context; resetting summary panel synchronously") - resetSummaryPanel(scanAgg, workspaceFolderPaths(conf)) } + // Fallback: scanner not available in context — reset synchronously. + // This path is only hit in tests or during early startup where the + // scanner is not yet wired into the context. + logger.Debug().Str("method", "WindowWorkDoneProgressCancelHandler"). + Msg("scanner not in context; resetting summary panel synchronously") + resetSummaryPanel(scanAgg, workspaceFolderPaths(conf)) return nil, nil }) } From 1f0e9ab8b2c636cfc9039ff23e9d6e8393543094 Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Wed, 10 Jun 2026 14:11:24 +0200 Subject: [PATCH 06/11] refactor(server): delete unreferenced resetSummaryPanelOnStopScan wrapper [IDE-1035] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wrapper had no production callers — only three TestResetSummaryPanel OnStopScan_* tests exercised it. The real stop-scan handler in windowWorkDoneProgressCancelHandler calls resetSummaryPanel directly, so the wrapper plus its dedicated tests gave false coverage of a path that never ran in production. Co-Authored-By: Claude Opus 4.7 (1M context) --- application/server/configuration.go | 8 ----- application/server/configuration_test.go | 38 ------------------------ 2 files changed, 46 deletions(-) diff --git a/application/server/configuration.go b/application/server/configuration.go index 1c062e774..a2194a8b8 100644 --- a/application/server/configuration.go +++ b/application/server/configuration.go @@ -877,14 +877,6 @@ func resetSummaryPanel(scanAgg scanstates.Aggregator, folderPaths []types.FilePa scanAgg.Init(folderPaths) } -// resetSummaryPanelOnStopScan resets the summary panel to its initial state -// when the user cancels a scan via the IDE stop-scan button (IDE-1035). -// It resolves current workspace folder paths from conf and delegates to -// resetSummaryPanel, which guards against nil aggregator and empty folder list. -func resetSummaryPanelOnStopScan(scanAgg scanstates.Aggregator, conf configuration.Configuration) { - resetSummaryPanel(scanAgg, workspaceFolderPaths(conf)) -} - func workspaceFolderPaths(conf configuration.Configuration) []types.FilePath { ws := config.GetWorkspace(conf) if ws == nil { diff --git a/application/server/configuration_test.go b/application/server/configuration_test.go index 6f2af893a..389043f75 100644 --- a/application/server/configuration_test.go +++ b/application/server/configuration_test.go @@ -2814,41 +2814,3 @@ func TestApplyUserSettingsPath_IgnoresUnchanged(t *testing.T) { assert.Equal(t, "/original", types.GetGlobalString(conf, types.SettingUserSettingsPath)) } -// IDE-1035: when the user presses the stop-scan button the IDE sends a -// window/workDoneProgress/cancel notification. The summary panel must be reset -// to its initial state so it no longer shows the counts from the canceled scan. -func TestResetSummaryPanelOnStopScan_CallsInit(t *testing.T) { - engine, _ := testutil.UnitTestWithEngine(t) - conf := engine.GetConfiguration() - - agg := &initRecordingAggregator{} - - // Seed a folder so workspaceFolderPaths returns something non-empty. - tmpDir := types.FilePath(t.TempDir()) - _, _ = workspaceutil.SetupWorkspace(t, engine, tmpDir) - folders := config.GetWorkspace(conf).Folders() - require.NotEmpty(t, folders, "precondition: workspace must contain at least one folder") - - resetSummaryPanelOnStopScan(agg, conf) - - assert.Len(t, agg.initCalls, 1, "Init must be called once to reset the summary panel") -} - -func TestResetSummaryPanelOnStopScan_NilAgg_DoesNotPanic(t *testing.T) { - engine, _ := testutil.UnitTestWithEngine(t) - conf := engine.GetConfiguration() - assert.NotPanics(t, func() { - resetSummaryPanelOnStopScan(nil, conf) - }) -} - -func TestResetSummaryPanelOnStopScan_NoFolders_DoesNotCallInit(t *testing.T) { - engine, _ := testutil.UnitTestWithEngine(t) - conf := engine.GetConfiguration() - // workspace has no folders — Init must not be called - agg := &initRecordingAggregator{} - assert.NotPanics(t, func() { - resetSummaryPanelOnStopScan(agg, conf) - }) - assert.Empty(t, agg.initCalls, "Init must not be called when there are no workspace folders") -} From 600831528ff488c66ecb050deb158475e997571f Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Wed, 10 Jun 2026 14:47:36 +0200 Subject: [PATCH 07/11] refactor(scanner,server): lift RegisterCancelCallback to interface, type the registry, cover handler [IDE-1035] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the application/server ↔ domain/snyk/scanner layering and type-safety feedback from PR review: - Lift RegisterCancelCallback(folderPath, fn) onto scanner.Scanner so the cancel handler calls it through the interface — drops the *DelegatingConcurrentScanner downcast and the racy synchronous summary-panel reset that used to fire when the type assertion failed. TestScanner gets a no-op implementation since it doesn't run the cancel-drain cycle. - Replace the cancelCallbackMap = sync.Map alias with a plain map[types.FilePath]func() guarded by a sync.Mutex. Drops the defensive v.(func()) assertion in consumeCancelCallback and keeps values typed; the access pattern (one register + one consume per folder) doesn't need sync.Map's machinery. - Extract the handler body into handleWindowWorkDoneProgressCancel so the new cancel_handler_test.go can drive it without going through jrpc2 indirection. Tests cover scan-token register-then- cancel ordering, non-scan-token cancel-without-register, and the no-scanner skip path (proving the racy sync fallback is gone). Co-Authored-By: Claude Opus 4.7 (1M context) --- application/server/cancel_handler_test.go | 169 ++++++++++++++++++++++ application/server/server.go | 88 +++++------ domain/snyk/scanner/scanner.go | 40 +++-- domain/snyk/scanner/test_scanner.go | 4 + 4 files changed, 241 insertions(+), 60 deletions(-) create mode 100644 application/server/cancel_handler_test.go diff --git a/application/server/cancel_handler_test.go b/application/server/cancel_handler_test.go new file mode 100644 index 000000000..4da0d4859 --- /dev/null +++ b/application/server/cancel_handler_test.go @@ -0,0 +1,169 @@ +/* + * © 2026 Snyk Limited + * + * 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 server + +import ( + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/snyk/snyk-ls/domain/scanstates" + scanner2 "github.com/snyk/snyk-ls/domain/snyk/scanner" + ctx2 "github.com/snyk/snyk-ls/internal/context" + "github.com/snyk/snyk-ls/internal/progress" + "github.com/snyk/snyk-ls/internal/testutil" + "github.com/snyk/snyk-ls/internal/testutil/workspaceutil" + "github.com/snyk/snyk-ls/internal/types" +) + +// fakeScanner records RegisterCancelCallback invocations so the handler test +// can assert which folders were wired without spinning up a real +// DelegatingConcurrentScanner. End-to-end ordering with SetScanDone is covered +// at the scanner layer by TestScan_CancelCallback_CalledAfterGoroutinesFinish. +type fakeScanner struct { + scanner2.TestScanner + mu sync.Mutex + callbacks map[types.FilePath]func() +} + +func (f *fakeScanner) RegisterCancelCallback(folderPath types.FilePath, fn func()) { + f.mu.Lock() + defer f.mu.Unlock() + if f.callbacks == nil { + f.callbacks = make(map[types.FilePath]func()) + } + f.callbacks[folderPath] = fn +} + +func (f *fakeScanner) registered() map[types.FilePath]func() { + f.mu.Lock() + defer f.mu.Unlock() + out := make(map[types.FilePath]func(), len(f.callbacks)) + for k, v := range f.callbacks { + out[k] = v + } + return out +} + +// Handler contract: a scan token registers reset-on-cancel for every +// workspace folder via the Scanner interface, then progress.Cancel fires +// (defer). The registration happens BEFORE the cancel — that is the +// IDE-1035 register-vs-consume race fix exercised end-to-end. +func TestHandleWindowWorkDoneProgressCancel_ScanToken_RegistersBeforeCancel(t *testing.T) { + engine := testutil.UnitTest(t) + conf := engine.GetConfiguration() + + folderA := types.FilePath(t.TempDir()) + folderB := types.FilePath(t.TempDir()) + _, _ = workspaceutil.SetupWorkspace(t, engine, folderA, folderB) + + scanner := &fakeScanner{} + agg := scanstates.NewNoopStateAggregator() + ctx := ctx2.NewContextWithDependencies(t.Context(), map[string]any{ + ctx2.DepScanners: scanner, + ctx2.DepScanStateAggregator: agg, + }) + + logger := engine.GetLogger() + tracker := progress.NewScanTracker(true, logger) + token := tracker.GetToken() + require.True(t, progress.IsScanToken(token), "precondition: NewScanTracker must register a scan token") + + _, err := handleWindowWorkDoneProgressCancel(ctx, + types.WorkdoneProgressCancelParams{Token: token}, + conf, + ) + require.NoError(t, err) + + // Both workspace folders must have had a callback registered. + got := scanner.registered() + assert.Contains(t, got, folderA, "callback must be registered for folder A") + assert.Contains(t, got, folderB, "callback must be registered for folder B") + assert.Len(t, got, 2, "exactly one callback per folder") + + // The deferred progress.Cancel must have run by the time the handler returns, + // so the scan token is no longer recognised. This is the ordering guarantee + // that prevents the register-vs-consume race. + assert.False(t, progress.IsScanToken(token), + "progress.Cancel must have fired (deferred) — registration happened first, then cancel") +} + +// Non-scan tokens (e.g. CLI download progress) must not register any reset +// callback. The cancel must still fire so the download stops. +func TestHandleWindowWorkDoneProgressCancel_NonScanToken_NoRegistration(t *testing.T) { + engine := testutil.UnitTest(t) + conf := engine.GetConfiguration() + + folderA := types.FilePath(t.TempDir()) + _, _ = workspaceutil.SetupWorkspace(t, engine, folderA) + + scanner := &fakeScanner{} + agg := scanstates.NewNoopStateAggregator() + ctx := ctx2.NewContextWithDependencies(t.Context(), map[string]any{ + ctx2.DepScanners: scanner, + ctx2.DepScanStateAggregator: agg, + }) + + logger := engine.GetLogger() + tracker := progress.NewTracker(true, logger) // plain tracker — NOT a scan token + token := tracker.GetToken() + require.False(t, progress.IsScanToken(token), "precondition: NewTracker must NOT register as a scan token") + + _, err := handleWindowWorkDoneProgressCancel(ctx, + types.WorkdoneProgressCancelParams{Token: token}, + conf, + ) + require.NoError(t, err) + + assert.Empty(t, scanner.registered(), + "non-scan tokens must not register a reset callback — generic progress must not wipe scan results") + assert.True(t, progress.IsCanceled(token), + "progress.Cancel must still fire for non-scan tokens so the download is stopped") +} + +// When the scanner is missing from context (early startup / tests that don't +// wire DI), the handler must NOT fall back to a synchronous reset that races +// in-flight SetScanDone writes. It should log and return cleanly. +func TestHandleWindowWorkDoneProgressCancel_ScanToken_NoScanner_NoSyncFallback(t *testing.T) { + engine := testutil.UnitTest(t) + conf := engine.GetConfiguration() + + folderA := types.FilePath(t.TempDir()) + _, _ = workspaceutil.SetupWorkspace(t, engine, folderA) + + // Aggregator is present, but scanner is intentionally missing. + agg := scanstates.NewNoopStateAggregator() + ctx := ctx2.NewContextWithDependencies(t.Context(), map[string]any{ + ctx2.DepScanStateAggregator: agg, + }) + + logger := engine.GetLogger() + tracker := progress.NewScanTracker(true, logger) + token := tracker.GetToken() + + _, err := handleWindowWorkDoneProgressCancel(ctx, + types.WorkdoneProgressCancelParams{Token: token}, + conf, + ) + require.NoError(t, err) + + // progress.Cancel still fires (defer). No reset happened — the racy sync + // fallback the reviewer flagged on #3382206417 is gone. + assert.False(t, progress.IsScanToken(token)) +} diff --git a/application/server/server.go b/application/server/server.go index 766c64b5b..ca1fac733 100644 --- a/application/server/server.go +++ b/application/server/server.go @@ -1178,54 +1178,54 @@ func textDocumentHover() jrpc2.Handler { func windowWorkDoneProgressCancelHandler(conf configuration.Configuration) jrpc2.Handler { return handler.New(func(ctx context.Context, params types.WorkdoneProgressCancelParams) (any, error) { - logger := ctx2.LoggerFromContext(ctx) - logger.Debug().Str("method", "WindowWorkDoneProgressCancelHandler").Interface("params", params).Msg("RECEIVING") - - // Only reset the summary panel for scan cancellations. Generic progress - // tokens (e.g. CLI download/install) must not wipe valid scan results. - isScan := progress.IsScanToken(params.Token) - - // Defer the cancel so any RegisterCancelCallback below is in place - // before scan goroutines observe cancellation and drain through - // consumeCancelCallback (scanner.go). Otherwise the reset can be - // silently dropped, and a stale callback fires on the folder's - // next scan (IDE-1035 register-vs-consume race). - defer progress.Cancel(params.Token) + return handleWindowWorkDoneProgressCancel(ctx, params, conf) + }) +} - if !isScan { - return nil, nil - } +// handleWindowWorkDoneProgressCancel is the testable body of +// windowWorkDoneProgressCancelHandler. Kept as a free function so integration +// tests can drive it without going through jrpc2.Handler indirection. +func handleWindowWorkDoneProgressCancel(ctx context.Context, params types.WorkdoneProgressCancelParams, conf configuration.Configuration) (any, error) { + logger := ctx2.LoggerFromContext(ctx) + logger.Debug().Str("method", "WindowWorkDoneProgressCancelHandler").Interface("params", params).Msg("RECEIVING") + + // Only reset the summary panel for scan cancellations. Generic progress + // tokens (e.g. CLI download/install) must not wipe valid scan results. + isScan := progress.IsScanToken(params.Token) + + // Defer the cancel so any RegisterCancelCallback below is in place + // before scan goroutines observe cancellation and drain through + // consumeCancelCallback (scanner.go). Otherwise the reset can be + // silently dropped, and a stale callback fires on the folder's + // next scan (IDE-1035 register-vs-consume race). + defer progress.Cancel(params.Token) + + if !isScan { + return nil, nil + } - scanAgg, ok := scanStateAggregatorFromContext(ctx) - if !ok || scanAgg == nil { - logger.Warn().Str("method", "WindowWorkDoneProgressCancelHandler"). - Msg("scan state aggregator not found in context; summary panel will not be reset") - return nil, nil - } - // Register the reset callback on the scanner so it fires AFTER all - // per-product goroutines have finished their SetScanDone writes (E, - // IDE-1035). The reset is keyed to each workspace folder so that - // concurrent folder scans reset independently. - sc, scOk := scannerFromContext(ctx) - if scOk { - if dcs, ok := sc.(*scanner2.DelegatingConcurrentScanner); ok { - folderPaths := workspaceFolderPaths(conf) - for _, fp := range folderPaths { - dcs.RegisterCancelCallback(fp, func() { - resetSummaryPanel(scanAgg, []types.FilePath{fp}) - }) - } - return nil, nil - } - } - // Fallback: scanner not available in context — reset synchronously. - // This path is only hit in tests or during early startup where the - // scanner is not yet wired into the context. + scanAgg, ok := scanStateAggregatorFromContext(ctx) + if !ok || scanAgg == nil { + logger.Warn().Str("method", "WindowWorkDoneProgressCancelHandler"). + Msg("scan state aggregator not found in context; summary panel will not be reset") + return nil, nil + } + // Register the reset callback on the scanner so it fires AFTER all + // per-product goroutines have finished their SetScanDone writes + // (IDE-1035). The reset is keyed to each workspace folder so that + // concurrent folder scans reset independently. + sc, scOk := scannerFromContext(ctx) + if !scOk { logger.Debug().Str("method", "WindowWorkDoneProgressCancelHandler"). - Msg("scanner not in context; resetting summary panel synchronously") - resetSummaryPanel(scanAgg, workspaceFolderPaths(conf)) + Msg("scanner not in context; summary panel will not be reset") return nil, nil - }) + } + for _, fp := range workspaceFolderPaths(conf) { + sc.RegisterCancelCallback(fp, func() { + resetSummaryPanel(scanAgg, []types.FilePath{fp}) + }) + } + return nil, nil } func codeActionResolveHandler(logger *zerolog.Logger, svc *codeaction.CodeActionsService, server types.Server) handler.Func { diff --git a/domain/snyk/scanner/scanner.go b/domain/snyk/scanner/scanner.go index 403d193af..29d2456ac 100644 --- a/domain/snyk/scanner/scanner.go +++ b/domain/snyk/scanner/scanner.go @@ -38,12 +38,6 @@ import ( "github.com/snyk/snyk-ls/internal/vcs" ) -// cancelCallbacks maps folderPath → reset function registered by the LSP -// cancel handler (IDE-1035). Callbacks are consumed (and removed) by Scan() -// after all per-product goroutines have finished, ensuring the reset happens -// only once and only after all SetScanDone writes are complete. -type cancelCallbackMap = sync.Map - var ( _ Scanner = (*DelegatingConcurrentScanner)(nil) _ snyk.InlineValueProvider = (*DelegatingConcurrentScanner)(nil) @@ -53,6 +47,11 @@ var ( type Scanner interface { types.Scanner Init(ctx context.Context) error + // RegisterCancelCallback registers fn to be invoked by Scan() for + // folderPath after all per-product goroutines have exited following a + // user-initiated cancel. Used by the LSP cancel handler to schedule a + // reset of the scan-state aggregator without racing in-flight writes. + RegisterCancelCallback(folderPath types.FilePath, fn func()) } // DelegatingConcurrentScanner is a simple Scanner Implementation that delegates on other scanners asynchronously @@ -70,8 +69,12 @@ type DelegatingConcurrentScanner struct { snykApiClient snyk_api.SnykApiClient configResolver types.ConfigResolverInterface // cancelCallbacks stores per-folder reset functions registered by the LSP - // cancel handler. Consumed by Scan() after all goroutines exit (IDE-1035). - cancelCallbacks cancelCallbackMap + // cancel handler (IDE-1035). Consumed and removed by Scan() after all + // per-product goroutines have exited so the reset happens only once and + // only after all SetScanDone writes are complete. A plain map guarded by + // cancelCallbacksMu keeps func() values typed without sync.Map's any cast. + cancelCallbacksMu sync.Mutex + cancelCallbacks map[types.FilePath]func() } // RegisterCancelCallback stores fn to be called by Scan() for folderPath once @@ -80,22 +83,27 @@ type DelegatingConcurrentScanner struct { // overwrites the previous one. This is intentional: the handler is only ever // called once per active cancel notification. func (sc *DelegatingConcurrentScanner) RegisterCancelCallback(folderPath types.FilePath, fn func()) { - sc.cancelCallbacks.Store(folderPath, fn) + sc.cancelCallbacksMu.Lock() + defer sc.cancelCallbacksMu.Unlock() + if sc.cancelCallbacks == nil { + sc.cancelCallbacks = make(map[types.FilePath]func()) + } + sc.cancelCallbacks[folderPath] = fn } // consumeCancelCallback fires and removes any cancel callback registered for // folderPath. Called by Scan() after both WaitGroups return so the callback // runs only after all SetScanDone writes have completed (IDE-1035). func (sc *DelegatingConcurrentScanner) consumeCancelCallback(folderPath types.FilePath) { - v, loaded := sc.cancelCallbacks.LoadAndDelete(folderPath) - if !loaded { - return + sc.cancelCallbacksMu.Lock() + fn, ok := sc.cancelCallbacks[folderPath] + if ok { + delete(sc.cancelCallbacks, folderPath) } - fn, ok := v.(func()) - if !ok { - return + sc.cancelCallbacksMu.Unlock() + if ok { + fn() } - fn() } func (sc *DelegatingConcurrentScanner) Issue(key string) types.Issue { diff --git a/domain/snyk/scanner/test_scanner.go b/domain/snyk/scanner/test_scanner.go index 472a30c31..3e5f92a00 100644 --- a/domain/snyk/scanner/test_scanner.go +++ b/domain/snyk/scanner/test_scanner.go @@ -48,6 +48,10 @@ func NewTestScanner() *TestScanner { func (s *TestScanner) Init(_ context.Context) error { return nil } +// RegisterCancelCallback is a no-op for TestScanner; tests that exercise +// the cancel-drain cycle should use DelegatingConcurrentScanner instead. +func (s *TestScanner) RegisterCancelCallback(_ types.FilePath, _ func()) {} + func (s *TestScanner) IsEnabled() bool { return true } From b4833304d084a4fcf184402cc53830737d8400aa Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Wed, 10 Jun 2026 14:52:25 +0200 Subject: [PATCH 08/11] refactor(scanner): collapse setupScannerWithResolver into setupScannerWithResolverAndAgg [IDE-1035] setupScannerWithResolver and setupScannerWithResolverAndAgg duplicated the full NewDelegatingScanner construction; the only difference was an explicit aggregator parameter. Have the former delegate to the latter with NewNoopStateAggregator as the default. Co-Authored-By: Claude Opus 4.7 (1M context) --- domain/snyk/scanner/scanner_test.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/domain/snyk/scanner/scanner_test.go b/domain/snyk/scanner/scanner_test.go index 61684a799..f6b40c263 100644 --- a/domain/snyk/scanner/scanner_test.go +++ b/domain/snyk/scanner/scanner_test.go @@ -139,17 +139,7 @@ func setupScannerWithResolver(t *testing.T, engine workflow.Engine, tokenService scanNotifier ScanNotifier, ) { t.Helper() - scanNotifier = NewMockScanNotifier() - notifier := notification.NewNotifier() - apiClient := &snyk_api.FakeApiClient{CodeEnabled: false} - persister := persistence.NewNopScanPersister() - scanStateAggregator := scanstates.NewNoopStateAggregator() - er := error_reporting.NewTestErrorReporter(engine) - authenticationProvider := authentication.NewFakeCliAuthenticationProvider(engine) - authenticationProvider.IsAuthenticated = true - authenticationService := authentication.NewAuthenticationService(engine, tokenService, authenticationProvider, er, notifier, configResolver) - sc = NewDelegatingScanner(engine, tokenService, initialize.NewDelegatingInitializer(), performance.NewInstrumentor(), scanNotifier, apiClient, authenticationService, notifier, persister, scanStateAggregator, configResolver, testProductScanners...) - return sc, scanNotifier + return setupScannerWithResolverAndAgg(t, engine, tokenService, configResolver, scanstates.NewNoopStateAggregator(), testProductScanners...) } func Test_userNotAuthenticated_ScanSkipped(t *testing.T) { From e511bbbacdeaf34955c822082d671b99c35968dd Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Wed, 10 Jun 2026 15:39:14 +0200 Subject: [PATCH 09/11] refactor(progress): collapse scanTokens registry into Tracker.isScan field [IDE-1035] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parallel scanTokens map (and its mutex) had to be kept in sync with the trackers map in three places — deleteTracker, Cancel, and CleanupChannels — so any future tracker-removal path that forgot the second mutation would leak or stale the scan-token state. Model "is scan" as an isScan bool field on Tracker. IsScanToken becomes a single trackers lookup under trackersMutex.RLock and returns false once the tracker entry is gone, so removal logic only has to touch trackers. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/progress/progress.go | 63 ++++++++++++++++------------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/internal/progress/progress.go b/internal/progress/progress.go index 8bc75ade3..ed0a38f06 100644 --- a/internal/progress/progress.go +++ b/internal/progress/progress.go @@ -34,14 +34,6 @@ var trackersMutex sync.RWMutex var trackers = make(map[types.ProgressToken]*Tracker) var ToServerProgressChannel = make(chan types.ProgressParams, 1000) -// scanTokensMutex guards scanTokens. -var scanTokensMutex sync.RWMutex - -// scanTokens is the set of progress tokens that belong to scan operations. -// Tokens created via NewScanTracker are recorded here so that the -// window/workDoneProgress/cancel handler can distinguish scan cancellations -// from download/install cancellations (IDE-1035). -var scanTokens = make(map[types.ProgressToken]struct{}) var _ ui.ProgressBar = (*Tracker)(nil) type Tracker struct { @@ -49,6 +41,12 @@ type Tracker struct { cancelChannel chan bool token types.ProgressToken cancellable bool + // isScan distinguishes scan-operation trackers (NewScanTracker) from + // generic progress trackers so window/workDoneProgress/cancel can scope + // the summary-panel reset to scan cancellations only (IDE-1035). + // Written under trackersMutex when the tracker is registered; only ever + // read via IsScanToken under the same lock. + isScan bool lastReport time.Time lastReportPercentage int finished bool @@ -88,25 +86,31 @@ func NewTracker(cancellable bool, logger *zerolog.Logger) *Tracker { return t } -// NewScanTracker creates a progress tracker and registers its token as a scan -// token so that IsScanToken returns true for it. Use this for scan operations -// (OSS, Code, IaC) instead of NewTracker. Download/install operations must -// continue to use NewTracker so their cancel events do not reset the summary -// panel (IDE-1035). +// NewScanTracker creates a progress tracker tagged as a scan token so +// IsScanToken returns true for it. Use this for scan operations (OSS, Code, +// IaC) instead of NewTracker. Download/install operations must continue to +// use NewTracker so their cancel events do not reset the summary panel +// (IDE-1035). func NewScanTracker(cancellable bool, logger *zerolog.Logger) *Tracker { t := NewTracker(cancellable, logger) - scanTokensMutex.Lock() - scanTokens[t.token] = struct{}{} - scanTokensMutex.Unlock() + trackersMutex.Lock() + t.isScan = true + trackersMutex.Unlock() return t } -// IsScanToken reports whether token was created via NewScanTracker. +// IsScanToken reports whether token belongs to an active scan tracker +// created via NewScanTracker. Returns false once the tracker is removed +// (e.g. after Cancel or deleteTracker), giving the cancel handler a +// single source of truth without a parallel registry. func IsScanToken(token types.ProgressToken) bool { - scanTokensMutex.RLock() - _, ok := scanTokens[token] - scanTokensMutex.RUnlock() - return ok + trackersMutex.RLock() + defer trackersMutex.RUnlock() + t, ok := trackers[token] + if !ok { + return false + } + return t.isScan } func (t *Tracker) GetChannel() chan types.ProgressParams { @@ -266,9 +270,6 @@ func (t *Tracker) deleteTracker() { trackersMutex.Lock() delete(trackers, t.token) trackersMutex.Unlock() - scanTokensMutex.Lock() - delete(scanTokens, t.token) - scanTokensMutex.Unlock() } func (t *Tracker) GetToken() types.ProgressToken { @@ -333,12 +334,6 @@ func CleanupChannels() { for token := range tempTrackers { Cancel(token) } - - // Clear any remaining scan tokens that may have been deregistered from the - // tracker registry already (e.g. via deleteTracker) but not yet cleaned up. - scanTokensMutex.Lock() - scanTokens = make(map[types.ProgressToken]struct{}) - scanTokensMutex.Unlock() } func (t *Tracker) IsCanceled() bool { @@ -354,11 +349,9 @@ func Cancel(token types.ProgressToken) { delete(trackers, token) close(t.cancelChannel) } - // Remove from scan-token set regardless of whether a tracker was found, - // so IsScanToken is consistent after cancellation. - scanTokensMutex.Lock() - delete(scanTokens, token) - scanTokensMutex.Unlock() + // Removing the tracker entry above is enough: IsScanToken looks up + // trackers and returns false once the entry is gone — no parallel + // registry to keep in sync. } func IsCanceled(token types.ProgressToken) bool { From c71618330248f27467ac3e6937e7cb1ab0728f52 Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Wed, 10 Jun 2026 16:31:00 +0200 Subject: [PATCH 10/11] style: fix gofmt alignment + misspell in cancel-handler test [IDE-1035] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - gofmt -w internal/progress/progress.go: the new isScan field's preceding comment split the Tracker struct's alignment group; reflow the pre-comment fields to the gofmt-canonical narrow alignment. - recognised → recognized in cancel_handler_test.go to satisfy golangci-lint's misspell checker. Co-Authored-By: Claude Opus 4.7 (1M context) --- application/server/cancel_handler_test.go | 2 +- internal/progress/progress.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/application/server/cancel_handler_test.go b/application/server/cancel_handler_test.go index 4da0d4859..574bbad56 100644 --- a/application/server/cancel_handler_test.go +++ b/application/server/cancel_handler_test.go @@ -98,7 +98,7 @@ func TestHandleWindowWorkDoneProgressCancel_ScanToken_RegistersBeforeCancel(t *t assert.Len(t, got, 2, "exactly one callback per folder") // The deferred progress.Cancel must have run by the time the handler returns, - // so the scan token is no longer recognised. This is the ordering guarantee + // so the scan token is no longer recognized. This is the ordering guarantee // that prevents the register-vs-consume race. assert.False(t, progress.IsScanToken(token), "progress.Cancel must have fired (deferred) — registration happened first, then cancel") diff --git a/internal/progress/progress.go b/internal/progress/progress.go index ed0a38f06..ff55e0203 100644 --- a/internal/progress/progress.go +++ b/internal/progress/progress.go @@ -37,10 +37,10 @@ var ToServerProgressChannel = make(chan types.ProgressParams, 1000) var _ ui.ProgressBar = (*Tracker)(nil) type Tracker struct { - channel chan types.ProgressParams - cancelChannel chan bool - token types.ProgressToken - cancellable bool + channel chan types.ProgressParams + cancelChannel chan bool + token types.ProgressToken + cancellable bool // isScan distinguishes scan-operation trackers (NewScanTracker) from // generic progress trackers so window/workDoneProgress/cancel can scope // the summary-panel reset to scan cancellations only (IDE-1035). From 84955a2280c0f4c902def61a15a2fc1aa8cbbb5d Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Thu, 11 Jun 2026 11:18:29 +0200 Subject: [PATCH 11/11] fix(lint): remove trailing newline in configuration_test.go (goimports) --- application/server/configuration_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/application/server/configuration_test.go b/application/server/configuration_test.go index 389043f75..4e7e794cb 100644 --- a/application/server/configuration_test.go +++ b/application/server/configuration_test.go @@ -2813,4 +2813,3 @@ func TestApplyUserSettingsPath_IgnoresUnchanged(t *testing.T) { assert.Equal(t, "/original", types.GetGlobalString(conf, types.SettingUserSettingsPath)) } -