Conversation
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if errors.Is(err, context.DeadlineExceeded) { | ||
| log.Printf("[%s] hook execution timeout: %v", requestID, err) | ||
| w.WriteHeader(http.StatusRequestTimeout) | ||
| fmt.Fprint(w, "Hook execution timeout. Please check your logs for more details.") |
There was a problem hiding this comment.
Streaming mode WriteHeader called after output written
When StreamCommandOutput is enabled, the command's stdout/stderr are written directly to the ResponseWriter w. If the command produces any output before timing out, w.WriteHeader(http.StatusRequestTimeout) will silently fail because HTTP headers must be sent before body content. The client will receive a 200 OK status (the default when WriteHeader isn't called before the first Write) with partial command output followed by the timeout error message, which is misleading.
| // 如果写入失败，立即清理这个文件 | ||
| if removeErr := os.Remove(tmpfile.Name()); removeErr != nil { | ||
| log.Printf("[%s] error removing failed temp file %s [%s]", r.ID, tmpfile.Name(), removeErr) | ||
| } |
There was a problem hiding this comment.
File handle not closed before removal attempt
When tmpfile.Write() fails, the code attempts to remove the file without first closing the file handle. This causes a file descriptor leak since tmpfile is never closed. Additionally, on Windows, os.Remove() will fail because the file is still open (Windows doesn't allow deleting open files). The tmpfile.Close() call needs to happen before the removal attempt.
| if errors.Is(err, context.DeadlineExceeded) { | ||
| log.Printf("[%s] hook execution timeout: %v", requestID, err) | ||
| fmt.Fprint(w, "Hook execution timeout. Please check your logs for more details.") | ||
| } else if matchedHook.CaptureCommandOutputOnError { |
There was a problem hiding this comment.
Timeout errors return wrong HTTP status code in capture mode
In CaptureCommandOutput mode, w.WriteHeader(http.StatusInternalServerError) is called unconditionally before checking the error type. This means timeout errors (context.DeadlineExceeded) incorrectly return HTTP 500 instead of HTTP 408 (StatusRequestTimeout). This is inconsistent with the streaming mode which correctly returns 408 for timeouts. The status code determination needs to happen before WriteHeader is called.
Note
Adds configurable execution limits and context-aware control for hooks, with corresponding CLI/env options and documentation.
HookExecutorwith semaphore-based concurrency control and per-exec timeout; integrated into request flowhandleHooknow context-aware (exec.CommandContext), with improved temp file cleanup and streaming response tracking-hook-timeout-seconds,-max-concurrent-hooks,-hook-execution-timeout(and corresponding env vars).gitignoreupdatedWritten by Cursor Bugbot for commit d3f90f8. This will update automatically on new commits. Configure here.