Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 169 additions & 0 deletions application/server/cancel_handler_test.go
Original file line number Diff line number Diff line change
@@ -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 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")
}

// 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))
}
17 changes: 9 additions & 8 deletions application/server/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
23 changes: 13 additions & 10 deletions application/server/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
34 changes: 34 additions & 0 deletions application/server/notification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 canceled")
}

func Test_NotifierShouldSendNotificationToClient(t *testing.T) {
engine, tokenService := testutil.UnitTestWithEngine(t)
loc, jsonRPCRecorder, _ := setupServer(t, engine, tokenService)
Expand Down
54 changes: 49 additions & 5 deletions application/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -1176,14 +1176,58 @@ 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)
return nil, nil
return handleWindowWorkDoneProgressCancel(ctx, params, conf)
})
}

// 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
// (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; summary panel will not be reset")
return nil, nil
}
for _, fp := range workspaceFolderPaths(conf) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix — cancel resets every folder, not the cancelled one. This loop registers a reset callback for every workspace folder, but params.Token identifies a single per-product tracker belonging to one folder. In a multi-folder workspace, cancelling folder A's scan also registers reset callbacks on folders B, C… Those callbacks are not consumed now (no in-flight scan there) and instead fire on each of those folders' next Scan() completion (scanner.go consumeCancelCallback), wiping valid results the user never asked to clear. Consider mapping the cancelled token back to its owning folder and registering the reset only for that folder.

(Three of the four reviewers independently flagged this as the root cause of a cluster of data-loss bugs.)

— AI review

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 {
return handler.New(ResolveCodeActionHandler(logger, svc, server))
}
Expand Down
Loading
Loading