Skip to content

feat: add remy-backed RemediationProvider skeleton [IDE-2052]#1328

Draft
bastiandoetsch wants to merge 2 commits into
feat/IDE-2052-pr2from
feat/IDE-2052-pr3
Draft

feat: add remy-backed RemediationProvider skeleton [IDE-2052]#1328
bastiandoetsch wants to merge 2 commits into
feat/IDE-2052-pr2from
feat/IDE-2052-pr3

Conversation

@bastiandoetsch

@bastiandoetsch bastiandoetsch commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

User description

Summary

  • Add remyProvider struct backed by gafRunner (invokes GAF "fix" workflow)
  • NewRemyProvider(engine, runner)runner is the test seam
  • FileChangeNotifier interface with InvalidateFile(path)
  • Wire InvalidateFile into textDocument/didChange and textDocument/didSave LSP handlers via deps.RemediationNotifier
  • Gate remyProvider construction behind remediation_agent_enabled config key

PR Stack — Merge Order

flowchart LR
    main(["main"])
    PR1["#1326 PR-1\nfindingId + kind"]
    PR2["#1327 PR-2\nCA provider"]
    PR3["#? PR-3 ← YOU ARE HERE\nremy initial"]
    PR4["#? PR-4\nworktree impl"]
    PR5["#? PR-5\nunit tests"]
    PR6["#? PR-6\ninteg+smoke tests"]
    main --> PR1 --> PR2 --> PR3 --> PR4 --> PR5 --> PR6
    style PR3 fill:#ffd700,color:#000
Loading

Depends on: #1327

Test plan

  • make test passes

PR Type

Enhancement


Description

  • Introduce Remy-backed RemediationProvider.

  • Integrate provider into CodeActionService.

  • Enable via configuration flag remediation_agent_enabled.

  • Convert git diffs to WorkspaceEdits.


Diagram Walkthrough

flowchart LR
  A[Configuration: remediation_agent_enabled] --> B(RemyProvider);
  B --> C(CodeActionService);
  B -- Uses --> D{Remy CLI};
  D -- Mutates --> E[Git Worktree];
  E -- Generates --> F(Git Diff);
  F --> B;
Loading

File Walkthrough

Relevant files
Enhancement
init.go
Integrate Remy RemediationProvider into DI.                           

application/di/init.go

  • Imports the remediation package.
  • Initializes RemyProvider conditionally based on the
    remediation_agent_enabled configuration.
  • Injects the RemediationProvider into the CodeActionService.
  • Defines the remediationAgentEnabledKey constant.
+18/-1   
provider.go
Define RemediationProvider interface.                                       

domain/snyk/remediation/provider.go

  • Defines the RemediationProvider interface for autonomous fixing.
  • Adds an "Isolation contract" comment clarifying implementation
    responsibilities regarding worktree mutation.
+4/-0     
remy.go
Implement Remy RemediationProvider logic.                               

domain/snyk/snyk/remediation/remy.go

  • Implements the RemediationProvider interface using the snyk remy CLI.
  • Handles worktree mutation via a remyRunner (defaulting to subprocess
    execution).
  • Generates types.WorkspaceEdit by parsing git diffs between pre- and
    post-mutation states.
  • Includes helper functions for git operations, diff parsing, and
    subprocess execution.

@snyk-io

snyk-io Bot commented Jun 8, 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.

@bastiandoetsch

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

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

Destructive Workspace Mutation 🔴 [critical]

The initApplication function wires the RemyProvider using the live workspace folder path (w / folderPath). However, the RemediationProvider isolation contract (defined in provider.go) states that implementations may mutate the root in place and requires an isolated copy (e.g., a git worktree). By passing the live folder, snyk remy will modify the user's source code on disk immediately when the code action is resolved (typically upon clicking the lightbulb), before the user has reviewed or accepted the WorkspaceEdit. This results in a 'double-apply' where the IDE tries to apply a diff to a file that has already changed on disk, potentially causing conflicts or corrupting the buffer.

	remediationProvider = remediation.NewRemyProvider(remediation.RemyOptions{
		CliPath: config.GetCliPath(conf),
		Logger:  &lp,
	}, nil)
}

codeActionService = codeaction.NewService(engine, w, fileWatcher, notifier, featureFlagService, configResolver, remediationProvider)
Data Loss Risk 🔴 [critical]

The Remediate function uses git diff HEAD (via gitChangedFiles and gitFileDiff) to calculate the changes to package into a WorkspaceEdit. Because the provider is currently wired to the live workspace rather than an isolated worktree, this diff will include all of the user's uncommitted changes in the final WorkspaceEdit. When the IDE applies this edit, it may overwrite the user's intentional, uncommitted modifications with a state derived from HEAD plus the AI's changes, leading to silent data loss of the user's local work.

	cmd := exec.CommandContext(ctx, "git", "-C", root, "diff", "--name-only", "HEAD")
	out, err := cmd.Output()
	if err != nil {
		return nil, fmt.Errorf("git diff --name-only: %w", err)
	}
	var paths []string
	for _, p := range strings.Split(strings.TrimSpace(string(out)), "\n") {
		if p != "" {
			paths = append(paths, p)
		}
	}
	return paths, nil
}

// gitFileDiff returns the unified diff for relPath from HEAD to the working
// tree in the repository at root. Returns an empty string if the file is
// unchanged.
func gitFileDiff(ctx context.Context, root, relPath string) (string, error) {
	cmd := exec.CommandContext(ctx, "git", "-C", root, "diff", "HEAD", "--", relPath)
Logic Error in Diff Parser 🟠 [major]

In parseDiffHunks, the handling of the \ No newline at end of file marker is incorrect. It decrements s.currentLine without removing the newline character (\n) that was automatically appended to the preceding insertion in applyInsertion. This will cause the remediation agent to incorrectly add a trailing newline to files that do not have one. Furthermore, if the marker appears at the start of a file or hunk, currentLine can become negative, causing makeLineEdit to return an error and fail the entire remediation for that file.

if line == `\ No newline at end of file` {
	// Compensate for the line counter being one ahead of reality.
	s.currentLine--
	continue
}
Missing Auth Context 🟠 [major]

makeSubprocessRunner inherits the parent process environment via os.Environ(). In most language server deployments, the authentication token is provided via LSP configuration and stored in the LS memory/engine, not in the process's OS environment variables. Without threading the configResolver to inject the SNYK_TOKEN, the subprocess call to snyk remy will likely fail due to lack of authentication.

cmd.Env = os.Environ()
📚 Repository Context Analyzed

This review considered 18 relevant code sections from 14 files (average relevance: 0.92)

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (9bac438)

@bastiandoetsch

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (9bac438)

@bastiandoetsch

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (2d61f9d)

@bastiandoetsch

Copy link
Copy Markdown
Contributor Author

Addressing bot findings:

Destructive Workspace Mutation + Data Loss Risk: Both criticals are valid for this PR viewed in isolation. The worktree isolation that prevents live workspace mutation is introduced in the next PR in this stack (#1329 feat/IDE-2052-pr4), which clones ContentRoot into a detached git worktree before invoking remy. The RemediationProvider interface contract (provider.go:39-41) documents this as a caller responsibility — PR #1329 fulfils it. Not fixed in this PR; fixed in the stacked parent.

@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (37e017b)

Concrete RemediationProvider that shells out to `snyk remy <contentRoot>`
against an isolated git worktree. The worktree is caller-supplied; the
provider mutates it in place and returns the changes as a WorkspaceEdit
built from a before/after git diff.

Key design points:
- remyRunner seam allows unit testing without a real CLI
- context propagated to all git subprocess calls for cancellation
- ContentRoot validated as an absolute path before use
- product guard: Remy actions offered for Snyk Code only
- feature is nil-gated in production (remediation_agent_enabled=false)

Interface doc updated with the worktree isolation contract.
Track lastWasInsertion on diffState to reliably detect whether the
immediately preceding diff line was a '+' insertion when the
'\ No newline at end of file' marker appears.

The old code unconditionally decremented currentLine, which was wrong
for insertions (applyInsertion never advances the cursor). A first fix
attempt used Range.Start.Line == s.currentLine as a proxy but that has
a false-positive in multi-hunk diffs where a later hunk header resets
currentLine to a value coincidentally matching a prior insertion line.

With lastWasInsertion: if the preceding line was '+', strip the
synthetic trailing '\n' from the last TextEdit. If '-', compensate
with currentLine-- as before. Reset the flag on every non-insertion
diff line (hunk header, deletion, context).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

depends-on-1327 Depends on PR #1327

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants