Skip to content

fix: remediation code-action follow-up fixes [IDE-2052]#1366

Open
basti-snyk wants to merge 1 commit into
feat/IDE-2052-finding-idfrom
feat/IDE-2052-remediation-followups
Open

fix: remediation code-action follow-up fixes [IDE-2052]#1366
basti-snyk wants to merge 1 commit into
feat/IDE-2052-finding-idfrom
feat/IDE-2052-remediation-followups

Conversation

@basti-snyk

@basti-snyk basti-snyk commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

User description

Summary

Follow-up fixes to the remediation code-action flow, surfaced by review of the fixFolder work (PR #1331). All are correctness/robustness fixes; ~217 lines of production change, the rest tests (outside-in TDD throughout).

  • Restrict remediation quick-fixes to supported products. The filter let fully-fixable Open Source issues through and offered a RemediationAgentQuickFix the agent can't apply. Replaced the negated IaC bypass with an explicit product switch (Code when it has an AI fix; IaC) — OSS/Secrets/Unknown excluded.
  • Propagate the codeAction/resolve context to the provider. The deferred edit ran with context.Background(), so cancelling a resolve didn't stop the remediation. The deferred-edit type now carries a context.Context threaded to Remediate; nil deferred edits are guarded in both the resolve and AI-fix paths.
  • Make the code-action cache concurrency-safe. actionsCache was an unsynchronised map read by codeAction/resolve and written by textDocument/codeAction. Added a mutex covering every access; the entry is read and (via defer) deleted under the lock, with the slow edit run lock-free — so a concurrent retry still finds the entry and the entry is always cleaned up, even on panic.
  • FixFolder rejects a dirty worktree (git status --porcelain --untracked-files=no) so it can't silently fold pre-existing tracked changes into its result, while ignoring untracked artifacts.
  • Gate the snyk.remediationAgent.fixFolder command advertisement on the feature flag (exported RemediationAgentEnabledKey shared with the DI gate). Fixed the filtered-issue debug count; removed the unused NoopProvider.

PR Stack — Merge Order

flowchart LR
    main(["main"])
    PR0["#1330\nremy unit tests"]
    PR1["#1331\nfixFolder + remy integ/smoke + cross-platform"]
    PR2["← YOU ARE HERE\nremediation code-action follow-up fixes"]
    main --> PR0 --> PR1 --> PR2
    style PR2 fill:#ffd700,color:#000
Loading

Depends on: #1331

Note on size

780 changed lines, but 217 are production and ~563 are tests (TDD acceptance/unit + a -race regression test for the cache). Kept as one cohesive bug-fix set rather than fragmenting the review.

Test plan

  • CI green

PR Type

Enhancement, Bug fix


Description

  • Improve remediation code action robustness and concurrency.

  • Enhance context propagation to remediation providers.

  • Add product filtering for remediation actions.

  • Strengthen FixFolder command preconditions and error handling.


Diagram Walkthrough

flowchart LR
  subgraph LSP Client
    Client["Client"]
  end
  subgraph Snyk LS Server
    A[textDocument/codeAction] --> B(CodeActionService.GetCodeActions);
    B --> C{IssuesProvider};
    B --> D[remediationCodeActions];
    D -- Filters --> E(Product Check);
    B --> F(cacheCodeAction);
    F -- Mutex --> G[(actionsCache)];
    H[codeAction/resolve] --> I(CodeActionService.ResolveCodeAction);
    I -- Read/Unlock --> G;
    I -- Pass Context --> J(DeferredEdit);
    I -- Mutex --> G;
    K[FixFolder Command] --> L(Remediator.FixFolder);
    L -- Git Checks --> M{Worktree Status};
    N[Initialize Server] --> O{ExecuteCommandProvider};
    O -- Conditionally adds --> P[RemediationAgentFixFolderCommand];
  end
  Client -- Request --> A;
  Client -- Request --> H;
  Client -- Execute --> K;
Loading

File Walkthrough

Relevant files
Enhancement
8 files
codeaction.go
Make code action cache concurrency-safe and improve remediation logic
+60/-11 
init.go
Export RemediationAgentEnabledKey and use it for feature flag
+6/-3     
codeaction_handlers.go
Pass context to ResolveCodeAction handler                               
+1/-1     
server.go
Conditionally advertise FixFolder command and use exported key
+46/-35 
code_fix.go
Pass context to deferred edits and handle nil deferred edits
+7/-2     
codeaction.go
Update deferred edit signature to accept context.Context 
+7/-3     
code_actions.go
Update OSS quickfix edit callback to accept context.Context
+2/-1     
issues.go
Update CodeAction interface for context in deferred edits
+2/-1     
Tests
11 files
codeaction_test.go
Add tests for cache concurrency and context propagation in code
actions
+186/-5 
export_test.go
Export cache length for testing                                                   
+32/-0   
race_test.go
Add race detector tests for code action cache                       
+145/-0 
remediation_test.go
Test remediation agent context propagation and OSS issue handling
+80/-1   
init_fix_folder_test.go
Use exported RemediationAgentEnabledKey in test                   
+1/-1     
server_smoke_test.go
Use exported RemediationAgentEnabledKey in smoke test       
+1/-1     
server_test.go
Test conditional command advertisement and remediation agent features
+38/-2   
code_fix_test.go
Add test for nil deferred edit in code fix command             
+43/-3   
codeaction_test.go
Update mock deferred edit to accept context.Context           
+3/-2     
remy_fix_folder_test.go
Add tests for FixFolder dirty worktree guard                         
+120/-7 
code_actions_test.go
Update deferred edit calls in OSS tests                                   
+8/-7     
Bug fix
1 files
remy.go
Guard FixFolder against dirty worktrees                                   
+13/-0   
Additional files
2 files
noop.go +0/-32   
noop_test.go +0/-34   

…E-2052]

