Drop Windows PowerShell's echo of the lines the client sends (v1.64.1)

CHANGELOG.md

@@ -1,3 +1,100 @@
+## 1.64.1
+
+### Fixed
+
+- **A PowerShell session on a Windows node no longer shows each command line
+  echoed above its output, nor a stray `PS>`.** For example, `ls` printed
+  `ls ; [Console]::Out.Write('__OMNYSHELL_CWD_…`.
+  - **Cause:** PowerShell, reading its commands from a pipe as these
+    sessions do, writes its prompt before reading each line, then echoes the
+    line it read, ending with `\n`. This is the same in Windows PowerShell
+    5.1 and PowerShell 7, as recorded on CI. The init line's `function
+    prompt { '' }` makes the prompt `PS>`, PowerShell's fallback for an
+    empty prompt; it does not remove it. The client assumed a piped
+    PowerShell neither prompts nor echoes. Its own prompt redraw usually
+    hid the `PS>`, leaving the rest of the line, the echo, visible. Earlier
+    versions showed fragments; the longer markers in 1.63.4/1.64.0 made whole
+    lines show.
+  - **Now:**
+    - **A sentinel prompt:** the session's init line sets the prompt to a
+      per-session sentinel (`CwdMarker.promptSentinel`). It is written in two
+      halves, so the init line's own echo never contains it.
+    - **What is dropped:** every line PowerShell reads comes back as one
+      output line starting with that exact string, and the client drops it
+      whole (a new `EchoFilter`, used for the PowerShell dialect). That holds
+      whoever sent the line: commands, markers, keystrokes typed ahead, or a
+      line sent before the first prompt.
+    - **The one exception:** the line read before the sentinel exists, the
+      init line itself after the default `PS C:\…> ` prompt, is dropped by
+      its known text.
+    - **What is left alone:** nothing else is touched, so a nested `pwsh`'s
+      prompt or a REPL's `>` shows as it is.
+    - **Holding back:** a sentinel is written before the next line is even
+      sent, so a line start that may still become one is held until its line
+      completes. Nothing else is held, and a chunk with no prompt in it
+      passes through uncopied.
+    - **Resumed sessions:** a session resumed from an earlier connection
+      already has the sentinel prompt.
+  - **Client-side only:** it works with nodes of any version, including the
+    1.63.3 node this was reported against.
+- **Windows PowerShell 5.1 error messages no longer show the client's marker
+  code.** An error quotes the failing line, about 80 characters of it:
+  ```
+  At line:1 char:1
+  + Get-Item … ; $__omnyLc=$LASTEXITCODE;if($n …
+  ```
+  It quoted the marker the client appends to every command, and for an AI
+  agent run also its exit-code prefix. The marker has to ride on the
+  command's line, since a program reading stdin would take a separate line
+  as input. So the client now leaves 256 blanks between the command and
+  what it adds around it, and the quoted window holds only the command.
+  PowerShell 7's shorter error format never quoted the line.
+
+### Tests
+
+- **`update_command_test.dart` no longer damages the real pub cache on
+  Windows.** It linked its throwaway pub cache's `hosted`/`git` folders to the
+  real cache, so `pub get` could resolve offline. On Windows those links are
+  junctions, and deleting the throwaway cache went through them and removed
+  packages from the real cache. On CI, the AI tests then failed to load
+  `package:http/testing.dart`. On Windows the throwaway cache now downloads
+  what it needs, with no links. Elsewhere the links are removed by what they
+  are, without following them, and the temporary directory is left in place
+  if any link survives.
+- New `test/integration/powershell_session_test.dart` (Windows). It runs real
+  `powershell.exe` and `pwsh.exe` sessions through the controller as the CLI
+  does. Cases:
+  - output without a final newline
+  - 3,000 lines over many chunks
+  - an error and the command after it
+  - non-ASCII and 2,000-character command lines
+  - `cd`
+  - AI agent runs: output, and exit codes 0, 1 and a program's own
+  - input typed into a command reading stdin
+  - a line typed ahead
+
+  Each checks that no prompt sentinel, echoed line or marker reaches the user
+  (it found the error-message leak above).
+- New `test/integration/powershell_pipe_test.dart` (Windows). It starts
+  `powershell.exe` and `pwsh.exe` the way a node does, sends the client's
+  protocol, and pins down what comes back: the default prompt and then the
+  sentinel before each line read, the echoes, `\n` endings. It also checks
+  that the filter leaves exactly the markers and the command's output. So a
+  change in PowerShell's behaviour fails here first.
+- Unit tests for the filter. They replay that transcript, whole and one byte
+  at a time, and cover:
+  - the prompt held across a command boundary
+  - typed-ahead lines, and a line sent before the first prompt
+  - a nested `pwsh`'s prompt and a `>` shown at once
+  - the sentinel mid-line, and near-misses
+  - the init echo dropped only once, and resumed sessions
+  - an uncopied pass-through
+- Controller tests replay the same format for commands, AI agent runs,
+  `Read-Host` input, stderr, resumed sessions and another code page.
+- `list_shortcut_test.dart` (Windows) runs real `powershell.exe` and
+  `pwsh.exe` sessions, locally and over the Hub, and checks that the output
+  is exactly the command's: no echo, no marker, no `PS>`.
+
 ## 1.64.0
 
 ### Added

lib/src/application/client/cwd_marker.dart

@@ -74,6 +74,12 @@ class CwdMarker {
   /// Creates a marker, using a random [nonce] by default.
   CwdMarker([String? nonce]) : token = '__OMNYSHELL_CWD_${nonce ?? newId()}__';
 
+  /// A per-session string distinct from [token] (which the marker scan looks
+  /// for), that the PowerShell dialect makes its prompt: PowerShell writes its
+  /// prompt before every line it reads, and this lets the client strip it — and
+  /// the echoed line after it — exactly (see `EchoFilter`).
+  String get promptSentinel => token.replaceFirst('_CWD_', '_PS_');
+
   /// The [token] split into two halves, for emitting it across two literal
   /// arguments so the full token never appears verbatim in the command text we
   /// send (which would false-match if an interactive program echoed it back).

lib/src/application/client/echo_filter.dart

@@ -0,0 +1,171 @@
+import 'dart:convert';
+import 'dart:typed_data';
+
+/// Removes PowerShell's prompts, and its echo of the lines it reads, from a
+/// session's output.
+///
+/// PowerShell (Windows PowerShell 5.1 and PowerShell 7 alike) reading its
+/// commands from a redirected stdin, as a pipe session does, writes its prompt
+/// before reading each line, then echoes the line it read, ending with `\n`
+/// (recorded by `test/integration/powershell_pipe_test`):
+///
+/// ```text
+/// PS C:\Users\u> function prompt { … }; …\n  ← the init line, before it ran
+/// <prompt>$__omnyLc=$LASTEXITCODE;…\n        ← the first marker, echoed
+/// <marker output>\n
+/// <prompt>Write-Output hi ; [Console]::Out…\n ← a command, echoed
+/// hi\r\n
+/// <marker output>\n
+/// <prompt>                                    ← waiting for the next line
+/// ```
+///
+/// The session's init line makes `<prompt>` a per-session sentinel
+/// (`CwdMarker.promptSentinel`, written in two halves so it never appears in
+/// the init line's own echo). Every line PowerShell reads, then, comes back as
+/// one output line starting with that exact string: the filter drops each such
+/// line whole — whoever sent what PowerShell read (the client's commands and
+/// markers, keystrokes typed ahead, continuation lines). Nothing else is
+/// touched: a program's own prompt (a nested `pwsh`'s `PS C:\…> `, a REPL's
+/// `>`) shows as it is.
+///
+/// The one line before the sentinel exists — the init line's echo, after the
+/// default `PS <location>> ` prompt — is dropped by its known text.
+///
+/// A prompt is written before the next line is even sent, so a line start
+/// that is, or may still become, the sentinel is held back until its line
+/// completes; nothing else is ever held.
+class EchoFilter {
+  /// Filters the output of a session whose prompt is [prompt] (the
+  /// sentinel), and whose first line read is [initLine] (the line that sets
+  /// that prompt; `null` when the session was set up earlier).
+  EchoFilter({required String prompt, String? initLine})
+    : _prompt = utf8.encode(prompt),
+      _initLine = initLine == null ? null : utf8.encode(initLine);
+
+  final Uint8List _prompt;
+
+  /// The init line's echo to drop, until it has been (or the init line has
+  /// otherwise taken effect).
+  Uint8List? _initLine;
+
+  Uint8List _held = Uint8List(0);
+  bool _lineStart = true;
+
+  static final _defaultPrompt = latin1.encode('PS ');
+
+  /// A command completed: the init line has run by now, so the default
+  /// prompt is no longer expected.
+  void commandCompleted() => _initLine = null;
+
+  /// [chunk] without PowerShell's prompts and the lines echoed after them.
+  /// Bytes that may still become a prompt are held for the next call.
+  Uint8List filter(Uint8List chunk) {
+    if (_held.isEmpty && _initLine == null && !_promptMayStartIn(chunk)) {
+      if (chunk.isNotEmpty) _lineStart = chunk.last == 0x0a;
+      return chunk;
+    }
+    final buf = _held.isEmpty
+        ? chunk
+        : (BytesBuilder(copy: false)
+                ..add(_held)
+                ..add(chunk))
+              .takeBytes();
+    _held = Uint8List(0);
+
+    final out = BytesBuilder(copy: false);
+    var start = 0;
+    while (start < buf.length) {
+      final newline = buf.indexOf(0x0a, start);
+      if (newline < 0) {
+        final rest = Uint8List.sublistView(buf, start);
+        if (_lineStart && _mayBecomeDropped(rest)) {
+          _held = Uint8List.fromList(rest);
+        } else {
+          out.add(rest);
+          _lineStart = false;
+        }
+        break;
+      }
+      if (!(_lineStart && _isDropped(buf, start, newline))) {
+        out.add(Uint8List.sublistView(buf, start, newline + 1));
+      }
+      _lineStart = true;
+      start = newline + 1;
+    }
+    return out.takeBytes();
+  }
+
+  /// Whether the line `buf[start..newline]` is a prompt and what PowerShell
+  /// read after it, or the init line's echo.
+  bool _isDropped(Uint8List buf, int start, int newline) {
+    if (_startsWith(buf, start, newline, _prompt)) {
+      _initLine = null; // the sentinel prompt: the init line has run
+      return true;
+    }
+    final init = _initLine;
+    if (init != null && _startsWith(buf, start, newline, _defaultPrompt)) {
+      final end = newline > start && buf[newline - 1] == 0x0d
+          ? newline - 1
+          : newline;
+      if (_endsWith(buf, start, end, init)) {
+        _initLine = null;
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /// Whether the partial line [rest] may still become a dropped line: the
+  /// sentinel so far, the sentinel and what follows, or — before the init
+  /// line has run — the default prompt so far.
+  bool _mayBecomeDropped(Uint8List rest) {
+    if (_startsWith(rest, 0, rest.length, _prompt) ||
+        _isPrefix(rest, _prompt)) {
+      return true;
+    }
+    return _initLine != null &&
+        (_startsWith(rest, 0, rest.length, _defaultPrompt) ||
+            _isPrefix(rest, _defaultPrompt)) &&
+        rest.length < 4096;
+  }
+
+  /// Whether a line of [chunk] (the first, if a line starts at the chunk's
+  /// start) begins with the sentinel's first byte: the cheap test that lets a
+  /// chunk with no prompt pass through without being copied.
+  bool _promptMayStartIn(Uint8List chunk) {
+    final first = _prompt[0];
+    if (_lineStart && chunk.isNotEmpty && chunk[0] == first) return true;
+    for (var i = 0; i < chunk.length - 1; i++) {
+      if (chunk[i] == 0x0a && chunk[i + 1] == first) return true;
+    }
+    return false;
+  }
+
+  /// Whether `buf[start..end)` starts with [prefix].
+  static bool _startsWith(Uint8List buf, int start, int end, List<int> prefix) {
+    if (end - start < prefix.length) return false;
+    for (var j = 0; j < prefix.length; j++) {
+      if (buf[start + j] != prefix[j]) return false;
+    }
+    return true;
+  }
+
+  /// Whether `buf[start..end)` ends with [suffix].
+  static bool _endsWith(Uint8List buf, int start, int end, List<int> suffix) {
+    if (end - start < suffix.length) return false;
+    final offset = end - suffix.length;
+    for (var j = 0; j < suffix.length; j++) {
+      if (buf[offset + j] != suffix[j]) return false;
+    }
+    return true;
+  }
+
+  /// Whether [part] (non-empty) is a proper prefix of [whole].
+  static bool _isPrefix(Uint8List part, List<int> whole) {
+    if (part.isEmpty || part.length >= whole.length) return false;
+    for (var j = 0; j < part.length; j++) {
+      if (part[j] != whole[j]) return false;
+    }
+    return true;
+  }
+}

lib/src/application/client/interactive_shell_controller.dart

@@ -5,6 +5,7 @@ import 'dart:typed_data';
 import '../../domain/backend/shell_family.dart';
 import 'command_classifier.dart';
 import 'cwd_marker.dart';
+import 'echo_filter.dart';
 import 'shell_dialect.dart';
 import 'shell_session_port.dart';
 
@@ -38,7 +39,9 @@ class ShellPromptState {
 /// Drives a [ShellSessionPort] as an interactive shell, terminal-agnostically.
 ///
 /// OmnyShell runs the remote shell as a **pipe (no PTY)**: it emits no prompt
-/// and no input echo, and there is no completion signal. This controller
+/// and no input echo (except Windows PowerShell, whose echo is filtered out:
+/// [ShellDialect.echoesInput]), and there is no completion signal. This
+/// controller
 /// supplies that missing protocol layer — the same one the CLI's `connect` loop
 /// and the web client both need:
 ///
@@ -114,6 +117,19 @@ class InteractiveShellController {
   Completer<ShellRunResult>? _agentRun;
   BytesBuilder? _agentCapture;
 
+  /// Drops the shell's prompts and its echo of the lines it reads, for a
+  /// dialect whose shell writes them ([ShellDialect.echoesInput]); `null`
+  /// otherwise. A resumed session's init line ran in an earlier connection,
+  /// so there is no init echo to expect.
+  late final EchoFilter? _echo = _dialect.echoesInput
+      ? EchoFilter(
+          prompt: _marker.promptSentinel,
+          initLine: _resumedInAltScreen
+              ? null
+              : _dialect.sessionInitLine(_marker),
+        )
+      : null;
+
   String? _cwd;
   String? _branch;
   String? _gitStatus;
@@ -166,7 +182,7 @@ class InteractiveShellController {
       onPassthrough(true);
       return;
     }
-    final init = _dialect.initLine;
+    final init = _dialect.sessionInitLine(_marker);
     if (init != null) _send('$init\n');
     _send('${_dialect.fullMarker(_marker)}\n');
   }
@@ -178,7 +194,7 @@ class InteractiveShellController {
     // Replenish the node's send window for the raw bytes consumed; without this
     // the channel's credit drains and output stalls (e.g. a TUI repainting).
     _session.grantWindow(bytes.length);
-    final scan = _marker.feed(bytes);
+    final scan = _marker.feed(_echo?.filter(bytes) ?? bytes);
     if (scan.output.isNotEmpty) {
       onOutput(scan.output);
       _agentCapture?.add(scan.output);
@@ -190,6 +206,7 @@ class InteractiveShellController {
       _privilege = scan.privilege;
     }
     if (scan.completed) {
+      _echo?.commandCompleted();
       _inFlight = false;
       onPassthrough(false);
       if (_pendingResumeCwd && _cwd == null) {