github / github/gh-stack

Push discards pre-push hook output, so an interactive hook looks like a hang

Đang mở
#420 0 bình luận 2 reaction 0 người được giao Xem trên GitHub
bug topic: cli - push
Ngôn ngữ chính
Go
Star
1.5k
Fork
70
Merge trung bình
1 ngày 8 giờ
Pull request đã merge (30 ngày)
7

Mô tả

## Summary

Every command that pushes (`sync`, `push`, `submit`, `link`) runs `git push` with stdout sent to `/dev/null` and stderr sent to an in-memory buffer that is only surfaced if the push fails. A pre-push hook that prompts for input therefore prints its question into the void while blocking on `/dev/tty`, and `gh stack sync` appears to hang forever with no output and no timeout.

The same discarding affects non-interactive hooks: their logging and git's own transfer progress never appear, so a merely slow hook (a test suite, a linter) is indistinguishable from a hang.

## Steps to reproduce

1. In a repo with a stack, add `.git/hooks/pre-push`:

```sh
#!/bin/sh
echo "Run the full test suite? [y/N] "
read ans < /dev/tty
```

2. Run `gh stack sync`.

**Expected:** the hook's prompt appears and you can answer it.

**Actual:** output stops after `Pushing N branches to origin...` and the command waits indefinitely. Typing `y` + Enter unblocks it, but nothing on screen indicates input is wanted.

## Root cause

`internal/git/gitops.go` → `defaultOps.Push` ends in `runSilent`, which routes through the package-level client in `internal/git/git.go:16`:

```go
var client = &cligit.Client{}
```

The client is zero-valued, so `Stdin`, `Stdout`, and `Stderr` are nil, and `Client.Command` copies them straight onto the `exec.Cmd` (`cli/cli/v2/git/client.go:95-97`). Nil `Stdout` on an `exec.Cmd` means `/dev/null`; `Command.Run` swaps nil `Stderr` for a `bytes.Buffer` (`cli/cli/v2/git/command.go:20-23`) that is only read back on failure. The hook process still inherits the controlling terminal, so `read < /dev/tty` blocks on real keystrokes — but the prompt it wrote to stdout/stderr is gone. `context.Background()` with no timeout means the wait is unbounded.

Verified with a pre-push hook that wrote to stdout, stderr, and a log file: the log confirmed the hook ran, and neither stream reached the terminal.

The codebase already has the right helper for this — `runInteractive` (`internal/git/git.go:60`) wires `os.Stdin/Stdout/Stderr` and is used for `git commit`. Push never got equivalent treatment.

## Suggested fix

Have `Push` run through `runInteractive` so hook prompts, hook output, and git's transfer progress reach the terminal as they are produced. Since git and the hook have then already written their own messages to stderr, the returned error should carry only the exit status rather than folding the buffered stderr back in and printing it twice.

I have this working locally with integration tests covering a real pre-push hook (output visible on success; stderr visible and an error returned when the hook rejects the push). Happy to open a PR if that's welcome, or leave it here for a maintainer to pick up.

## Related

- #135 and #293 both ask for `--no-verify`. This is a separate problem: those are about *skipping* hooks, this is about hooks that run correctly but invisibly. A `--no-verify` flag would not fix the invisible-prompt hang for people who need their hooks to run.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Read internal/git/gitops.go around defaultOps.Push and internal/git/git.go around runInteractive, then compare the existing interactive commit path. Reproduce the pre-push hook scenario and run the integration tests; done means hook prompts, output, and git progress reach the terminal, while rejected pushes still return an error without duplicate stderr.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
go
Lĩnh vực
cli
Loại issue
Lỗi
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
76/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.