feat: remy worktree isolation, per-root mutex, parseDiffHunks fix [IDE-2052]

[bastiandoetsch]

User description

Summary

• Worktree isolation: remy runs in a detached git worktree, not the live workspace
• GAF runner: invokes the "fix" workflow extension via workflow.Engine; changes discovered via git diff
• Per-root mutex: serializes concurrent remy invocations for the same ContentRoot
• Cache: stores edits for files not yet resolved; evicts empty entries after last file consumed
• cacheValid lock precondition documented; cacheValid called inside cacheMu
• tryServeFromCache: atomic read-validate-consume under cacheMu (no lock-split)
• Unit tests: 83.2% coverage (26 tests across guards, cache hit/miss, eviction, No-newline fix, concurrent mutex)

PR Stack — Merge Order

```mermaid
flowchart LR
main(["main"])
PR1["#1326 PR-1\nfindingId + code action kind"]
PR2["#1327 PR-2\nremy provider + multi-product"]
PR3["#1328 PR-3\nNo-newline fix + goimports"]
PR4["#1329 PR-4 ← YOU ARE HERE\nGAF runner + worktree + cache + tests"]
PR5["#1330 PR-5\nunit tests for remy"]
FNDID["#1331 PR-6\nintegration + smoke tests"]
main --> PR1 --> PR2 --> PR3 --> PR4 --> PR5 --> FNDID
style PR4 fill:#ffd700,color:#000
```

Depends on: #1328

Deferred to later PRs

• Integration and smoke tests (PR-6 / finding-id)

Test plan

• go test ./domain/snyk/remediation/... — 83.2% coverage
• make test — all packages pass
• make lint — 0 issues

---

PR Type

Enhancement, Bug fix

---

Description

• Introduce Remy remediation agent with detached git worktree isolation.

• Integrate Go Application Framework runner and implement file caching.

• Add per-root mutexes for concurrent remediation calls.

• Fix logic in parseDiffHunks for accurate diff processing.

---

Diagram Walkthrough

flowchart LR
  LSP["LSP Request (Code Action)"] --> Remediate
  Remediate --> CacheLookup["Check Cache"]
  CacheLookup -- Cache Miss --> CreateWorktree
  CreateWorktree --> Runner["Invoke Remy Runner (GAF)"]
  Runner --> GitWorktree["Git Worktree Operations"]
  GitWorktree --> DiffParse["Parse Diff (parseDiffHunks)"]
  DiffParse --> WorkspaceEdit["Build WorkspaceEdit"]
  Runner --> PopulateCache["Populate Cache (other files)"]
  WorkspaceEdit --> Remediate
  PopulateCache --> Remediate
  CacheLookup -- Cache Hit --> WorkspaceEdit
  LSP --> FileChange["LSP Event (didChange/didSave)"]
  FileChange --> InvalidateFile["Call InvalidateFile"]
  InvalidateFile --> CacheClear["Clear Cache Entry"]

Loading

File Walkthrough

	Relevant files
Enhancement	init.go Integrate remediation notifier into DI                                      application/di/init.go Adds remediationNotifier to the Dependencies struct. Initializes the remediationNotifier when the remediation agent is enabled. Wires the FileChangeNotifier interface into the dependency injection system. +23/-19  server.go LSP handlers trigger file change invalidation                        application/server/server.go Passes the onFileChange callback (RemediationNotifier.InvalidateFile) to LSP handlers. textDocument/didChange and textDocument/didSave handlers now invoke onFileChange. +15/-4    provider.go Define FileChangeNotifier interface                                            domain/snyk/remediation/provider.go Introduces the FileChangeNotifier interface with the InvalidateFile method. Defines the contract for providers that cache results and need to invalidate them. +7/-0      remy.go Core Remy provider logic with GAF, cache, and concurrency domain/snyk/remediation/remy.go Implements worktree isolation using git worktree add --detach. Integrates Go Application Framework (GAF) for running the "fix" workflow. Adds caching mechanism for previously computed edits to avoid re-computation. Introduces per-root mutexes to serialize concurrent Remediate calls for the same root. Fixes parseDiffHunks by using an inHunk flag to correctly handle diff lines. Implements FileChangeNotifier interface. Adds resolveGitRoot to handle monorepos. +248/-132
Tests	export_for_test.go Export diff parsing helper for tests                                          domain/snyk/remediation/export_for_test.go New file exporting workspaceEditFromContent for testing purposes. +25/-0    remy_provider_test.go Unit tests for remediation provider                                            domain/snyk/remediation/remy_provider_test.go Adds comprehensive unit tests for the remyProvider. Covers guard conditions, runner errors, cache hits/misses, and eviction. Tests concurrency handling and file change invalidation logic. Includes tests for diff parsing edge cases using the exported helper. +694/-0

---

[snyk-io]

✅ 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]

/describe

[snyk-pr-review-bot]

PR Description updated to latest commit (8c0d6a5)

[snyk-pr-review-bot]

PR Reviewer Guide 🔍

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Global Lock Contention 🟠 [major] The tryServeFromCache function holds the global cacheMu lock while calling cacheValid, which performs synchronous os.Stat calls for every file in a cache entry. Since InvalidateFile (called on every textDocument/didChange LSP notification) also requires this lock, typing responsiveness in the IDE will be blocked by file I/O performed during remediation requests. This creates a bottleneck that directly impacts the perceived performance of the editor. p.cacheMu.Lock() defer p.cacheMu.Unlock() entry, exists := p.cache[root] if !exists || !p.cacheValid(entry) { return nil, false } Inefficient Stale Cache Handling 🟡 [minor] In tryServeFromCache, if p.cacheValid(entry) returns false (meaning the cache is stale due to disk modifications), the function returns false but leaves the stale entry in the p.cache map. Subsequent calls for other files in the same root will repeatedly perform redundant os.Stat calls on the same stale entry until a full Remediate cycle completes and overwrites the entry. The stale entry should be deleted immediately when cacheValid fails. if !exists || !p.cacheValid(entry) { return nil, false

📚 Repository Context Analyzed This review considered 21 relevant code sections from 13 files (average relevance: 0.97) domain/ide/workspace/folder.go (lines 1-225) application/codeaction/codeaction.go (lines 1-227) infrastructure/code/types.go (lines 1-117) application/server/server.go (lines 1-204) application/server/configuration.go (lines 1-197) infrastructure/code/code.go (lines 1-163) domain/ide/command/code_fix.go (lines 1-148) ... and 6 more file(s)

[bastiandoetsch]

/describe

[bastiandoetsch]

/describe

[snyk-pr-review-bot]

PR Description updated to latest commit (c1286ff)

[snyk-pr-review-bot]

PR Description updated to latest commit (c1286ff)

[bastiandoetsch]

/describe

[snyk-pr-review-bot]

PR Description updated to latest commit (4ab7fd6)

[bastiandoetsch]

/describe

[snyk-pr-review-bot]

PR Description updated to latest commit (4ab7fd6)

[bastiandoetsch]

Addressing bot finding — Global Lock Contention:

The tryServeFromCache function holds cacheMu during cacheValid (which calls os.Stat). This was reviewed and deliberately kept as the atomic read-validate-consume design rather than a three-phase lock-split.

Rationale: The lock-split alternative (snapshot paths, unlock, stat, relock) introduces a TOCTOU race where a concurrent Remediate call can populate a new cache entry for the same root between the stat pass and the re-acquire. The new entry would be served without freshness validation. The adversarial reviewer confirmed this as a correctness defect more severe than the latency concern.

InvalidateFile (called on textDocument/didChange) holds cacheMu only for a fast map delete — no I/O. The stat calls in cacheValid are bounded to the number of files changed by one remy run (typically 1–5 files). On a local filesystem this is microseconds. If os.Stat latency is measured to be a real problem (e.g., on NFS), a generation-counter approach can be added in a follow-up without the TOCTOU risk.

Keeping the simple design. No code change.

[basti-snyk]

/describe

[snyk-pr-review-bot]

PR Description updated to latest commit (131f83f)