From 6bdcae769d311b5deb82136d5e35d7ad986bca28 Mon Sep 17 00:00:00 2001
From: KernelDeimos <eric.alex.dube@gmail.com>
Date: Wed, 4 Sep 2024 23:50:42 -0400
Subject: [PATCH] fix: double-echo in phoenix

---
 src/phoenix/doc/devlog.md                     | 40 +++++++++++++++++++
 src/phoenix/src/pty/XDocumentPTT.js           | 27 +++++++++++++
 src/phoenix/src/puter-shell/main.js           |  1 +
 .../providers/PuterAppCommandProvider.js      | 10 +++++
 4 files changed, 78 insertions(+)

diff --git a/src/phoenix/doc/devlog.md b/src/phoenix/doc/devlog.md
index 59aad3f9..3d028208 100644
--- a/src/phoenix/doc/devlog.md
+++ b/src/phoenix/doc/devlog.md
@@ -1150,3 +1150,43 @@ This change can be made incrementally as follows:
   - update the command to use the new implementation
 - Once all commands are updated, the XDocumentPuterShell class will
   be dormant and can safely be removed.
+
+## 2024-09-04
+
+### Terminal I/O
+
+Prior to recent changes it was not possible to run phoenix shell inside
+another instance of phoenix shell. The following had to be resolved for
+this to work:
+- Phoenix was waiting for a configuration object from the parent app,
+  under the assumption that this parent app is a terminal. Phoenix now
+  does not require this configuration object.
+- Initial terminal size had to be requested for by Phoenix after some
+  initialization to avoid a race condition.
+- Apps called by an application under a terminal were not able to control
+  `echo` behavior.
+
+The new IO functionality follows the
+[Selective Layer Implementation Pattern](https://puter.com/@ed/documentation/glossary.md##Selective-Layer-Implementation-Pattern)
+with the following implemented:
+- IOCTL: `TIOCGWINSZ`, sent via postMessage
+- signal: `ioctl.set` event to simulate "SIGWINCH",
+  but this should be merged with existing code that
+  implements the concept of signals.
+- ptt.termios: a high-level mimic of termios
+  (currently only echo control is implemented)
+
+XDocumentPTT now implements `TIOCGWINSZ` (named after the POSIX equivalent)
+which requests the window size. This function calls `postMessage` on the
+AppConnection with the following
+[type-tagged object](../../../doc/api/type-tagged.md):
+`{ $: 'ioctl.request', requestCode: 104 }`.
+
+The window size information is still provided
+by the badly-named `ioctl.set` event, which should later be changed to
+a similar convention, like `{ $: 'signal', code: 28 }`.
+
+The termios implementation is a high-level mimic and does not actually
+use the underlying IOCTL implementation. Instead it sends a type-tagged
+object that looks like `{ $: 'chtermios', termios: { echo: false } }`,
+where `echo` can be `true` or `false`.
diff --git a/src/phoenix/src/pty/XDocumentPTT.js b/src/phoenix/src/pty/XDocumentPTT.js
index 3ea15e56..b1054d39 100644
--- a/src/phoenix/src/pty/XDocumentPTT.js
+++ b/src/phoenix/src/pty/XDocumentPTT.js
@@ -41,6 +41,18 @@ export class XDocumentPTT {
             };
         }
 
+        this.termios = new Proxy({}, {
+            set (target, k, v) {
+                terminalConnection.postMessage({
+                    $: 'chtermios',
+                    termios: {
+                        [k]: v,
+                    }
+                });
+                return Reflect.set(target, k, v);
+            }
+        });
+
         this.ioctl_listeners = {};
 
         this.readableStream = new ReadableStream({
@@ -91,4 +103,19 @@ export class XDocumentPTT {
             listener(evt);
         }
     }
+
+    once (name, listener) {
+        const wrapper = evt => {
+            listener(evt);
+            this.off(name, wrapper);
+        };
+        this.on(name, wrapper);
+    }
+
+    off (name, listener) {
+        if ( ! this.ioctl_listeners.hasOwnProperty(name) ) return;
+        this.ioctl_listeners[name] = this.ioctl_listeners[name].filter(
+            l => l !== listener
+        );
+    }
 }
diff --git a/src/phoenix/src/puter-shell/main.js b/src/phoenix/src/puter-shell/main.js
index 7ba7f882..0d4b94cd 100644
--- a/src/phoenix/src/puter-shell/main.js
+++ b/src/phoenix/src/puter-shell/main.js
@@ -138,6 +138,7 @@ export const launchPuterShell = async (ctx) => {
 
     // NEXT
     ptt.TIOCGWINSZ();
+    ptt.termios.echo = false;
 
     const fire = (text) => {
         // Define fire-like colors (ANSI 256-color codes)
diff --git a/src/phoenix/src/puter-shell/providers/PuterAppCommandProvider.js b/src/phoenix/src/puter-shell/providers/PuterAppCommandProvider.js
index c2589244..20149893 100644
--- a/src/phoenix/src/puter-shell/providers/PuterAppCommandProvider.js
+++ b/src/phoenix/src/puter-shell/providers/PuterAppCommandProvider.js
@@ -88,6 +88,15 @@ export class PuterAppCommandProvider {
                         if (message.$ === 'stdout') {
                             ctx.externs.out.write(decoder.decode(message.data));
                         }
+                        if (message.$ === 'chtermios') {
+                            if ( message.termios.echo !== undefined ) {
+                                if ( message.termios.echo ) {
+                                    ctx.externs.echo.on();
+                                } else {
+                                    ctx.externs.echo.off();
+                                }
+                            }
+                        }
                     });
 
                     // Repeatedly copy data from stdin to the child, while it's running.
@@ -108,6 +117,7 @@ export class PuterAppCommandProvider {
                     setTimeout(next_data, 0);
                 }
 
+                // TODO: propagate sigint to the app
                 return Promise.race([ app_close_promise, sigint_promise ]);
             }
         };