fix: add timeouts to IPC operations to prevent shell hook hangs#106
Conversation
Fixes #26
The shell hook (`pitchfork cd`) could hang indefinitely if the IPC
connection had issues, causing shell startup to freeze.
Changes:
- Add 5 second timeout to IPC request/response operations
- Add timeout to read operations with proper error handling
- Use dynamic timeout for daemon start based on ready_delay
- Remove infinite loop in request() that could hang forever
- Return proper errors instead of Option for better diagnostics
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| let start_time = chrono::Local::now(); | ||
| let rsp = self.request(IpcRequest::Run(opts.clone())).await?; | ||
| // Use longer timeout for daemon start - ready_delay can be up to 60s+ | ||
| let timeout = Duration::from_secs(opts.ready_delay.unwrap_or(3) + 60); |
There was a problem hiding this comment.
Timeout calculation ignores retry backoff time
Medium Severity
The run method timeout calculation opts.ready_delay.unwrap_or(3) + 60 only accounts for a single ready_delay period. When retry > 0 is configured, the server-side run performs multiple attempts with exponential backoff between failures. The total server time can be (retry + 1) × ready_delay + (2^retry - 1) seconds, which can exceed the client timeout. For instance, retry = 5 and ready_delay = 10 yields ~91 seconds server-side but only 70 seconds client timeout, causing spurious timeout errors before the server completes.
## 🤖 New release * `pitchfork-cli`: 0.2.1 -> 0.3.0 <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.3.0](v0.2.1...v0.3.0) - 2026-01-18 ### Added - *(web)* add devilish pitchfork theming to web UI ([#115](#115)) - *(web)* add web UI for daemon management ([#112](#112)) - show startup logs on successful daemon start ([#111](#111)) - add HTTP ready check for daemon startup ([#110](#110)) - delay autostopping daemons when leaving directory ([#108](#108)) - *(logs)* clear all logs when no daemon specified ([#109](#109)) - *(list)* show error messages in daemon list output ([#107](#107)) - refactor the code structure of `start` and `run`, allowing for parallel starting daemons ([#56](#56)) - [**breaking**] support auto start on boot ([#53](#53)) - print logs when failed on `pf start|run` ([#52](#52)) - [**breaking**] support global system/user config ([#46](#46)) - *(test)* refactor tests and add tests for `interval_watch` and `cron_watch` ([#45](#45)) ### Fixed - add timeouts to IPC operations to prevent shell hook hangs ([#106](#106)) - *(deps)* update rust crate toml to 0.9 ([#50](#50)) - replace panics with proper error handling ([#90](#90)) - *(deps)* update rust crate notify to v8 ([#78](#78)) - *(deps)* update rust crate duct to v1 ([#72](#72)) - *(deps)* update rust crate dirs to v6 ([#64](#64)) - *(deps)* update rust crate cron to 0.15 ([#48](#48)) - *(deps)* update rust crate sysinfo to 0.37 ([#49](#49)) - *(deps)* update rust crate itertools to 0.14 ([#33](#33)) - *(deps)* update rust crate strum to 0.27 ([#35](#35)) - *(deps)* update rust crate console to 0.16 ([#32](#32)) - give a user-friendly error when the work fails ([#44](#44)) ### Other - *(cli)* add long_about with examples to CLI commands ([#91](#91)) - fix documentation issues and inconsistencies ([#89](#89)) - *(deps)* lock file maintenance ([#88](#88)) - *(deps)* update rust crate serde_json to v1.0.149 ([#87](#87)) - *(deps)* lock file maintenance ([#85](#85)) - *(deps)* update rust crate serde_json to v1.0.148 ([#84](#84)) - *(deps)* update rust crate tempfile to v3.24.0 ([#82](#82)) - *(deps)* update rust crate rmp-serde to v1.3.1 ([#80](#80)) - *(deps)* update rust crate serde_json to v1.0.147 ([#81](#81)) - *(deps)* lock file maintenance ([#79](#79)) - *(deps)* update rust crate shell-words to v1.1.1 ([#77](#77)) - *(deps)* lock file maintenance ([#76](#76)) - *(deps)* update rust crate log to v0.4.29 ([#75](#75)) - *(deps)* lock file maintenance ([#73](#73)) - *(deps)* lock file maintenance ([#68](#68)) - *(deps)* lock file maintenance ([#65](#65)) - *(deps)* lock file maintenance ([#62](#62)) - *(deps)* update rust crate clap to v4.5.51 ([#60](#60)) - *(deps)* lock file maintenance ([#59](#59)) - *(deps)* update rust crate clap to v4.5.50 ([#57](#57)) - Update README ([#55](#55)) - *(deps)* lock file maintenance ([#54](#54)) - *(deps)* lock file maintenance ([#47](#47)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixes #26
Summary
The shell hook (
pitchfork cd) could hang indefinitely if the IPC connection had issues, causing shell startup to freeze on macOS 15.4 and potentially other systems.Root Cause
The
request()method had an infinite loop waiting for IPC responses with no timeout:If
read()returnedNoneor blocked indefinitely, the shell hook would hang forever.Changes
ready_delayconfigResulterrors instead ofOptionfor better diagnosticsTest plan
cargo buildpassescargo clippy -- -D warningspassescargo testpasses (34 tests)🤖 Generated with Claude Code
Note
Adds bounded waits and better error propagation to IPC.
REQUEST_TIMEOUT(5s) andread(timeout)withtokio::time::timeout, returningResulton errors/timeoutsrequest_with_timeout(...)used byrequest(...)run(...)now uses a longer timeout derived fromready_delay + 60sfor daemon startupResulterrorsWritten by Cursor Bugbot for commit cc66765. This will update automatically on new commits. Configure here.