Skip to content

feat: add snyk.remediationAgent.fixFolder command [IDE-2052]#1331

Draft
bastiandoetsch wants to merge 4 commits into
feat/IDE-2052-pr5from
feat/IDE-2052-finding-id
Draft

feat: add snyk.remediationAgent.fixFolder command [IDE-2052]#1331
bastiandoetsch wants to merge 4 commits into
feat/IDE-2052-pr5from
feat/IDE-2052-finding-id

Conversation

@bastiandoetsch

@bastiandoetsch bastiandoetsch commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

User description

Summary

Adds the LSP workspace/executeCommand snyk.remediationAgent.fixFolder plus the remy provider integration/smoke tests.

  • New command takes exactly one file:// folder-URI argument and runs the remediation agent's fix workflow in place in that folder, which is a caller-created git worktree (a detached-HEAD clone of the workspace root).
  • Results are delivered to the client via workspace/applyEdit, with edit paths keyed under the passed folder so the caller's path-prefix remap matches; the executeCommand response is nil on success, an error on failure.
  • The fix-on-a-directory capability reuses the existing remy provider via a new optional FolderRemediator interface (discovered by type assertion, mirroring FileChangeNotifier); shared snapshot→run→diff logic is factored into collectFixEdits so the per-finding and folder paths don't duplicate it.
  • Feature-gated: nil provider → returns an error (never silent success). Requires the client to advertise workspace/applyEdit. FixFolder requires the passed folder to be the git repository root and rejects subdirectories explicitly rather than silently dropping edits.
  • Wires the remediation provider through command.NewService into the command factory, closing a gap where it was constructed but never reached the command service.

PR Stack — Merge Order

flowchart LR
    main(["main"])
    PR0["#1330\nremy unit tests"]
    PR1["#1331 ← YOU ARE HERE\nfixFolder command + remy integ/smoke tests"]
    main --> PR0 --> PR1
    style PR1 fill:#ffd700,color:#000
Loading

Depends on: #1330

Testing

  • Build, vet, lint, unit, and integration/DI-wiring tests pass. Outside-in TDD: acceptance round-trip through the real command service, factory/DI wiring tests, and unit tests for arg validation, URI conversion, in-place execution, applyEdit delivery, feature-gate-off, git-root precondition.
  • make test recorded for the commit.

Known environmental caveat

  • Test_Smoke_RemediationAgent_CodeAction (added in this stack) does not complete in the local dev environment: it fails in shared setup while downloading the ~149 MB Snyk CLI binary (unexpected EOF) and cloning fixture repos (git clone exit 128). These are network/infra limitations of the local environment, not code defects — the failure occurs before any remediation logic runs. CI (with reliable network) should exercise it.

Test plan

  • CI green
  • Smoke test passes in CI environment

PR Type

enhancement


Description

  • Implement snyk.remediationAgent.fixFolder command.

  • Add FolderRemediator interface and Remy provider implementation.

  • Integrate into DI, command service, and LSP handlers.

  • Enhance cache validation with content hashing; add extensive tests.


Diagram Walkthrough

flowchart LR
  Client --> CommandService
  CommandService --> CommandFactory
  CommandFactory --> RemediationFixFolderCommand["remediationFixFolderCommand\n(command/remediation_fix_folder.go)"]
  RemediationFixFolderCommand --> FolderRemediator["FolderRemediator\n(domain/snyk/remediation/provider.go)"]
  FolderRemediator --> RemyProvider["remyProvider.FixFolder\n(domain/snyk/remediation/remy.go)"]
  RemyProvider --> GitCommands["Git commands\n(internal operations)"]
  RemyProvider --> Cache["Cache validation\n(content hashing)"]
  RemediationFixFolderCommand --> ApplyEdit["workspace/applyEdit\n(via Notifier)"]
Loading

File Walkthrough

Relevant files
Documentation
1 files
command.go
Define new command constant for fixFolder                               
+3/-0     
Enhancement
7 files
provider.go
Define FolderRemediator interface                                               
+16/-0   
remy.go
Implement FixFolder, enhance cache validation                       
+204/-33
remediation_fix_folder.go
Implement fixFolder command handler logic                               
+97/-0   
init.go
Wire FolderRemediator into command service                             
+5/-1     
command_service.go
Add FolderRemediator to CommandService                                     
+29/-26 
command_factory.go
Create remediationFixFolderCommand handler                             
+9/-0     
server.go
Register fixFolder command in ExecuteCommandProvider         
+1/-0     
Tests
11 files
remediation_fix_folder_test.go
Unit tests for remediationFixFolderCommand logic                 
+453/-0 
remy_fix_folder_test.go
Unit tests for remediation.FixFolder implementation           
+318/-0 
remy_provider_test.go
Tests for cache validation and path handling                         
+269/-13
init_fix_folder_test.go
Integration tests for FixFolder command wiring                     
+148/-0 
server_test.go
Integration tests for command execution and capabilities 
+174/-0 
server_smoke_test.go
Smoke test for RemediationAgent quickfix roundtrip             
+137/-0 
command_factory_test.go
Test CreateFromCommandData wires provider and notifier     
+81/-0   
export_for_test.go
Export helpers for testing remediation provider                   
+63/-1   
export_for_test.go
Export command constructors for testing                                   
+56/-0   
remediation_test.go
Add tests for non-AI-fixable and non-Code issues                 
+42/-0   
converter_test.go
Add tests for various LSP type conversions                             
+174/-0 
Additional files
5 files
authentication_flows_e2e_test.go +1/-0     
execute_command_test.go +1/-1     
command_service_test.go +1/-1     
remy_test.go +299/-20
command_remediation_test.go +33/-0   

