Skip to content

v20260715.145353

  • v20260715.145353
  • 1ce2570
  • Partially verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
  • Choose a tag to compare

  • v20260715.145353
  • 1ce2570
  • Choose a tag to compare

  • Partially verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
@acke acke tagged this 15 Jul 12:53
* 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.

* fix: correct misspelling cancelled → canceled in test comment [IDE-1035]

Lint catch: golangci-lint misspell wants the American spelling.

* fix(server): gate stop-scan reset to scan tokens; converge after goroutines exit [IDE-1035]

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)

* style(server,scanner): address golangci-lint feedback [IDE-1035]

- 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)

* 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) <noreply@anthropic.com>

* refactor(server): delete unreferenced resetSummaryPanelOnStopScan wrapper [IDE-1035]

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) <noreply@anthropic.com>

* refactor(scanner,server): lift RegisterCancelCallback to interface, type the registry, cover handler [IDE-1035]

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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* refactor(progress): collapse scanTokens registry into Tracker.isScan field [IDE-1035]

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) <noreply@anthropic.com>

* style: fix gofmt alignment + misspell in cancel-handler test [IDE-1035]

- 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) <noreply@anthropic.com>

* fix(lint): remove trailing newline in configuration_test.go (goimports)

* fix(scanner): scope stop-scan summary reset to the canceled folder only [IDE-1035]

The cancel handler reset the summary panel for every open workspace folder,
so canceling one folder's scan silently wiped another folder's valid,
already-displayed results on its next completed scan. Tracker now records
its owning folder at creation and the handler looks that folder up instead
of iterating every workspace folder.

* fix(server): drop dead conf param from cancel handler [IDE-1035]

handleWindowWorkDoneProgressCancel never read its conf parameter — the
scan-state aggregator and scanner are both pulled from ctx. Remove it
from the function and its one caller, and drop the now-unused conf
plumbing from the three call sites in cancel_handler_test.go.

* fix(progress): avoid NewScanTracker foot-gun for uncancellable code-client token [IDE-1035]

NewScanTracker(true, ..., "") tagged this tracker as isScan=true with an
empty folder path. FolderForScanToken has no empty-path guard, so a future
change making this token cancellable would flow an empty folder key into
resetSummaryPanel. This token is never IDE-cancellable, so it never needs
scan classification — use NewTracker instead.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Nick Yasnohorodskyi <nikita.yasnohorodskyi@snyk.io>
Assets 2
Loading