From 7d9eaead543c1b57768ed0899b67f85133a78443 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 29 Jul 2025 10:43:42 +0200 Subject: [PATCH] Close the pty instead of killing the process for runAndDetectCredentialRequest As with the previous commit, this is not strictly necessary for anything, just cleaner. --- pkg/commands/oscommands/cmd_obj_runner.go | 38 ++++++++++--------- .../oscommands/cmd_obj_runner_test.go | 2 +- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/pkg/commands/oscommands/cmd_obj_runner.go b/pkg/commands/oscommands/cmd_obj_runner.go index 34430b2ae..b19ec89b5 100644 --- a/pkg/commands/oscommands/cmd_obj_runner.go +++ b/pkg/commands/oscommands/cmd_obj_runner.go @@ -336,7 +336,7 @@ func (self *cmdObjRunner) runAndDetectCredentialRequest( tr := io.TeeReader(handler.stdoutPipe, cmdWriter) go utils.Safe(func() { - self.processOutput(tr, handler.stdinPipe, promptUserForCredential, cmdObj) + self.processOutput(tr, handler.stdinPipe, promptUserForCredential, handler.close, cmdObj) }) }) } @@ -345,6 +345,7 @@ func (self *cmdObjRunner) processOutput( reader io.Reader, writer io.Writer, promptUserForCredential func(CredentialType) <-chan string, + closeFunc func() error, cmdObj *CmdObj, ) { checkForCredentialRequest := self.getCheckForCredentialRequestFunc() @@ -358,25 +359,26 @@ func (self *cmdObjRunner) processOutput( if ok { responseChan := promptUserForCredential(askFor) if responseChan == nil { - // Returning a nil channel means we should kill the process. - // Note that we don't break the loop after this, because we - // still need to drain the output, otherwise the Wait() call - // later might block. - if err := Kill(cmdObj.GetCmd()); err != nil { + // Returning a nil channel means we should terminate the process. + // We achieve this by closing the pty that it's running in. Note that this won't + // work for the case where we're not running in a pty (i.e. on Windows), but + // in that case we'll never be prompted for credentials, so it's not a concern. + if err := closeFunc(); err != nil { self.log.Error(err) } - } else { - if task != nil { - task.Pause() - } - toInput := <-responseChan - if task != nil { - task.Continue() - } - // If the return data is empty we don't write anything to stdin - if toInput != "" { - _, _ = writer.Write([]byte(toInput)) - } + break + } + + if task != nil { + task.Pause() + } + toInput := <-responseChan + if task != nil { + task.Continue() + } + // If the return data is empty we don't write anything to stdin + if toInput != "" { + _, _ = writer.Write([]byte(toInput)) } } } diff --git a/pkg/commands/oscommands/cmd_obj_runner_test.go b/pkg/commands/oscommands/cmd_obj_runner_test.go index fa88b6698..74e3a1985 100644 --- a/pkg/commands/oscommands/cmd_obj_runner_test.go +++ b/pkg/commands/oscommands/cmd_obj_runner_test.go @@ -127,7 +127,7 @@ func TestProcessOutput(t *testing.T) { writer := &strings.Builder{} cmdObj := &CmdObj{task: gocui.NewFakeTask()} - runner.processOutput(reader, writer, toChanFn(scenario.promptUserForCredential), cmdObj) + runner.processOutput(reader, writer, toChanFn(scenario.promptUserForCredential), func() error { return nil }, cmdObj) if writer.String() != scenario.expectedToWrite { t.Errorf("expected to write '%s' but got '%s'", scenario.expectedToWrite, writer.String())