Connect: Close terminal tab if last input was Ctrl+D, even on non-zero exit code (#59836)

* Include the last input in the exit event

* Close terminal tab if last input was Ctrl+D

* Remove use of create functions

Author: ravicious

gen/proto/ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb.ts

@@ -93,6 +93,7 @@ message PtyEventOpen {}
 message PtyEventExit {
   uint32 exit_code = 1;
   optional uint32 signal = 2;
+  string last_input = 3;
 }
 
 // PtyEventStartError is sent by the PTY process when the shared process fails to start it.

proto/teleport/web/teleterm/ptyhost/v1/pty_host_service.proto

@@ -21,9 +21,6 @@ import { DuplexStreamingCall } from '@protobuf-ts/runtime-rpc';
 import {
   ManagePtyProcessRequest,
   ManagePtyProcessResponse,
-  PtyEventData,
-  PtyEventResize,
-  PtyEventStart,
 } from 'gen-proto-ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb';
 
 import {
@@ -56,7 +53,7 @@ export class PtyEventsStreamHandler {
     await this.send({
       event: {
         oneofKind: 'start',
-        start: PtyEventStart.create({ columns, rows }),
+        start: { columns, rows },
       },
     });
   }
@@ -65,7 +62,7 @@ export class PtyEventsStreamHandler {
     await this.send({
       event: {
         oneofKind: 'data',
-        data: PtyEventData.create({ message: data }),
+        data: { message: data },
       },
     });
   }
@@ -74,7 +71,7 @@ export class PtyEventsStreamHandler {
     await this.send({
       event: {
         oneofKind: 'resize',
-        resize: PtyEventResize.create({ columns, rows }),
+        resize: { columns, rows },
       },
     });
   }
@@ -110,7 +107,11 @@ export class PtyEventsStreamHandler {
   }
 
   onExit(
-    callback: (reason: { exitCode: number; signal?: number }) => void
+    callback: (reason: {
+      exitCode: number;
+      signal?: number;
+      lastInput: string;
+    }) => void
   ): RemoveListenerFunction {
     return this.addDataListenerAndReturnRemovalFunction(
       (event: ManagePtyProcessResponse) => {

web/packages/teleterm/src/services/pty/ptyHost/ptyEventsStreamHandler.ts

@@ -61,7 +61,13 @@ export function createPtyProcess(
       return stream.onOpen(callback);
     },
 
-    onExit(callback: (reason: { exitCode: number; signal?: number }) => void) {
+    onExit(
+      callback: (reason: {
+        exitCode: number;
+        signal?: number;
+        lastInput: string;
+      }) => void
+    ) {
       return stream.onExit(callback);
     },
 

web/packages/teleterm/src/services/pty/ptyHost/ptyProcess.ts

@@ -22,11 +22,8 @@ import {
   ManagePtyProcessRequest,
   ManagePtyProcessResponse,
   PtyEventData,
-  PtyEventExit,
-  PtyEventOpen,
   PtyEventResize,
   PtyEventStart,
-  PtyEventStartError,
 } from 'gen-proto-ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb';
 
 import {
@@ -75,44 +72,36 @@ export class PtyEventsStreamHandler {
 
   private handleStartEvent(event: PtyEventStart): void {
     this.ptyProcess.onData(data =>
-      this.stream.write(
-        ManagePtyProcessResponse.create({
-          event: {
-            oneofKind: 'data',
-            data: PtyEventData.create({ message: data }),
-          },
-        })
-      )
+      this.stream.write({
+        event: {
+          oneofKind: 'data',
+          data: { message: data },
+        },
+      })
     );
     this.ptyProcess.onOpen(() =>
-      this.stream.write(
-        ManagePtyProcessResponse.create({
-          event: {
-            oneofKind: 'open',
-            open: PtyEventOpen.create(),
-          },
-        })
-      )
+      this.stream.write({
+        event: {
+          oneofKind: 'open',
+          open: {},
+        },
+      })
     );
-    this.ptyProcess.onExit(({ exitCode, signal }) =>
-      this.stream.write(
-        ManagePtyProcessResponse.create({
-          event: {
-            oneofKind: 'exit',
-            exit: PtyEventExit.create({ exitCode, signal }),
-          },
-        })
-      )
+    this.ptyProcess.onExit(payload =>
+      this.stream.write({
+        event: {
+          oneofKind: 'exit',
+          exit: payload,
+        },
+      })
     );
     this.ptyProcess.onStartError(message => {
-      this.stream.write(
-        ManagePtyProcessResponse.create({
-          event: {
-            oneofKind: 'startError',
-            startError: PtyEventStartError.create({ message }),
-          },
-        })
-      );
+      this.stream.write({
+        event: {
+          oneofKind: 'startError',
+          startError: { message },
+        },
+      });
     });
     // PtyProcess.prototype.start always returns a fulfilled promise. If an error is caught during
     // start, it's reported through PtyProcess.prototype.onStartError. Similarly, the information

web/packages/teleterm/src/sharedProcess/ptyHost/ptyEventsStreamHandler.ts

@@ -17,10 +17,6 @@
  */
 
 import { Struct } from 'gen-proto-ts/google/protobuf/struct_pb';
-import {
-  CreatePtyProcessResponse,
-  GetCwdResponse,
-} from 'gen-proto-ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb';
 import { IPtyHostService } from 'gen-proto-ts/teleport/web/teleterm/ptyhost/v1/pty_host_service_pb.grpc-server';
 
 import Logger from 'teleterm/logger';
@@ -55,7 +51,7 @@ export function createPtyHostService(): IPtyHostService & {
         callback(error);
         return;
       }
-      callback(null, CreatePtyProcessResponse.create({ id: ptyId }));
+      callback(null, { id: ptyId });
       logger.info(`created PTY process for id ${ptyId}`);
     },
     getCwd: (call, callback) => {
@@ -69,8 +65,7 @@ export function createPtyHostService(): IPtyHostService & {
       ptyProcess
         .getCwd()
         .then(cwd => {
-          const response = GetCwdResponse.create({ cwd });
-          callback(null, response);
+          callback(null, { cwd });
         })
         .catch(error => {
           logger.error(`could not read CWD for id: ${id}`, error);

web/packages/teleterm/src/sharedProcess/ptyHost/ptyHostService.ts

@@ -52,4 +52,29 @@ describe('PtyProcess', () => {
       );
     });
   });
+
+  if (process.platform !== 'win32') {
+    test('including the last input in the exit event', async () => {
+      const pty = new PtyProcess({
+        path: 'sh',
+        env: {},
+        args: [],
+        useConpty: true,
+        ptyId: '1234',
+      });
+      await pty.start(80, 24);
+      const listener = jest.fn();
+      pty.onExit(listener);
+
+      pty.write('\x04');
+
+      await expect(() => listener.mock.calls.length > 0).toEventuallyBeTrue({
+        waitFor: 2000,
+        tick: 10,
+      });
+      expect(listener).toHaveBeenCalledWith(
+        expect.objectContaining({ lastInput: '\x04' })
+      );
+    });
+  }
 });

web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.test.ts

@@ -42,6 +42,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
   private _logger: Logger;
   private _status: Status = 'not_initialized';
   private _disposed = false;
+  private _lastInput = '';
 
   constructor(private options: PtyProcessOptions & { ptyId: string }) {
     super();
@@ -115,6 +116,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
       return;
     }
 
+    this._lastInput = data;
     this._process.write(data);
   }
 
@@ -184,7 +186,9 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
     return this.addListenerAndReturnRemovalFunction(TermEventEnum.Open, cb);
   }
 
-  onExit(cb: (ev: { exitCode: number; signal?: number }) => void) {
+  onExit(
+    cb: (ev: { exitCode: number; signal?: number; lastInput: string }) => void
+  ) {
     return this.addListenerAndReturnRemovalFunction(TermEventEnum.Exit, cb);
   }
 
@@ -229,7 +233,7 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
   }
 
   private _handleExit(e: { exitCode: number; signal?: number }) {
-    this.emit(TermEventEnum.Exit, e);
+    this.emit(TermEventEnum.Exit, { ...e, lastInput: this._lastInput });
     this._logger.info(`pty has been terminated with exit code: ${e.exitCode}`);
     this._setStatus('terminated');
   }

web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts

@@ -45,7 +45,7 @@ export type IPtyProcess = {
   onOpen(cb: () => void): RemoveListenerFunction;
   onStartError(cb: (message: string) => void): RemoveListenerFunction;
   onExit(
-    cb: (ev: { exitCode: number; signal?: number }) => void
+    cb: (ev: { exitCode: number; signal?: number; lastInput: string }) => void
   ): RemoveListenerFunction;
 };
 

web/packages/teleterm/src/sharedProcess/ptyHost/types.ts

@@ -209,15 +209,12 @@ async function setUpPtyProcess(
 
   ptyProcess.onExit(event => {
     // Not closing the tab on non-zero exit code lets us show the error to the user if, for example,
-    // tsh ssh cannot connect to the given node.
+    // tsh ssh couldn't connect to the given node.
     //
-    // The downside of this is that if you open a local shell, then execute a command that fails
-    // (for example, `cd` to a nonexistent directory), and then try to execute `exit` or press
-    // Ctrl + D, the tab won't automatically close, because the last exit code is not zero.
-    //
-    // We can look up how the terminal in vscode handles this problem, since in the scenario
-    // described above they do close the tab correctly.
-    if (event.exitCode === 0) {
+    // We also have to account for Ctrl+D, as executing it makes the shell exit with the last
+    // reported exit code. If we depended on the exit code alone, it'd mean that the terminal tab
+    // wouldn't close if Ctrl+D followed a command that failed, say cd to a nonexistent directory.
+    if (event.exitCode === 0 || event.lastInput === /* Ctrl+D */ '\x04') {
       documentsService.close(doc.uri);
     }
   });