@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 bastiandoetsch added the depends-on-1330 Depends on PR #1330 label Jun 8, 2026
@bastiandoetsch

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (8eb44e4)

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

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

Brittle Command Execution 🟡 [minor]

In substituteRemyFlow, the stub callback manually constructs an exec.Command using previewCLIPath and hardcoded flags --agentic, --experimental, and --auto-approve. While this mirrors the intended CLI behavior, it bypasses the standard GAF configuration and snykCli executor used elsewhere in the codebase, making the test sensitive to CLI flag changes that aren't reflected in the LS configuration.

cmd := exec.CommandContext(t.Context(), previewCLIPath,
	"fix", contentRoot,
	"--agentic", "--experimental", "--auto-approve",
)
📚 Repository Context Analyzed

This review considered 23 relevant code sections from 14 files (average relevance: 1.00)

@bastiandoetsch bastiandoetsch marked this pull request as draft June 8, 2026 12:17
@bastiandoetsch

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (a5d4002)

@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (6095425)

@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (e36ab05)

@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (e36ab05)

…052]

Add an LSP workspace/executeCommand named snyk.remediationAgent.fixFolder
that takes one file:// folder URI and runs the remediation agent's fix
workflow in place in that folder, delivering the resulting changes to the
client via workspace/applyEdit. The folder is a caller-created git worktree
(a detached-HEAD clone of the workspace root); the command runs the fix
directly in it without creating a nested worktree, so edit paths stay keyed
under the passed folder for the caller's prefix remap.

The fix-on-a-directory capability reuses the existing remy provider via a
new optional FolderRemediator interface (discovered by type assertion,
mirroring FileChangeNotifier); the shared snapshot-run-diff logic is
factored into collectFixEdits so the per-finding and folder paths do not
duplicate it. The command is feature-gated: when the remediation provider
is absent it returns an error rather than silently succeeding, and it
requires the client to support workspace/applyEdit. FixFolder requires the
passed folder to be the git repository root and rejects subdirectories
explicitly instead of silently dropping edits.

Also wires the remediation provider through command.NewService into the
command factory, closing a gap where it was constructed but never reached
the command service.
@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (e36ab05)

@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (119ee0a)

@basti-snyk basti-snyk changed the title test: add integration and smoke tests for remy provider [IDE-2052] feat: add snyk.remediationAgent.fixFolder command [IDE-2052] Jun 29, 2026
@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (119ee0a)

…IDE-2052]

The remy provider's per-file cache marked an entry stale only when a
cached file's mtime was strictly after the entry's wall-clock creation
time. On filesystems with coarse mtime granularity, a file modified just
after the entry was created could carry an mtime that, once truncated,
sorted before the nanosecond-precise creation time — so a genuinely
changed file was treated as fresh and stale edits were retained. This
made TestCacheValid_StaleFile fail nondeterministically on CI.

Replace the timestamp comparison with a per-file SHA-256 content hash
captured when the entry is populated and recompared on each lookup: any
content change (including same-length edits that mtime/size checks miss)
now reliably invalidates the entry, independent of filesystem timestamp
resolution. The requested file's presence is checked before any hashing
so a miss does no I/O, an empty entry is never stored, and the content
and hash maps are kept in sync on every mutation.

Also set core.checkStat=minimal in the fixFolder git test helpers
(matching the remy test helper) to avoid an overlay-filesystem
write-ordering flake when reading freshly written git objects in CI.
@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (87adbe8)

@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (87adbe8)

…ws [IDE-2052]

The remy provider mixed non-canonical temporary paths with git's canonical
paths. On macOS the temp dir is under /var (a symlink to /private/var) and on
Windows it is an 8.3 short name; git rev-parse --show-toplevel returns the
resolved form. The divergence made the per-finding Remediate flow's worktree
diff and cache-key matching fail, so it returned an empty WorkspaceEdit on
those platforms while passing on Linux. The failure was masked until now by an
unrelated flaky test that aborted the package run first.

Canonicalize paths with filepath.EvalSymlinks where the worktree flow needs
git's view: Remediate resolves ContentRoot and FilePath (so cache write,
serve, and invalidate keys agree), and runRemyInWorktree resolves the
MkdirTemp parent. InvalidateFile resolves its path too, falling back to the
parent directory when the file has already been deleted. FixFolder
deliberately does NOT canonicalize: its caller remaps edits by the exact
folder path it passed, so edits must stay keyed under that path; it runs git
directly in that folder and needs no resolution.

Also: guard NewRemyProvider against a nil runner with a nil engine, lower the
empty-diff diagnostic to debug, and fix a folder-URI test that used a path
that is not absolute on Windows. Regression tests reproduce the symlink class
on Linux for Remediate, InvalidateFile (including the deleted-file path), and
the FixFolder passed-path contract.
@basti-snyk

Copy link
Copy Markdown
Contributor

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (a54cb36)

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

Labels

depends-on-1330 Depends on PR #1330

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants