Skip to content

feat: verify fixedIssueIds/preventedIssueIds against an in-process sc…#48

Open
MaVictor wants to merge 4 commits into
mainfrom
feat/memcache-support
Open

feat: verify fixedIssueIds/preventedIssueIds against an in-process sc…#48
MaVictor wants to merge 4 commits into
mainfrom
feat/memcache-support

Conversation

@MaVictor

@MaVictor MaVictor commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Description

Cache the freshest scan findings per file path from successful snyk_code_scan/snyk_sca_scan calls, then check snyk_send_feedback's claimed IDs against that cache and stamp the result (verified/unverifiable/mismatch) on the analytics event. Verification never blocks or delays the feedback call - it's a best-effort, observational check only.

Checklist

  • Tests added and all succeed
  • Regenerated mocks, etc. (make generate)
  • Linted (make lint-fix)
  • README.md updated, if user-facing
  • License file updated, if new 3rd-party dependency is introduced

Note

Medium Risk
Touches the scan tool success path and feedback analytics semantics; incorrect cache scoping could mislabel verification, but feedback is observational only and does not gate tool success.

Overview
Adds an in-process scan-result cache (capped at 500 file paths) that records the latest SAST/SCA findings from successful snyk_code_scan and snyk_sca_scan calls. The cache is updated from raw CLI output before LLM-facing enhancement, using the tool’s original path argument as clear/authority scope so single-file rescans do not wipe sibling files.

snyk_send_feedback now accepts optional fixedIssueIds, emits those IDs on analytics, and runs a best-effort verification of combined preventedIssueIds + fixedIssueIds against the cache. Events get mcp::verification of verified, unverifiable, or mismatch (worst ID wins); verification never blocks the tool. Tool schema/docs are updated accordingly.

Cache logic handles clean rescans (zero findings), directory vs file scope, SAST/SCA separation on the same path, and LRU eviction. Broad unit and handler integration tests cover these cases.

Reviewed by Cursor Bugbot for commit d889a2d. Bugbot is set up for automated code reviews on this repo. Configure here.

@MaVictor MaVictor requested review from a team as code owners July 14, 2026 19:59
@CLAassistant

CLAassistant commented Jul 14, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ifeanyiecheruo
❌ MaVictor
You have signed the CLA already but the status is still pending? Let us recheck it.

@snyk-io

snyk-io Bot commented Jul 14, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Comment thread internal/mcp/scan_cache.go
Comment thread internal/mcp/tools.go Outdated
@ifeanyiecheruo ifeanyiecheruo force-pushed the feat/memcache-support branch from 5282e64 to 21ef5e7 Compare July 14, 2026 23:59
Base automatically changed from feat/session-correlation-id to main July 15, 2026 13:37
MaVictor and others added 3 commits July 15, 2026 09:48
…an-result cache

Cache the freshest scan findings per file path from successful
snyk_code_scan/snyk_sca_scan calls, then check snyk_send_feedback's
claimed IDs against that cache and stamp the result
(verified/unverifiable/mismatch) on the analytics event. Verification
never blocks or delays the feedback call - it's a best-effort,
observational check only.
@MaVictor MaVictor force-pushed the feat/memcache-support branch from 21ef5e7 to 4357d22 Compare July 15, 2026 15:44
Comment thread internal/mcp/tools.go Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d889a2d. Configure here.

return verificationUnverifiable
}

return verificationVerified

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Eviction yields false verified state

Medium Severity

VerifyID treats a claimed issue as verified whenever that ID is absent from every remaining cache entry of the matching scan type. After LRU eviction removes the only entries that ever held the ID, unrelated scans can still leave other same-type entries in the cache, so foundRelevantScan stays true and feedback can be tagged verified with no fresh evidence for that file or ID.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d889a2d. Configure here.

@ifeanyiecheruo ifeanyiecheruo left a comment

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.

Changes look good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants