Skip to content

fix(hooks): materialize proxied hook stdin into a temp file#108

Merged
pol-rivero merged 2 commits into
pol-rivero:mainfrom
ignatremizov:fix/hooks-proxy-stdin-file-upstream
Mar 27, 2026
Merged

fix(hooks): materialize proxied hook stdin into a temp file#108
pol-rivero merged 2 commits into
pol-rivero:mainfrom
ignatremizov:fix/hooks-proxy-stdin-file-upstream

Conversation

@ignatremizov

@ignatremizov ignatremizov commented Mar 26, 2026

Copy link
Copy Markdown

Summary

Fix Linux hook proxy stdin forwarding in GitHub Desktop Plus by replacing fragile fd-backed hook stdin paths with a FIFO-backed handoff that preserves streaming behavior and cleans up correctly across success, failure, and cancellation paths.

Problem

GUI-driven Git operations on Linux could fail before repository hook logic ran normally because the hook proxy passed stdin to git hook run through pseudo-paths that were not reliably reopenable in the Electron/process-proxy environment.

Observed failures:

  • fatal: could not open '/dev/stdin' for reading: No such device or address
  • fatal: could not open '/proc/self/fd/0' for reading: No such device or address

A naive temp-file buffering approach also introduced its own regressions around stdin semantics, cleanup, and partial-read hooks.

Changes

  • replace Linux hook stdin forwarding with a temporary FIFO under the system temp directory instead of /dev/stdin or /proc/self/fd/0
  • start the FIFO writer only after spawn(...) succeeds so pre-spawn failures cannot deadlock on a writer waiting for its first reader
  • stream proxy stdin into that FIFO so hook stdin behavior stays streaming instead of fully buffering the payload before hook startup
  • attach an immediate handler to the FIFO forward promise so early reader disconnects or aborts cannot surface as unhandled promise rejections
  • treat hook completion as authoritative instead of waiting for the FIFO writer to drain, so hooks that intentionally stop reading stdin early do not hang the proxy on successful exit
  • surface unexpected stdin transport failures before reporting hook success, while still treating broken-pipe and abort conditions as expected during teardown
  • keep non-Linux behavior on /dev/stdin unchanged
  • register the proxy abort handler before stdin forwarding work begins so GUI-side cancellation still propagates during hook setup
  • wrap the FIFO lifecycle in try/finally and always clean up the temporary directory after hook execution

Behavioral effect

Hooks that depend on stdin still receive the expected payload, but Linux GUI clients no longer depend on reopening /dev/stdin or /proc/self/fd/0 through the Electron/process-proxy stack.

Verification

  • reproduced the failure locally through GitHub Desktop Plus during a hook-triggering push
  • confirmed that both /dev/stdin and /proc/self/fd/0 could fail in the packaged Desktop hook path even when terminal Git worked
  • rebuilt and reinstalled GitHub Desktop Plus with the hardened FIFO-based proxy change
  • verified that GUI-driven push flow now succeeds without hook proxy stdin errors
  • ran focused validation with yarn eslint app/src/lib/hooks/hooks-proxy.ts
  • ran repeated local LLM reviewer passes and addressed follow-up issues around cleanup, partial-read hooks, pre-spawn deadlocks, and unhandled forward-promise rejections

@ignatremizov ignatremizov force-pushed the fix/hooks-proxy-stdin-file-upstream branch 5 times, most recently from f144044 to 44799b9 Compare March 26, 2026 23:52
Avoid forwarding hook stdin through `/dev/stdin` or `/proc/self/fd/0` when GitHub Desktop Plus runs Git hooks through the process proxy on Linux.

- create a temporary FIFO under the system temp directory for Linux hook stdin forwarding instead of relying on fd-backed pseudo-paths that `git hook run` may fail to reopen
- start the FIFO writer only after `spawn(...)` succeeds so pre-spawn failures cannot deadlock on a writer waiting for its first reader
- attach an immediate handler to the FIFO forward promise so early reader disconnects or aborts cannot surface as unhandled promise rejections
- surface unexpected stdin transport failures before reporting hook success, while still treating broken-pipe and abort conditions as expected during teardown
- stream the proxy connection stdin into that FIFO so hook stdin behavior stays streaming instead of fully buffering the payload before hook startup
- keep non-Linux behavior on `/dev/stdin` unchanged
- register the proxy abort handler before stdin forwarding work begins so GUI-side cancellation still propagates during hook setup
- wrap the temporary FIFO lifecycle in `try/finally`, abort the writer on early exit, and always clean up the temp directory after hook execution
- treat hook completion as authoritative instead of waiting for the FIFO writer to drain, so hooks that stop reading stdin early do not hang the proxy on successful exit

Behavioral effect:
Hooks that depend on stdin still receive the expected payload, but Linux GUI clients no longer depend on reopening `/dev/stdin` or `/proc/self/fd/0` through the Electron/process-proxy stack.
@ignatremizov ignatremizov force-pushed the fix/hooks-proxy-stdin-file-upstream branch from 44799b9 to b64f7a3 Compare March 27, 2026 00:09
@pol-rivero pol-rivero linked an issue Mar 27, 2026 that may be closed by this pull request
@pol-rivero

Copy link
Copy Markdown
Owner

All your changes are reasonable, and it certainly works as advertised. Thank you!

@pol-rivero pol-rivero merged commit fcb24a3 into pol-rivero:main Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Push with hooks warning: fail to read /dev/stdin

2 participants