Follow-up fixes to the remediation code-action path, surfaced by review of
the fixFolder work:

- Restrict remediation quick-fixes to products the agent can handle. The
  filter previously let fixable Open Source issues through and offered a
  RemediationAgentQuickFix the LLM agent cannot apply; replace the negated
  IaC bypass with an explicit product switch (Code when it has an AI fix,
  IaC) and drop everything else.
- Propagate the codeAction/resolve request context to the provider. The
  deferred edit ran with context.Background(), so cancelling a resolve left
  the remediation running; the deferred-edit type now carries a context that
  is threaded through to Remediate, and a nil deferred edit is guarded in
  both the resolve and the AI-fix command paths.
- Make the code-action cache concurrency-safe. actionsCache was an
  unsynchronised map read by codeAction/resolve and written by
  textDocument/codeAction; add a mutex covering every access, take it only to
  read and (via defer) delete the entry, and run the slow edit without the
  lock held so the entry survives a concurrent retry and is always cleaned up,
  even on panic.
- FixFolder now rejects a worktree with uncommitted tracked changes
  (git status --porcelain --untracked-files=no) so it cannot silently fold
  pre-existing edits into its result, while ignoring untracked artifacts.
- Advertise snyk.remediationAgent.fixFolder only when the remediation agent
  is enabled, via an exported RemediationAgentEnabledKey shared with the DI
  gate. Correct the filtered-issue debug count and remove the unused
  NoopProvider.
@basti-snyk

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-io

snyk-io Bot commented Jun 29, 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
Code Security 0 0 0 0 0 issues

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

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (7a5aa40)

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Race Condition Risk 🟡 [minor]

In ResolveCodeAction, the actionsCacheMu is released before invoking the slow deferred edit. While this is intentional to avoid blocking other LSP requests, if a client sends two identical resolve requests for the same UUID simultaneously, both will find the entry in the cache (before the defer cleanup runs) and trigger two independent, expensive remediation calls to the provider. The PR comments suggest providers must be safe for concurrent calls, but this introduces redundant work and potential file system contention in the provider layer.

cached, found := c.actionsCache[key]
c.actionsCacheMu.Unlock()

if !found {
	return types.LSPCodeAction{}, errors.New(fmt.Sprint("could not find cached action for uuid ", key))
}

// Remove the cache entry once the edit is done, even if it panics.
// Using defer guarantees cleanup on any exit path — normal return or panic.
defer func() {
	c.actionsCacheMu.Lock()
	delete(c.actionsCache, key)
	c.actionsCacheMu.Unlock()
}()

// Invoke the deferred edit outside the lock. When DeferredEdit is nil the
// action carries no edit (e.g. a command-only CodeAction); skip the call and
// leave edit as nil so the resolved action is returned without an Edit field.
var edit *types.WorkspaceEdit
if de := cached.action.GetDeferredEdit(); de != nil {
	edit = (*de)(ctx)
📚 Repository Context Analyzed

This review considered 49 relevant code sections from 15 files (average relevance: 0.96)

@basti-snyk

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (7a5aa40)

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

Labels

depends-on-1331 Depends on PR #1331

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant