fix: LSP git diff subprocess timeout (#13)

[EffortlessSteven]

Fix #13: LSP git diff subprocess timeout

Problem

Command::new("git").output() blocks the LSP server indefinitely on large repos. No timeout configured.

Fix

Changed run_git_diff() in crates/diffguard-lsp/src/server.rs:

• Spawn the process + poll try_wait() in a loop (100ms intervals)
• 10-second maximum timeout before killing the child process
• No new dependencies required (uses only std::process::Command, std::thread, std::time::Instant)

Verification

cargo build -p diffguard-lsp   ✓  
cargo clippy -p diffguard-lsp  ✓
cargo fmt --check              ✓
cargo test -p diffguard-lsp    ✓  (9 tests)

Also included

Fixes a race condition in cmd_validate test by adding ENV_LOCK to prevent parallel test interference

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cf13c37a-6336-4509-a05a-0dd217348af6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/lsp-git-diff-timeout

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request introduces a 10-second timeout for the git diff command in the LSP server and adds a mutex lock to a CLI test to prevent environment variable leakage during parallel execution. Feedback focuses on the git diff implementation, noting that the child process should capture output to avoid corrupting the LSP stream, must be reaped after being killed to prevent zombie processes, and should avoid blocking the main thread or deadlocking on pipe buffers.

[gemini-code-assist]

Using command.spawn() without capturing output causes the child process to inherit the LSP server's standard streams. Since the LSP communicates with the client via stdout, any output from git diff will be written directly to the communication channel, corrupting the protocol stream and likely causing the client to disconnect. You must configure the command to capture stdout and stderr using Stdio::piped().

    let mut child = command
        .stdout(std::process::Stdio::piped())
        .stderr(std::process::Stdio::piped())
        .spawn()
        .context("spawn git diff")?;

[gemini-code-assist]

Failing to reap a killed process leaves a 'zombie' in the process table. In a long-running LSP server, this can lead to resource exhaustion over time. It is important to call child.wait() after child.kill() to ensure the process is properly reaped by the operating system.

Suggested change

	if Instant::now() >= deadline {
	let _ = child.kill();
	bail!("git diff timed out after {}s", GIT_DIFF_TIMEOUT.as_secs());
	}
	if Instant::now() >= deadline {
	let _ = child.kill();
	let _ = child.wait();
	bail!("git diff timed out after {}s", GIT_DIFF_TIMEOUT.as_secs());
	}

[gemini-code-assist]

This polling loop blocks the main LSP message processing thread for up to 10 seconds, making the server unresponsive to other requests (such as cancellations) during this time. Furthermore, if the git diff output exceeds the system's pipe buffer size (often 64KB), the child process will block indefinitely because the loop does not read from the pipes while waiting. This will eventually trigger the timeout and cause the operation to fail even if it would have succeeded otherwise. Consider offloading the process execution to a separate thread and using a channel with recv_timeout to handle the result safely.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f749b1b44d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Pipe git diff output before waiting

run_git_diff now uses command.spawn() without configuring stdio, so git inherits the LSP process stdout/stderr. In Rust this means wait_with_output() returns empty buffers, so successful diffs are seen as empty and diagnostics are suppressed, and the raw unified diff is emitted onto the server’s stdout stream (which can corrupt stdio-based LSP messaging) whenever a file has git changes.

Useful? React with 👍 / 👎.