fix: stop task wait hanging on stdin and clearing the terminal line - #633
Draft
NickJosevski wants to merge 1 commit into
Draft
fix: stop task wait hanging on stdin and clearing the terminal line#633NickJosevski wants to merge 1 commit into
NickJosevski wants to merge 1 commit into
Conversation
Two faults, both triggered by running `task wait` outside a plain interactive shell. `task wait` read stdin unconditionally to pick up piped task IDs. Reading blocks until the writer closes the pipe, and a caller that shells out with stdin attached, such as Ruby's Open3.popen3, usually holds it open for the lifetime of the child. The command therefore never issued a single request, no matter that the task had long finished. Only consult stdin when no task IDs were supplied; piping them in still works. The spinner was also installed based on interactivity read in NewClientFactoryFromConfig, which runs from main before cobra parses flags, so --no-prompt had not been applied yet and the spinner ran regardless. Backgrounding the command then left it erasing the terminal line every poll. The round-tripper now decides per request, once the flag has been applied. Setting CI=1 already worked, since that disables interactive mode early enough. Fixes #526 Fixes #290 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #526
Fixes #290
Two faults in
task wait, both triggered by running it somewhere other than a plain interactive shell. They looked like separate reports; they are separate causes, in the same command.#526 — hangs forever when shelled out to
task waitread stdin unconditionally to pick up piped task IDs:IsCalledFromPipe()is true whenever stdin is not a character device, andscanner.Scan()then blocks until the writer closes the pipe. A caller that shells out with stdin attached — Ruby'sOpen3.popen3, Python'ssubprocess.Popen— generally holds it open for the lifetime of the child, so the command never issued a single request, no matter that the task had long since finished.This also explains the reporter's observation that
runbook runworked fine from the same code:task waitis the only command in the CLI that callsReadValuesFromPipe.Fix: only consult stdin when no task IDs were given.
Piping still works:
echo ServerTasks-1 | octopus task waitbehaves as before.#290 — erases the terminal line when backgrounded
The spinner is installed in
NewClientFactoryFromConfig:That runs from
main.go:53, before cobra parses any flags.--no-promptdoes not disable interactive mode untilPersistentPreRunE(root.go:131), by which point the decision is already made — so--no-promptnever suppressed the spinner. Backgrounded, it redraws every poll by erasing the current terminal line, which is what made the terminal unusable.Measured on a pty, counting
ESC [ K(erase-line) sequences:--no-prompt, before--no-prompt, afterSetting
CI=1already worked, because that callsDisableInteractive()atmain.go:48, early enough to be seen. That difference is what pinned the cause.Fix: the round-tripper decides per request rather than at construction, once the flag has been applied.
Verification
Against a live instance, with a FIFO held open by a background writer to imitate
popen3, timing the CLI process itself rather than the shell pipeline — the first attempt measured the pipeline and looked like no change at all.Unit tests cover the resolution of task IDs versus stdin, and the spinner's per-request decision including the case where interactivity changes after the round-tripper is built, which is the actual bug.
🤖 Generated with Claude Code