From 4cb4bfa53d83fd18737f32db9e311b0df5dc4dec Mon Sep 17 00:00:00 2001
From: Greg Colombo <greg@oxidecomputer.com>
Date: Wed, 14 Feb 2024 23:17:37 +0000
Subject: [PATCH 1/8] PHD: add Windows Server 2016 adapter

---
 phd-tests/framework/src/guest_os/mod.rs       |  7 +++
 phd-tests/framework/src/guest_os/windows.rs   |  9 +++-
 .../src/guest_os/windows_server_2016.rs       | 45 +++++++++++++++++++
 3 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 phd-tests/framework/src/guest_os/windows_server_2016.rs

diff --git a/phd-tests/framework/src/guest_os/mod.rs b/phd-tests/framework/src/guest_os/mod.rs
index 15cd22639..b3f14dae5 100644
--- a/phd-tests/framework/src/guest_os/mod.rs
+++ b/phd-tests/framework/src/guest_os/mod.rs
@@ -13,6 +13,7 @@ mod alpine;
 mod debian11_nocloud;
 mod ubuntu22_04;
 mod windows;
+mod windows_server_2016;
 mod windows_server_2019;
 mod windows_server_2022;
 
@@ -64,6 +65,7 @@ pub enum GuestOsKind {
     Alpine,
     Debian11NoCloud,
     Ubuntu2204,
+    WindowsServer2016,
     WindowsServer2019,
     WindowsServer2022,
 }
@@ -76,6 +78,8 @@ impl FromStr for GuestOsKind {
             "alpine" => Ok(Self::Alpine),
             "debian11nocloud" => Ok(Self::Debian11NoCloud),
             "ubuntu2204" => Ok(Self::Ubuntu2204),
+            "windowsserver2016" => Ok(Self::WindowsServer2016),
+            "windowsserver2019" => Ok(Self::WindowsServer2019),
             "windowsserver2022" => Ok(Self::WindowsServer2022),
             _ => Err(std::io::Error::new(
                 std::io::ErrorKind::InvalidInput,
@@ -92,6 +96,9 @@ pub(super) fn get_guest_os_adapter(kind: GuestOsKind) -> Box<dyn GuestOs> {
             Box::new(debian11_nocloud::Debian11NoCloud)
         }
         GuestOsKind::Ubuntu2204 => Box::new(ubuntu22_04::Ubuntu2204),
+        GuestOsKind::WindowsServer2016 => {
+            Box::new(windows_server_2016::WindowsServer2016)
+        }
         GuestOsKind::WindowsServer2019 => {
             Box::new(windows_server_2019::WindowsServer2019)
         }
diff --git a/phd-tests/framework/src/guest_os/windows.rs b/phd-tests/framework/src/guest_os/windows.rs
index d7eefc18d..3486d5dd3 100644
--- a/phd-tests/framework/src/guest_os/windows.rs
+++ b/phd-tests/framework/src/guest_os/windows.rs
@@ -17,7 +17,9 @@ use super::{CommandSequence, CommandSequenceEntry, GuestOsKind};
 pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence {
     assert!(matches!(
         guest,
-        GuestOsKind::WindowsServer2019 | GuestOsKind::WindowsServer2022
+        GuestOsKind::WindowsServer2016
+            | GuestOsKind::WindowsServer2019
+            | GuestOsKind::WindowsServer2022
     ));
 
     let mut commands = vec![
@@ -49,7 +51,10 @@ pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence {
     // characters and letting the recipient display them in whatever style it
     // likes. This only happens once the command prompt has been activated, so
     // only switch buffering modes after entering credentials.
-    if let GuestOsKind::WindowsServer2019 = guest {
+    if matches!(
+        guest,
+        GuestOsKind::WindowsServer2016 | GuestOsKind::WindowsServer2019
+    ) {
         commands.extend([
             CommandSequenceEntry::ChangeSerialConsoleBuffer(
                 crate::serial::BufferKind::Vt80x24,
diff --git a/phd-tests/framework/src/guest_os/windows_server_2016.rs b/phd-tests/framework/src/guest_os/windows_server_2016.rs
new file mode 100644
index 000000000..59efd2422
--- /dev/null
+++ b/phd-tests/framework/src/guest_os/windows_server_2016.rs
@@ -0,0 +1,45 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Guest OS adaptations for Windows Server 2016 images. See [the general
+//! Windows module](mod@super::windows) documentation for more information.
+
+use std::borrow::Cow;
+
+use super::{CommandSequence, GuestOs, GuestOsKind};
+
+/// The guest adapter for Windows Server 2016 images. See [the general
+/// Windows module](mod@super::windows) documentation for more information about
+/// the configuration this adapter requires.
+pub(super) struct WindowsServer2016;
+
+impl GuestOs for WindowsServer2016 {
+    fn get_login_sequence(&self) -> CommandSequence {
+        super::windows::get_login_sequence_for(GuestOsKind::WindowsServer2016)
+    }
+
+    fn get_shell_prompt(&self) -> &'static str {
+        "Administrator@PHD-WINDOWS:$ "
+    }
+
+    fn read_only_fs(&self) -> bool {
+        false
+    }
+
+    fn amend_shell_command<'a>(&self, cmd: &'a str) -> Cow<'a, str> {
+        // Ensure that after executing a shell command, the 80x24 serial console
+        // buffer contains only the output of the command.
+        //
+        // `reset` is used to try to force the guest to clear and redraw the
+        // entire terminal so that there is no "left over" command prompt in the
+        // guest's terminal buffer. This ensures that the guest will print the
+        // new prompt instead of eliding it because it believes it's already on
+        // the screen.
+        Cow::from(format!("reset; {}", cmd))
+    }
+}

From 91478154c28b0b6145214b7f0eba02f4c4ebbdd7 Mon Sep 17 00:00:00 2001
From: Greg Colombo <greg@oxidecomputer.com>
Date: Thu, 15 Feb 2024 18:00:20 +0000
Subject: [PATCH 2/8] PHD: make GuestOs generate shell command sequences

---
 phd-tests/framework/src/guest_os/alpine.rs    | 13 +++--
 .../src/guest_os/debian11_nocloud.rs          |  6 +--
 phd-tests/framework/src/guest_os/mod.rs       | 23 ++++-----
 .../framework/src/guest_os/shell_commands.rs  | 49 +++++++++++++++++++
 .../framework/src/guest_os/ubuntu22_04.rs     | 10 ++--
 phd-tests/framework/src/guest_os/windows.rs   | 46 ++++++++---------
 .../src/guest_os/windows_server_2016.rs       | 16 +++---
 .../src/guest_os/windows_server_2019.rs       | 20 +++-----
 phd-tests/framework/src/test_vm/mod.rs        | 42 +++++++++-------
 9 files changed, 139 insertions(+), 86 deletions(-)
 create mode 100644 phd-tests/framework/src/guest_os/shell_commands.rs

diff --git a/phd-tests/framework/src/guest_os/alpine.rs b/phd-tests/framework/src/guest_os/alpine.rs
index 00b226288..df5072149 100644
--- a/phd-tests/framework/src/guest_os/alpine.rs
+++ b/phd-tests/framework/src/guest_os/alpine.rs
@@ -11,9 +11,9 @@ pub(super) struct Alpine;
 impl GuestOs for Alpine {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("localhost login: "),
-            CommandSequenceEntry::WriteStr("root"),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt()),
+            CommandSequenceEntry::WaitFor("localhost login: ".into()),
+            CommandSequenceEntry::WriteStr("root".into()),
+            CommandSequenceEntry::WaitFor(self.get_shell_prompt().into()),
         ])
     }
 
@@ -24,4 +24,11 @@ impl GuestOs for Alpine {
     fn read_only_fs(&self) -> bool {
         true
     }
+
+    fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        super::shell_commands::shell_command_sequence(
+            std::borrow::Cow::Borrowed(cmd),
+            crate::serial::BufferKind::Raw,
+        )
+    }
 }
diff --git a/phd-tests/framework/src/guest_os/debian11_nocloud.rs b/phd-tests/framework/src/guest_os/debian11_nocloud.rs
index dceca4d42..48c27f29e 100644
--- a/phd-tests/framework/src/guest_os/debian11_nocloud.rs
+++ b/phd-tests/framework/src/guest_os/debian11_nocloud.rs
@@ -11,9 +11,9 @@ pub(super) struct Debian11NoCloud;
 impl GuestOs for Debian11NoCloud {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("debian login: "),
-            CommandSequenceEntry::WriteStr("root"),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt()),
+            CommandSequenceEntry::WaitFor("debian login: ".into()),
+            CommandSequenceEntry::WriteStr("root".into()),
+            CommandSequenceEntry::WaitFor(self.get_shell_prompt().into()),
         ])
     }
 
diff --git a/phd-tests/framework/src/guest_os/mod.rs b/phd-tests/framework/src/guest_os/mod.rs
index b3f14dae5..3020e4e12 100644
--- a/phd-tests/framework/src/guest_os/mod.rs
+++ b/phd-tests/framework/src/guest_os/mod.rs
@@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize};
 
 mod alpine;
 mod debian11_nocloud;
+mod shell_commands;
 mod ubuntu22_04;
 mod windows;
 mod windows_server_2016;
@@ -19,12 +20,12 @@ mod windows_server_2022;
 
 /// An entry in a sequence of interactions with the guest's command prompt.
 #[derive(Debug)]
-pub(super) enum CommandSequenceEntry {
+pub(super) enum CommandSequenceEntry<'a> {
     /// Wait for the supplied string to appear on the guest serial console.
-    WaitFor(&'static str),
+    WaitFor(Cow<'a, str>),
 
     /// Write the specified string as a command to the guest serial console.
-    WriteStr(&'static str),
+    WriteStr(Cow<'a, str>),
 
     /// Change the serial console buffering discipline to the supplied
     /// discipline.
@@ -35,7 +36,7 @@ pub(super) enum CommandSequenceEntry {
     SetSerialByteWriteDelay(std::time::Duration),
 }
 
-pub(super) struct CommandSequence(pub Vec<CommandSequenceEntry>);
+pub(super) struct CommandSequence<'a>(pub Vec<CommandSequenceEntry<'a>>);
 
 pub(super) trait GuestOs: Send + Sync {
     /// Retrieves the command sequence used to wait for the OS to boot and log
@@ -48,13 +49,13 @@ pub(super) trait GuestOs: Send + Sync {
     /// Indicates whether the guest has a read-only filesystem.
     fn read_only_fs(&self) -> bool;
 
-    /// Some guests need to amend incoming shell commands from tests in order to
-    /// get output to display on the serial console in a way those guests can
-    /// accept (e.g. by clearing the screen immediately before running each
-    /// command). This function amends an incoming command according to the
-    /// guest adapter's instructions.
-    fn amend_shell_command<'a>(&self, cmd: &'a str) -> Cow<'a, str> {
-        Cow::Borrowed(cmd)
+    /// Returns the sequence of serial console operations a test VM should issue
+    /// in order to execute `cmd` in the guest's shell.
+    fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        shell_commands::shell_command_sequence(
+            Cow::Borrowed(cmd),
+            crate::serial::BufferKind::Raw,
+        )
     }
 }
 
diff --git a/phd-tests/framework/src/guest_os/shell_commands.rs b/phd-tests/framework/src/guest_os/shell_commands.rs
new file mode 100644
index 000000000..b9781e25b
--- /dev/null
+++ b/phd-tests/framework/src/guest_os/shell_commands.rs
@@ -0,0 +1,49 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Common helper functions for issuing shell commands to guests and handling
+//! their outputs.
+
+use std::borrow::Cow;
+
+use super::{CommandSequence, CommandSequenceEntry};
+
+/// Produces the shell command sequence necessary to execute `cmd` in a guest's
+/// shell, given that the guest is using the supplied serial console buffering
+/// discipline.
+///
+/// This routine assumes that multi-line commands will be echoed with `> ` at
+/// the start of each line in the command. This is technically shell-dependent
+/// but is true for all the shell types in PHD's currently-supported guests.
+pub(super) fn shell_command_sequence<'a>(
+    cmd: Cow<'a, str>,
+    buffer_kind: crate::serial::BufferKind,
+) -> CommandSequence {
+    let echo = cmd.trim_end().replace('\n', "\n> ");
+    match buffer_kind {
+        crate::serial::BufferKind::Raw => CommandSequence(vec![
+            CommandSequenceEntry::WriteStr(cmd.into()),
+            CommandSequenceEntry::WaitFor(echo.into()),
+            CommandSequenceEntry::WriteStr("\n".into()),
+        ]),
+
+        crate::serial::BufferKind::Vt80x24 => {
+            // In 80x24 mode, it's simplest to issue multi-line operations one
+            // line at a time and wait for each line to be echoed before
+            // starting the next. For very long commands (more than 24 lines),
+            // this avoids having to deal with lines scrolling off the buffer
+            // before they can be waited for.
+            let cmd_lines = cmd.trim_end().lines();
+            let echo_lines = echo.lines();
+            let mut seq = vec![];
+            for (cmd, echo) in cmd_lines.zip(echo_lines) {
+                seq.push(CommandSequenceEntry::WriteStr(cmd.to_owned().into()));
+                seq.push(CommandSequenceEntry::WaitFor(echo.to_owned().into()));
+                seq.push(CommandSequenceEntry::WriteStr("\n".into()));
+            }
+
+            CommandSequence(seq)
+        }
+    }
+}
diff --git a/phd-tests/framework/src/guest_os/ubuntu22_04.rs b/phd-tests/framework/src/guest_os/ubuntu22_04.rs
index 1d3cc7143..17587db25 100644
--- a/phd-tests/framework/src/guest_os/ubuntu22_04.rs
+++ b/phd-tests/framework/src/guest_os/ubuntu22_04.rs
@@ -12,11 +12,11 @@ pub(super) struct Ubuntu2204;
 impl GuestOs for Ubuntu2204 {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("ubuntu login: "),
-            CommandSequenceEntry::WriteStr("ubuntu"),
-            CommandSequenceEntry::WaitFor("Password: "),
-            CommandSequenceEntry::WriteStr("1!Passw0rd"),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt()),
+            CommandSequenceEntry::WaitFor("ubuntu login: ".into()),
+            CommandSequenceEntry::WriteStr("ubuntu".into()),
+            CommandSequenceEntry::WaitFor("Password: ".into()),
+            CommandSequenceEntry::WriteStr("1!Passw0rd".into()),
+            CommandSequenceEntry::WaitFor(self.get_shell_prompt().into()),
         ])
     }
 
diff --git a/phd-tests/framework/src/guest_os/windows.rs b/phd-tests/framework/src/guest_os/windows.rs
index 3486d5dd3..29b6c5441 100644
--- a/phd-tests/framework/src/guest_os/windows.rs
+++ b/phd-tests/framework/src/guest_os/windows.rs
@@ -14,7 +14,9 @@ use super::{CommandSequence, CommandSequenceEntry, GuestOsKind};
 /// - Cygwin is installed to C:\cygwin and can be launched by invoking
 ///   C:\cygwin\cygwin.bat.
 /// - The local administrator account is enabled with password `0xide#1Fan`.
-pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence {
+pub(super) fn get_login_sequence_for<'a>(
+    guest: GuestOsKind,
+) -> CommandSequence<'a> {
     assert!(matches!(
         guest,
         GuestOsKind::WindowsServer2016
@@ -24,26 +26,26 @@ pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence {
 
     let mut commands = vec![
         CommandSequenceEntry::WaitFor(
-            "Computer is booting, SAC started and initialized.",
+            "Computer is booting, SAC started and initialized.".into(),
         ),
         CommandSequenceEntry::WaitFor(
-            "EVENT: The CMD command is now available.",
+            "EVENT: The CMD command is now available.".into(),
         ),
-        CommandSequenceEntry::WaitFor("SAC>"),
-        CommandSequenceEntry::WriteStr("cmd"),
-        CommandSequenceEntry::WaitFor("Channel: Cmd0001"),
-        CommandSequenceEntry::WaitFor("SAC>"),
-        CommandSequenceEntry::WriteStr("ch -sn Cmd0001"),
+        CommandSequenceEntry::WaitFor("SAC>".into()),
+        CommandSequenceEntry::WriteStr("cmd".into()),
+        CommandSequenceEntry::WaitFor("Channel: Cmd0001".into()),
+        CommandSequenceEntry::WaitFor("SAC>".into()),
+        CommandSequenceEntry::WriteStr("ch -sn Cmd0001".into()),
         CommandSequenceEntry::WaitFor(
-            "Use any other key to view this channel.",
+            "Use any other key to view this channel.".into(),
         ),
-        CommandSequenceEntry::WriteStr(""),
-        CommandSequenceEntry::WaitFor("Username:"),
-        CommandSequenceEntry::WriteStr("Administrator"),
-        CommandSequenceEntry::WaitFor("Domain  :"),
-        CommandSequenceEntry::WriteStr(""),
-        CommandSequenceEntry::WaitFor("Password:"),
-        CommandSequenceEntry::WriteStr("0xide#1Fan"),
+        CommandSequenceEntry::WriteStr("".into()),
+        CommandSequenceEntry::WaitFor("Username:".into()),
+        CommandSequenceEntry::WriteStr("Administrator".into()),
+        CommandSequenceEntry::WaitFor("Domain  :".into()),
+        CommandSequenceEntry::WriteStr("".into()),
+        CommandSequenceEntry::WaitFor("Password:".into()),
+        CommandSequenceEntry::WriteStr("0xide#1Fan".into()),
     ];
 
     // Windows Server 2019's serial console-based command prompts default to
@@ -75,14 +77,14 @@ pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence {
         // eat the command and just process the newline). It also appears to
         // prefer carriage returns to linefeeds. Accommodate this behavior
         // until Cygwin is launched.
-        CommandSequenceEntry::WaitFor("C:\\Windows\\system32>"),
-        CommandSequenceEntry::WriteStr("cls\r"),
-        CommandSequenceEntry::WaitFor("C:\\Windows\\system32>"),
-        CommandSequenceEntry::WriteStr("C:\\cygwin\\cygwin.bat\r"),
-        CommandSequenceEntry::WaitFor("$ "),
+        CommandSequenceEntry::WaitFor("C:\\Windows\\system32>".into()),
+        CommandSequenceEntry::WriteStr("cls\r".into()),
+        CommandSequenceEntry::WaitFor("C:\\Windows\\system32>".into()),
+        CommandSequenceEntry::WriteStr("C:\\cygwin\\cygwin.bat\r".into()),
+        CommandSequenceEntry::WaitFor("$ ".into()),
         // Tweak the command prompt so that it appears on a single line with
         // no leading newlines.
-        CommandSequenceEntry::WriteStr("PS1='\\u@\\h:$ '"),
+        CommandSequenceEntry::WriteStr("PS1='\\u@\\h:$ '".into()),
     ]);
 
     CommandSequence(commands)
diff --git a/phd-tests/framework/src/guest_os/windows_server_2016.rs b/phd-tests/framework/src/guest_os/windows_server_2016.rs
index 59efd2422..c49715538 100644
--- a/phd-tests/framework/src/guest_os/windows_server_2016.rs
+++ b/phd-tests/framework/src/guest_os/windows_server_2016.rs
@@ -31,15 +31,11 @@ impl GuestOs for WindowsServer2016 {
         false
     }
 
-    fn amend_shell_command<'a>(&self, cmd: &'a str) -> Cow<'a, str> {
-        // Ensure that after executing a shell command, the 80x24 serial console
-        // buffer contains only the output of the command.
-        //
-        // `reset` is used to try to force the guest to clear and redraw the
-        // entire terminal so that there is no "left over" command prompt in the
-        // guest's terminal buffer. This ensures that the guest will print the
-        // new prompt instead of eliding it because it believes it's already on
-        // the screen.
-        Cow::from(format!("reset; {}", cmd))
+    fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        let cmd = format!("reset && {cmd}");
+        super::shell_commands::shell_command_sequence(
+            Cow::Owned(cmd),
+            crate::serial::BufferKind::Vt80x24,
+        )
     }
 }
diff --git a/phd-tests/framework/src/guest_os/windows_server_2019.rs b/phd-tests/framework/src/guest_os/windows_server_2019.rs
index 6a8e4dcd7..ce71cc173 100644
--- a/phd-tests/framework/src/guest_os/windows_server_2019.rs
+++ b/phd-tests/framework/src/guest_os/windows_server_2019.rs
@@ -27,19 +27,11 @@ impl GuestOs for WindowsServer2019 {
         false
     }
 
-    fn amend_shell_command<'a>(&self, cmd: &'a str) -> Cow<'a, str> {
-        // The simplest way to ensure that the 80x24 terminal buffer contains
-        // just the output of the most recent command and the subsequent prompt
-        // is to ask Windows to clear the screen and run the command in a single
-        // statement.
-        //
-        // Use Cygwin bash's `reset` instead of `clear` or `cls` to try to force
-        // Windows to clear and redraw the entire terminal before displaying any
-        // command output. Without this, Windows sometimes reprints a new
-        // command prompt to its internal screen buffer before re-rendering
-        // anything to the terminal; when this happens, it doesn't re-send the
-        // new command prompt, since it's "already there" on the output terminal
-        // (even though it may have been cleared from the match buffer).
-        Cow::from(format!("reset; {}", cmd))
+    fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        let cmd = format!("reset && {cmd}");
+        super::shell_commands::shell_command_sequence(
+            Cow::Owned(cmd),
+            crate::serial::BufferKind::Vt80x24,
+        )
     }
 }
diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs
index f0db51151..47097b5f2 100644
--- a/phd-tests/framework/src/test_vm/mod.rs
+++ b/phd-tests/framework/src/test_vm/mod.rs
@@ -650,13 +650,13 @@ impl TestVm {
                 match step {
                     CommandSequenceEntry::WaitFor(s) => {
                         self.wait_for_serial_output(
-                            s,
+                            s.as_ref(),
                             SerialOutputTimeout::CallerTimeout,
                         )
                         .await?;
                     }
                     CommandSequenceEntry::WriteStr(s) => {
-                        self.send_serial_str(s).await?;
+                        self.send_serial_str(s.as_ref()).await?;
                         self.send_serial_str("\n").await?;
                     }
                     CommandSequenceEntry::ChangeSerialConsoleBuffer(kind) => {
@@ -731,21 +731,27 @@ impl TestVm {
     /// [`Self::wait_for_serial_output`] and returns any text that was buffered
     /// to the serial console after the command was sent.
     pub async fn run_shell_command(&self, cmd: &str) -> Result<String> {
-        // Send the command out the serial port, including any amendments
-        // required by the guest. Do not send the final '\n' keystroke that
-        // actually issues the command.
-        let to_send = self.guest_os.amend_shell_command(cmd);
-        self.send_serial_str(&to_send).await?;
-
-        // Wait for the command to be echoed back. This ensures that the echoed
-        // command is consumed from the buffer such that it won't be returned
-        // as output when waiting for the post-command shell prompt to appear.
-        //
-        // Tests may send multi-line commands. Assume these won't be echoed
-        // literally and that each line will instead be preceded by `> `.
-        let echo = to_send.trim_end().replace('\n', "\n> ");
-        self.wait_for_serial_output(&echo, Duration::from_secs(15)).await?;
-        self.send_serial_str("\n").await?;
+        let command_sequence = self.guest_os.shell_command_sequence(cmd);
+        for step in command_sequence.0 {
+            match step {
+                CommandSequenceEntry::WaitFor(s) => {
+                    self.wait_for_serial_output(
+                        s.as_ref(),
+                        std::time::Duration::from_secs(15),
+                    )
+                    .await?;
+                }
+                CommandSequenceEntry::WriteStr(s) => {
+                    self.send_serial_str(s.as_ref()).await?;
+                }
+                _ => {
+                    anyhow::bail!(
+                        "Unexpected command sequence entry {step:?} while \
+                        running shell command"
+                    );
+                }
+            }
+        }
 
         // Once the command has run, the guest should display another prompt.
         // Treat the unconsumed buffered text before this point as the command
@@ -764,7 +770,7 @@ impl TestVm {
         Ok(out.trim().to_string())
     }
 
-    async fn send_serial_str(&self, string: &str) -> Result<()> {
+    pub(crate) async fn send_serial_str(&self, string: &str) -> Result<()> {
         if !string.is_empty() {
             self.send_serial_bytes_async(Vec::from(string.as_bytes())).await
         } else {

From 9ade09d3b67594d96d08861f53a517de17425eba Mon Sep 17 00:00:00 2001
From: Greg Colombo <greg@oxidecomputer.com>
Date: Thu, 15 Feb 2024 18:57:28 +0000
Subject: [PATCH 3/8] PHD: only apply debounce on repeated characters

---
 phd-tests/framework/src/guest_os/mod.rs     |  2 +-
 phd-tests/framework/src/guest_os/windows.rs | 23 +++++++++++----------
 phd-tests/framework/src/serial/mod.rs       | 11 +++++++---
 phd-tests/framework/src/test_vm/mod.rs      |  4 +++-
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/phd-tests/framework/src/guest_os/mod.rs b/phd-tests/framework/src/guest_os/mod.rs
index 3020e4e12..c8db81a3c 100644
--- a/phd-tests/framework/src/guest_os/mod.rs
+++ b/phd-tests/framework/src/guest_os/mod.rs
@@ -33,7 +33,7 @@ pub(super) enum CommandSequenceEntry<'a> {
 
     /// Set a delay between writing individual bytes to the guest serial console
     /// to avoid keyboard debouncing logic in guests.
-    SetSerialByteWriteDelay(std::time::Duration),
+    SetRepeatedCharacterDebounce(std::time::Duration),
 }
 
 pub(super) struct CommandSequence<'a>(pub Vec<CommandSequenceEntry<'a>>);
diff --git a/phd-tests/framework/src/guest_os/windows.rs b/phd-tests/framework/src/guest_os/windows.rs
index 29b6c5441..61ed3c2c9 100644
--- a/phd-tests/framework/src/guest_os/windows.rs
+++ b/phd-tests/framework/src/guest_os/windows.rs
@@ -48,11 +48,11 @@ pub(super) fn get_login_sequence_for<'a>(
         CommandSequenceEntry::WriteStr("0xide#1Fan".into()),
     ];
 
-    // Windows Server 2019's serial console-based command prompts default to
-    // trying to drive a VT100 terminal themselves instead of emitting
-    // characters and letting the recipient display them in whatever style it
-    // likes. This only happens once the command prompt has been activated, so
-    // only switch buffering modes after entering credentials.
+    // Earlier Windows Server versions' serial console-based command prompts
+    // default to trying to drive a VT100 terminal themselves instead of
+    // emitting characters and letting the recipient display them in whatever
+    // style it likes. This only happens once the command prompt has been
+    // activated, so only switch buffering modes after entering credentials.
     if matches!(
         guest,
         GuestOsKind::WindowsServer2016 | GuestOsKind::WindowsServer2019
@@ -61,12 +61,13 @@ pub(super) fn get_login_sequence_for<'a>(
             CommandSequenceEntry::ChangeSerialConsoleBuffer(
                 crate::serial::BufferKind::Vt80x24,
             ),
-            // Server 2019 also likes to debounce keystrokes, so set a small
-            // delay between characters to try to avoid this. (This value was
-            // chosen by experimentation; there doesn't seem to be a guest
-            // setting that controls this interval.)
-            CommandSequenceEntry::SetSerialByteWriteDelay(
-                std::time::Duration::from_millis(125),
+            // These versions also like to debounce keystrokes, so set a delay
+            // between repeated characters to try to avoid this. This is a very
+            // conservative delay to try to avoid test flakiness; fortunately,
+            // it only applies when typing the same character multiple times in
+            // a row.
+            CommandSequenceEntry::SetRepeatedCharacterDebounce(
+                std::time::Duration::from_secs(1),
             ),
         ]);
     }
diff --git a/phd-tests/framework/src/serial/mod.rs b/phd-tests/framework/src/serial/mod.rs
index bdad7e2e1..27feffc0e 100644
--- a/phd-tests/framework/src/serial/mod.rs
+++ b/phd-tests/framework/src/serial/mod.rs
@@ -202,15 +202,20 @@ async fn serial_task(
                                 );
                             }
                         } else {
-                            for b in bytes {
-                                if let Err(e) = stream.send(Message::Binary(vec![b])).await {
+                            let mut bytes = bytes.iter().peekable();
+                            while let Some(b) = bytes.next() {
+                                if let Err(e) = stream.send(Message::Binary(vec![*b])).await {
                                     error!(
                                         ?e,
                                         "failed to send input to serial console websocket"
                                     );
                                 }
 
-                                tokio::time::sleep(debounce).await;
+                                if let Some(next) = bytes.peek() {
+                                    if *next == b {
+                                        tokio::time::sleep(debounce).await;
+                                    }
+                                }
                             }
                         }
                     }
diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs
index 47097b5f2..26c2283c0 100644
--- a/phd-tests/framework/src/test_vm/mod.rs
+++ b/phd-tests/framework/src/test_vm/mod.rs
@@ -662,7 +662,9 @@ impl TestVm {
                     CommandSequenceEntry::ChangeSerialConsoleBuffer(kind) => {
                         self.change_serial_buffer_kind(kind)?;
                     }
-                    CommandSequenceEntry::SetSerialByteWriteDelay(duration) => {
+                    CommandSequenceEntry::SetRepeatedCharacterDebounce(
+                        duration,
+                    ) => {
                         self.set_serial_byte_write_delay(duration)?;
                     }
                 }

From 1c73c8c1349e1c35192444e533af4e73039481be Mon Sep 17 00:00:00 2001
From: Greg Colombo <greg@oxidecomputer.com>
Date: Thu, 15 Feb 2024 16:50:22 +0000
Subject: [PATCH 4/8] PHD: wait for entire command to be typed before starting
 serial timeout

---
 phd-tests/framework/src/serial/mod.rs  | 16 +++++++++++-----
 phd-tests/framework/src/test_vm/mod.rs |  7 ++++++-
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/phd-tests/framework/src/serial/mod.rs b/phd-tests/framework/src/serial/mod.rs
index 27feffc0e..0fe5b1b20 100644
--- a/phd-tests/framework/src/serial/mod.rs
+++ b/phd-tests/framework/src/serial/mod.rs
@@ -59,7 +59,7 @@ pub enum BufferKind {
 /// The set of commands that the serial console can send to its processing task.
 enum TaskCommand {
     /// Send the supplied bytes to the VM.
-    SendBytes(Vec<u8>),
+    SendBytes { bytes: Vec<u8>, done: oneshot::Sender<()> },
 
     /// Register to be notified if and when a supplied string appears in the
     /// serial console's buffer.
@@ -107,9 +107,13 @@ impl SerialConsole {
     }
 
     /// Directs the console worker thread to send the supplied `bytes` to the
-    /// guest.
-    pub fn send_bytes(&self, bytes: Vec<u8>) -> anyhow::Result<()> {
-        self.cmd_tx.send(TaskCommand::SendBytes(bytes))?;
+    /// guest, after which it should send to `done`.
+    pub fn send_bytes(
+        &self,
+        bytes: Vec<u8>,
+        done: oneshot::Sender<()>,
+    ) -> anyhow::Result<()> {
+        self.cmd_tx.send(TaskCommand::SendBytes { bytes, done })?;
         Ok(())
     }
 
@@ -193,7 +197,7 @@ async fn serial_task(
                     break;
                 };
                 match cmd {
-                    TaskCommand::SendBytes(bytes) => {
+                    TaskCommand::SendBytes { bytes, done } => {
                         if debounce.is_zero() {
                             if let Err(e) = stream.send(Message::Binary(bytes)).await {
                                 error!(
@@ -218,6 +222,8 @@ async fn serial_task(
                                 }
                             }
                         }
+
+                        let _ = done.send(());
                     }
                     TaskCommand::RegisterWait(waiter) => {
                         buffer.register_wait_for_output(waiter);
diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs
index 26c2283c0..143e3538f 100644
--- a/phd-tests/framework/src/test_vm/mod.rs
+++ b/phd-tests/framework/src/test_vm/mod.rs
@@ -782,7 +782,12 @@ impl TestVm {
 
     async fn send_serial_bytes_async(&self, bytes: Vec<u8>) -> Result<()> {
         match &self.state {
-            VmState::Ensured { serial } => serial.send_bytes(bytes),
+            VmState::Ensured { serial } => {
+                let (done_tx, done_rx) = oneshot::channel();
+                serial.send_bytes(bytes, done_tx)?;
+                done_rx.await?;
+                Ok(())
+            }
             VmState::New => Err(VmStateError::InstanceNotEnsured.into()),
         }
     }

From 73f52ee138a62fe3764e35394061a03c0599f9c5 Mon Sep 17 00:00:00 2001
From: Greg Colombo <greg@oxidecomputer.com>
Date: Thu, 15 Feb 2024 20:09:06 +0000
Subject: [PATCH 5/8] whoa we're half way there / whoa clippy on a prayer

---
 phd-tests/framework/src/guest_os/shell_commands.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/phd-tests/framework/src/guest_os/shell_commands.rs b/phd-tests/framework/src/guest_os/shell_commands.rs
index b9781e25b..895195500 100644
--- a/phd-tests/framework/src/guest_os/shell_commands.rs
+++ b/phd-tests/framework/src/guest_os/shell_commands.rs
@@ -16,14 +16,14 @@ use super::{CommandSequence, CommandSequenceEntry};
 /// This routine assumes that multi-line commands will be echoed with `> ` at
 /// the start of each line in the command. This is technically shell-dependent
 /// but is true for all the shell types in PHD's currently-supported guests.
-pub(super) fn shell_command_sequence<'a>(
-    cmd: Cow<'a, str>,
+pub(super) fn shell_command_sequence(
+    cmd: Cow<'_, str>,
     buffer_kind: crate::serial::BufferKind,
 ) -> CommandSequence {
     let echo = cmd.trim_end().replace('\n', "\n> ");
     match buffer_kind {
         crate::serial::BufferKind::Raw => CommandSequence(vec![
-            CommandSequenceEntry::WriteStr(cmd.into()),
+            CommandSequenceEntry::WriteStr(cmd),
             CommandSequenceEntry::WaitFor(echo.into()),
             CommandSequenceEntry::WriteStr("\n".into()),
         ]),

From bb39823bcc66f59b02d9ce9ef8028218001916c8 Mon Sep 17 00:00:00 2001
From: Greg Colombo <greg@oxidecomputer.com>
Date: Thu, 15 Feb 2024 20:58:20 +0000
Subject: [PATCH 6/8] PHD: make callers consume serial buffer by hand

Change the semantics of waiting for characters in the serial buffer so that
waits return the preceding buffer contents but don't actually erase anything
from the buffer. Instead, make callers manually issue a "clear" command to
discard from the buffer anything they know they're not going to want, and update
the guest shell command sequences to clear the buffer between echoing the entire
command and pressing Enter to execute it.

This fixes issues with command echoing for Windows guests that don't redraw
characters that the guest believes are already on the screen--now those
characters don't get consumed from the buffer, so it's OK that the guest doesn't
explicitly reprint them.
---
 phd-tests/framework/src/guest_os/mod.rs       |  3 ++
 .../framework/src/guest_os/shell_commands.rs  | 15 +++++-
 .../src/guest_os/windows_server_2016.rs       |  4 ++
 .../src/guest_os/windows_server_2019.rs       |  4 ++
 phd-tests/framework/src/serial/mod.rs         | 25 ++++++++--
 phd-tests/framework/src/serial/raw_buffer.rs  | 13 +++--
 phd-tests/framework/src/serial/vt80x24.rs     | 48 +++----------------
 phd-tests/framework/src/test_vm/mod.rs        | 17 ++++++-
 8 files changed, 73 insertions(+), 56 deletions(-)

diff --git a/phd-tests/framework/src/guest_os/mod.rs b/phd-tests/framework/src/guest_os/mod.rs
index c8db81a3c..908476230 100644
--- a/phd-tests/framework/src/guest_os/mod.rs
+++ b/phd-tests/framework/src/guest_os/mod.rs
@@ -27,6 +27,9 @@ pub(super) enum CommandSequenceEntry<'a> {
     /// Write the specified string as a command to the guest serial console.
     WriteStr(Cow<'a, str>),
 
+    /// Tell the serial console task to clear its buffer.
+    ClearBuffer,
+
     /// Change the serial console buffering discipline to the supplied
     /// discipline.
     ChangeSerialConsoleBuffer(crate::serial::BufferKind),
diff --git a/phd-tests/framework/src/guest_os/shell_commands.rs b/phd-tests/framework/src/guest_os/shell_commands.rs
index 895195500..2baa6dff7 100644
--- a/phd-tests/framework/src/guest_os/shell_commands.rs
+++ b/phd-tests/framework/src/guest_os/shell_commands.rs
@@ -25,6 +25,7 @@ pub(super) fn shell_command_sequence(
         crate::serial::BufferKind::Raw => CommandSequence(vec![
             CommandSequenceEntry::WriteStr(cmd),
             CommandSequenceEntry::WaitFor(echo.into()),
+            CommandSequenceEntry::ClearBuffer,
             CommandSequenceEntry::WriteStr("\n".into()),
         ]),
 
@@ -37,12 +38,22 @@ pub(super) fn shell_command_sequence(
             let cmd_lines = cmd.trim_end().lines();
             let echo_lines = echo.lines();
             let mut seq = vec![];
-            for (cmd, echo) in cmd_lines.zip(echo_lines) {
+
+            let mut iter = cmd_lines.zip(echo_lines).peekable();
+            while let Some((cmd, echo)) = iter.next() {
                 seq.push(CommandSequenceEntry::WriteStr(cmd.to_owned().into()));
                 seq.push(CommandSequenceEntry::WaitFor(echo.to_owned().into()));
-                seq.push(CommandSequenceEntry::WriteStr("\n".into()));
+
+                if iter.peek().is_some() {
+                    seq.push(CommandSequenceEntry::WriteStr("\n".into()));
+                }
             }
 
+            // Before issuing the command, clear any stale echoed characters
+            // from the serial console buffer. This ensures that the
+            seq.push(CommandSequenceEntry::ClearBuffer);
+            seq.push(CommandSequenceEntry::WriteStr("\n".into()));
+
             CommandSequence(seq)
         }
     }
diff --git a/phd-tests/framework/src/guest_os/windows_server_2016.rs b/phd-tests/framework/src/guest_os/windows_server_2016.rs
index c49715538..3b3a141aa 100644
--- a/phd-tests/framework/src/guest_os/windows_server_2016.rs
+++ b/phd-tests/framework/src/guest_os/windows_server_2016.rs
@@ -32,6 +32,10 @@ impl GuestOs for WindowsServer2016 {
     }
 
     fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        // `reset` the command prompt before issuing the command to try to force
+        // Windows to redraw the subsequent command prompt. Without this,
+        // Windows may not draw the prompt if the post-command state happens to
+        // place a prompt at a location that already had one pre-command.
         let cmd = format!("reset && {cmd}");
         super::shell_commands::shell_command_sequence(
             Cow::Owned(cmd),
diff --git a/phd-tests/framework/src/guest_os/windows_server_2019.rs b/phd-tests/framework/src/guest_os/windows_server_2019.rs
index ce71cc173..39dddc3c3 100644
--- a/phd-tests/framework/src/guest_os/windows_server_2019.rs
+++ b/phd-tests/framework/src/guest_os/windows_server_2019.rs
@@ -28,6 +28,10 @@ impl GuestOs for WindowsServer2019 {
     }
 
     fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> {
+        // `reset` the command prompt before issuing the command to try to force
+        // Windows to redraw the subsequent command prompt. Without this,
+        // Windows may not draw the prompt if the post-command state happens to
+        // place a prompt at a location that already had one pre-command.
         let cmd = format!("reset && {cmd}");
         super::shell_commands::shell_command_sequence(
             Cow::Owned(cmd),
diff --git a/phd-tests/framework/src/serial/mod.rs b/phd-tests/framework/src/serial/mod.rs
index 0fe5b1b20..d66690ac2 100644
--- a/phd-tests/framework/src/serial/mod.rs
+++ b/phd-tests/framework/src/serial/mod.rs
@@ -35,6 +35,9 @@ trait Buffer: Send {
     /// Processes the supplied `bytes` as input to the buffer.
     fn process_bytes(&mut self, bytes: &[u8]);
 
+    /// Clears the unprocessed contents of the buffer.
+    fn clear(&mut self);
+
     /// Registers a new request to wait for a string to appear in the buffer.
     fn register_wait_for_output(&mut self, waiter: OutputWaiter);
 
@@ -61,6 +64,10 @@ enum TaskCommand {
     /// Send the supplied bytes to the VM.
     SendBytes { bytes: Vec<u8>, done: oneshot::Sender<()> },
 
+    /// Clears the contents of the task's console buffer. This does not cancel
+    /// the active wait, if there is one.
+    Clear,
+
     /// Register to be notified if and when a supplied string appears in the
     /// serial console's buffer.
     RegisterWait(OutputWaiter),
@@ -117,12 +124,21 @@ impl SerialConsole {
         Ok(())
     }
 
+    /// Directs the console worker thread to clear the serial console buffer.
+    pub fn clear(&self) -> anyhow::Result<()> {
+        self.cmd_tx.send(TaskCommand::Clear)?;
+        Ok(())
+    }
+
     /// Registers with the current buffer a request to wait for `wanted` to
     /// appear in the console buffer. When a match is found, the buffer sends
-    /// all buffered characters preceding the match to `preceding_tx`, consuming
-    /// those characters and the matched string. If the buffer already contains
-    /// one or more matches at the time the waiter is registered, the last match
-    /// is used to satisfy the wait immediately.
+    /// all buffered characters preceding the match to `preceding_tx`. If the
+    /// buffer already contains one or more matches at the time the waiter is
+    /// registered, the last match is used to satisfy the wait immediately.
+    ///
+    /// Note that this function *does not* clear any characters from the buffer.
+    /// Callers who want to retire previously-echoed characters in the buffer
+    /// must explicitly call `clear`.
     pub fn register_wait_for_string(
         &self,
         wanted: String,
@@ -225,6 +241,7 @@ async fn serial_task(
 
                         let _ = done.send(());
                     }
+                    TaskCommand::Clear => buffer.clear(),
                     TaskCommand::RegisterWait(waiter) => {
                         buffer.register_wait_for_output(waiter);
                     }
diff --git a/phd-tests/framework/src/serial/raw_buffer.rs b/phd-tests/framework/src/serial/raw_buffer.rs
index 843506944..3e411a1b9 100644
--- a/phd-tests/framework/src/serial/raw_buffer.rs
+++ b/phd-tests/framework/src/serial/raw_buffer.rs
@@ -87,19 +87,14 @@ impl RawBuffer {
     fn satisfy_or_set_wait(&mut self, waiter: OutputWaiter) {
         assert!(self.waiter.is_none());
         if let Some(idx) = self.wait_buffer.rfind(&waiter.wanted) {
-            // Send all of the data in the buffer prior to the target string
-            // out the waiter's channel.
-            //
+            let out = self.wait_buffer[..idx].to_owned();
+
             // Because incoming bytes from Propolis may be processed on a
             // separate task than the task that registered the wait, this
             // can race such that the wait is satisfied just as the waiter
             // times out and closes its half of the channel. There's nothing
             // to be done about this, so just ignore any errors here.
-            let out = self.wait_buffer.drain(..idx).collect();
             let _ = waiter.preceding_tx.send(out);
-
-            // Clear the matched string out of the wait buffer.
-            self.wait_buffer = self.wait_buffer.split_off(waiter.wanted.len());
         } else {
             self.waiter = Some(waiter);
         }
@@ -126,6 +121,10 @@ impl Buffer for RawBuffer {
         }
     }
 
+    fn clear(&mut self) {
+        self.wait_buffer.clear();
+    }
+
     fn register_wait_for_output(&mut self, waiter: OutputWaiter) {
         self.satisfy_or_set_wait(waiter);
     }
diff --git a/phd-tests/framework/src/serial/vt80x24.rs b/phd-tests/framework/src/serial/vt80x24.rs
index 5df55b753..3b1b3daff 100644
--- a/phd-tests/framework/src/serial/vt80x24.rs
+++ b/phd-tests/framework/src/serial/vt80x24.rs
@@ -110,47 +110,6 @@ impl Vt80x24 {
         let mut contents = self.surface.screen_chars_to_string();
         trace!(?contents, "termwiz contents");
         if let Some(idx) = contents.rfind(&waiter.wanted) {
-            // Callers who set waits assume that matched strings are consumed
-            // from the buffer and won't match again unless the string is
-            // rendered to the buffer again. In the raw buffering case, this is
-            // straightforward: the buffer is already a String, so it suffices
-            // just to split the string just after the match. In this case,
-            // however, life is harder, because the buffer is not a string but a
-            // collection of cells containing `char`s.
-            //
-            // To "consume" the buffer, overwrite everything up through the
-            // match string with spaces. Start by truncating the current buffer
-            // contents down to the substring that ends in the match.
-            let last_byte = idx + waiter.wanted.len();
-            contents.truncate(last_byte);
-
-            // Then move the cursor to the top left and "type" as many blank
-            // spaces as there were characters in the buffer. Note that termwiz
-            // inserts extra '\n' characters at the end of every line that need
-            // to be ignored here. (It's assumed that none of the actual
-            // terminal cells contain a '\n' control character--if one of those
-            // is printed the cursor moves instead.)
-            let char_count = contents.chars().filter(|c| *c != '\n').count();
-
-            // Before typing anything, remember the old cursor position so that
-            // it can be restored after typing the spaces.
-            //
-            // It's insufficient to assume that the last character of the match
-            // was actually "typed" such that the cursor advanced past it. For
-            // example, if a match string ends with "$ ", and the guest moves to
-            // the start of an empty line and types a single "$", the cursor is
-            // in column 1 (just past the "$"), but the match is satisfied by
-            // the "pre-existing" space in that column.
-            let (old_col, old_row) = self.surface.cursor_position();
-            self.surface.add_change(make_absolute_cursor_position(0, 0));
-            self.surface.add_change(Change::Text(" ".repeat(char_count)));
-            let seq = self
-                .surface
-                .add_change(make_absolute_cursor_position(old_col, old_row));
-            self.surface.flush_changes_older_than(seq);
-
-            // Remove the match string from the match contents and push
-            // everything else back to the listener.
             contents.truncate(idx);
             let _ = waiter.preceding_tx.send(contents);
         } else {
@@ -165,6 +124,13 @@ impl Buffer for Vt80x24 {
         self.apply_actions(&actions);
     }
 
+    fn clear(&mut self) {
+        let seq = self
+            .surface
+            .add_change(Change::ClearScreen(ColorAttribute::Default));
+        self.surface.flush_changes_older_than(seq);
+    }
+
     fn register_wait_for_output(&mut self, waiter: OutputWaiter) {
         self.satisfy_or_set_wait(waiter);
     }
diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs
index 143e3538f..a2fbf8902 100644
--- a/phd-tests/framework/src/test_vm/mod.rs
+++ b/phd-tests/framework/src/test_vm/mod.rs
@@ -659,6 +659,9 @@ impl TestVm {
                         self.send_serial_str(s.as_ref()).await?;
                         self.send_serial_str("\n").await?;
                     }
+                    CommandSequenceEntry::ClearBuffer => {
+                        self.clear_serial_buffer()?
+                    }
                     CommandSequenceEntry::ChangeSerialConsoleBuffer(kind) => {
                         self.change_serial_buffer_kind(kind)?;
                     }
@@ -686,8 +689,8 @@ impl TestVm {
     }
 
     /// Waits for up to `timeout_duration` for `line` to appear on the guest
-    /// serial console, then returns the unconsumed portion of the serial
-    /// console buffer that preceded the requested string.
+    /// serial console, then returns the contents of the console buffer that
+    /// preceded the requested string.
     #[instrument(skip_all, fields(vm = self.spec.vm_name, vm_id = %self.id))]
     pub async fn wait_for_serial_output(
         &self,
@@ -746,6 +749,9 @@ impl TestVm {
                 CommandSequenceEntry::WriteStr(s) => {
                     self.send_serial_str(s.as_ref()).await?;
                 }
+                CommandSequenceEntry::ClearBuffer => {
+                    self.clear_serial_buffer()?
+                }
                 _ => {
                     anyhow::bail!(
                         "Unexpected command sequence entry {step:?} while \
@@ -792,6 +798,13 @@ impl TestVm {
         }
     }
 
+    fn clear_serial_buffer(&self) -> Result<()> {
+        match &self.state {
+            VmState::Ensured { serial } => serial.clear(),
+            VmState::New => Err(VmStateError::InstanceNotEnsured.into()),
+        }
+    }
+
     fn change_serial_buffer_kind(&self, kind: BufferKind) -> Result<()> {
         match &self.state {
             VmState::Ensured { serial } => serial.change_buffer_kind(kind),

From 2689f987c71ff76878bc461dd98d33971efb394e Mon Sep 17 00:00:00 2001
From: Greg Colombo <greg@oxidecomputer.com>
Date: Fri, 16 Feb 2024 17:50:30 +0000
Subject: [PATCH 7/8] PR feedback: clean up CommandSequenceEntry & comments

---
 phd-tests/framework/src/guest_os/alpine.rs    |  6 +--
 .../src/guest_os/debian11_nocloud.rs          |  6 +--
 phd-tests/framework/src/guest_os/mod.rs       | 12 ++++-
 .../framework/src/guest_os/shell_commands.rs  | 19 ++++----
 .../framework/src/guest_os/ubuntu22_04.rs     | 10 ++--
 phd-tests/framework/src/guest_os/windows.rs   | 48 +++++++++----------
 6 files changed, 56 insertions(+), 45 deletions(-)

diff --git a/phd-tests/framework/src/guest_os/alpine.rs b/phd-tests/framework/src/guest_os/alpine.rs
index df5072149..69dc34697 100644
--- a/phd-tests/framework/src/guest_os/alpine.rs
+++ b/phd-tests/framework/src/guest_os/alpine.rs
@@ -11,9 +11,9 @@ pub(super) struct Alpine;
 impl GuestOs for Alpine {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("localhost login: ".into()),
-            CommandSequenceEntry::WriteStr("root".into()),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt().into()),
+            CommandSequenceEntry::wait_for("localhost login: "),
+            CommandSequenceEntry::write_str("root"),
+            CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
     }
 
diff --git a/phd-tests/framework/src/guest_os/debian11_nocloud.rs b/phd-tests/framework/src/guest_os/debian11_nocloud.rs
index 48c27f29e..dec331470 100644
--- a/phd-tests/framework/src/guest_os/debian11_nocloud.rs
+++ b/phd-tests/framework/src/guest_os/debian11_nocloud.rs
@@ -11,9 +11,9 @@ pub(super) struct Debian11NoCloud;
 impl GuestOs for Debian11NoCloud {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("debian login: ".into()),
-            CommandSequenceEntry::WriteStr("root".into()),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt().into()),
+            CommandSequenceEntry::wait_for("debian login: "),
+            CommandSequenceEntry::write_str("root"),
+            CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
     }
 
diff --git a/phd-tests/framework/src/guest_os/mod.rs b/phd-tests/framework/src/guest_os/mod.rs
index 908476230..e77d70008 100644
--- a/phd-tests/framework/src/guest_os/mod.rs
+++ b/phd-tests/framework/src/guest_os/mod.rs
@@ -34,11 +34,21 @@ pub(super) enum CommandSequenceEntry<'a> {
     /// discipline.
     ChangeSerialConsoleBuffer(crate::serial::BufferKind),
 
-    /// Set a delay between writing individual bytes to the guest serial console
+    /// Set a delay between writing identical bytes to the guest serial console
     /// to avoid keyboard debouncing logic in guests.
     SetRepeatedCharacterDebounce(std::time::Duration),
 }
 
+impl<'a> CommandSequenceEntry<'a> {
+    fn write_str(s: impl Into<Cow<'a, str>>) -> Self {
+        Self::WriteStr(s.into())
+    }
+
+    fn wait_for(s: impl Into<Cow<'a, str>>) -> Self {
+        Self::WaitFor(s.into())
+    }
+}
+
 pub(super) struct CommandSequence<'a>(pub Vec<CommandSequenceEntry<'a>>);
 
 pub(super) trait GuestOs: Send + Sync {
diff --git a/phd-tests/framework/src/guest_os/shell_commands.rs b/phd-tests/framework/src/guest_os/shell_commands.rs
index 2baa6dff7..97d926b70 100644
--- a/phd-tests/framework/src/guest_os/shell_commands.rs
+++ b/phd-tests/framework/src/guest_os/shell_commands.rs
@@ -23,10 +23,10 @@ pub(super) fn shell_command_sequence(
     let echo = cmd.trim_end().replace('\n', "\n> ");
     match buffer_kind {
         crate::serial::BufferKind::Raw => CommandSequence(vec![
-            CommandSequenceEntry::WriteStr(cmd),
-            CommandSequenceEntry::WaitFor(echo.into()),
+            CommandSequenceEntry::write_str(cmd),
+            CommandSequenceEntry::wait_for(echo),
             CommandSequenceEntry::ClearBuffer,
-            CommandSequenceEntry::WriteStr("\n".into()),
+            CommandSequenceEntry::write_str("\n"),
         ]),
 
         crate::serial::BufferKind::Vt80x24 => {
@@ -41,19 +41,20 @@ pub(super) fn shell_command_sequence(
 
             let mut iter = cmd_lines.zip(echo_lines).peekable();
             while let Some((cmd, echo)) = iter.next() {
-                seq.push(CommandSequenceEntry::WriteStr(cmd.to_owned().into()));
-                seq.push(CommandSequenceEntry::WaitFor(echo.to_owned().into()));
+                seq.push(CommandSequenceEntry::write_str(cmd.to_owned()));
+                seq.push(CommandSequenceEntry::wait_for(echo.to_owned()));
 
                 if iter.peek().is_some() {
-                    seq.push(CommandSequenceEntry::WriteStr("\n".into()));
+                    seq.push(CommandSequenceEntry::write_str("\n"));
                 }
             }
 
             // Before issuing the command, clear any stale echoed characters
-            // from the serial console buffer. This ensures that the
+            // from the serial console buffer. This ensures that the next prompt
+            // is preceded in the buffer only by the output of the issued
+            // command.
             seq.push(CommandSequenceEntry::ClearBuffer);
-            seq.push(CommandSequenceEntry::WriteStr("\n".into()));
-
+            seq.push(CommandSequenceEntry::write_str("\n"));
             CommandSequence(seq)
         }
     }
diff --git a/phd-tests/framework/src/guest_os/ubuntu22_04.rs b/phd-tests/framework/src/guest_os/ubuntu22_04.rs
index 17587db25..a888f4551 100644
--- a/phd-tests/framework/src/guest_os/ubuntu22_04.rs
+++ b/phd-tests/framework/src/guest_os/ubuntu22_04.rs
@@ -12,11 +12,11 @@ pub(super) struct Ubuntu2204;
 impl GuestOs for Ubuntu2204 {
     fn get_login_sequence(&self) -> CommandSequence {
         CommandSequence(vec![
-            CommandSequenceEntry::WaitFor("ubuntu login: ".into()),
-            CommandSequenceEntry::WriteStr("ubuntu".into()),
-            CommandSequenceEntry::WaitFor("Password: ".into()),
-            CommandSequenceEntry::WriteStr("1!Passw0rd".into()),
-            CommandSequenceEntry::WaitFor(self.get_shell_prompt().into()),
+            CommandSequenceEntry::wait_for("ubuntu login: "),
+            CommandSequenceEntry::write_str("ubuntu"),
+            CommandSequenceEntry::wait_for("Password: "),
+            CommandSequenceEntry::write_str("1!Passw0rd"),
+            CommandSequenceEntry::wait_for(self.get_shell_prompt()),
         ])
     }
 
diff --git a/phd-tests/framework/src/guest_os/windows.rs b/phd-tests/framework/src/guest_os/windows.rs
index 61ed3c2c9..7a06cf338 100644
--- a/phd-tests/framework/src/guest_os/windows.rs
+++ b/phd-tests/framework/src/guest_os/windows.rs
@@ -25,27 +25,27 @@ pub(super) fn get_login_sequence_for<'a>(
     ));
 
     let mut commands = vec![
-        CommandSequenceEntry::WaitFor(
-            "Computer is booting, SAC started and initialized.".into(),
+        CommandSequenceEntry::wait_for(
+            "Computer is booting, SAC started and initialized.",
         ),
-        CommandSequenceEntry::WaitFor(
-            "EVENT: The CMD command is now available.".into(),
+        CommandSequenceEntry::wait_for(
+            "EVENT: The CMD command is now available.",
         ),
-        CommandSequenceEntry::WaitFor("SAC>".into()),
-        CommandSequenceEntry::WriteStr("cmd".into()),
-        CommandSequenceEntry::WaitFor("Channel: Cmd0001".into()),
-        CommandSequenceEntry::WaitFor("SAC>".into()),
-        CommandSequenceEntry::WriteStr("ch -sn Cmd0001".into()),
-        CommandSequenceEntry::WaitFor(
-            "Use any other key to view this channel.".into(),
+        CommandSequenceEntry::wait_for("SAC>"),
+        CommandSequenceEntry::write_str("cmd"),
+        CommandSequenceEntry::wait_for("Channel: Cmd0001"),
+        CommandSequenceEntry::wait_for("SAC>"),
+        CommandSequenceEntry::write_str("ch -sn Cmd0001"),
+        CommandSequenceEntry::wait_for(
+            "Use any other key to view this channel.",
         ),
-        CommandSequenceEntry::WriteStr("".into()),
-        CommandSequenceEntry::WaitFor("Username:".into()),
-        CommandSequenceEntry::WriteStr("Administrator".into()),
-        CommandSequenceEntry::WaitFor("Domain  :".into()),
-        CommandSequenceEntry::WriteStr("".into()),
-        CommandSequenceEntry::WaitFor("Password:".into()),
-        CommandSequenceEntry::WriteStr("0xide#1Fan".into()),
+        CommandSequenceEntry::write_str(""),
+        CommandSequenceEntry::wait_for("Username:"),
+        CommandSequenceEntry::write_str("Administrator"),
+        CommandSequenceEntry::wait_for("Domain  :"),
+        CommandSequenceEntry::write_str(""),
+        CommandSequenceEntry::wait_for("Password:"),
+        CommandSequenceEntry::write_str("0xide#1Fan"),
     ];
 
     // Earlier Windows Server versions' serial console-based command prompts
@@ -78,14 +78,14 @@ pub(super) fn get_login_sequence_for<'a>(
         // eat the command and just process the newline). It also appears to
         // prefer carriage returns to linefeeds. Accommodate this behavior
         // until Cygwin is launched.
-        CommandSequenceEntry::WaitFor("C:\\Windows\\system32>".into()),
-        CommandSequenceEntry::WriteStr("cls\r".into()),
-        CommandSequenceEntry::WaitFor("C:\\Windows\\system32>".into()),
-        CommandSequenceEntry::WriteStr("C:\\cygwin\\cygwin.bat\r".into()),
-        CommandSequenceEntry::WaitFor("$ ".into()),
+        CommandSequenceEntry::wait_for("C:\\Windows\\system32>"),
+        CommandSequenceEntry::write_str("cls\r"),
+        CommandSequenceEntry::wait_for("C:\\Windows\\system32>"),
+        CommandSequenceEntry::write_str("C:\\cygwin\\cygwin.bat\r"),
+        CommandSequenceEntry::wait_for("$ "),
         // Tweak the command prompt so that it appears on a single line with
         // no leading newlines.
-        CommandSequenceEntry::WriteStr("PS1='\\u@\\h:$ '".into()),
+        CommandSequenceEntry::write_str("PS1='\\u@\\h:$ '"),
     ]);
 
     CommandSequence(commands)

From b5b8d814a9994ae0fe71c82825cb10fc8da28f62 Mon Sep 17 00:00:00 2001
From: Greg Colombo <greg@oxidecomputer.com>
Date: Fri, 16 Feb 2024 19:27:08 +0000
Subject: [PATCH 8/8] PR feedback & comment cleanup

---
 phd-tests/framework/src/serial/mod.rs  | 11 ++---
 phd-tests/framework/src/test_vm/mod.rs | 61 +++++++++++++-------------
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/phd-tests/framework/src/serial/mod.rs b/phd-tests/framework/src/serial/mod.rs
index d66690ac2..fbff7f50c 100644
--- a/phd-tests/framework/src/serial/mod.rs
+++ b/phd-tests/framework/src/serial/mod.rs
@@ -114,14 +114,15 @@ impl SerialConsole {
     }
 
     /// Directs the console worker thread to send the supplied `bytes` to the
-    /// guest, after which it should send to `done`.
+    /// guest. Returns a `oneshot::Receiver` that the console worker thread
+    /// signals once all the bytes have been set.
     pub fn send_bytes(
         &self,
         bytes: Vec<u8>,
-        done: oneshot::Sender<()>,
-    ) -> anyhow::Result<()> {
+    ) -> anyhow::Result<oneshot::Receiver<()>> {
+        let (done, done_rx) = oneshot::channel();
         self.cmd_tx.send(TaskCommand::SendBytes { bytes, done })?;
-        Ok(())
+        Ok(done_rx)
     }
 
     /// Directs the console worker thread to clear the serial console buffer.
@@ -164,7 +165,7 @@ impl SerialConsole {
     }
 
     /// Sets the delay to insert between sending individual bytes to the guest.
-    pub fn set_guest_write_delay(
+    pub fn set_repeated_character_debounce(
         &self,
         delay: std::time::Duration,
     ) -> Result<()> {
diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs
index a2fbf8902..9298fa043 100644
--- a/phd-tests/framework/src/test_vm/mod.rs
+++ b/phd-tests/framework/src/test_vm/mod.rs
@@ -668,7 +668,7 @@ impl TestVm {
                     CommandSequenceEntry::SetRepeatedCharacterDebounce(
                         duration,
                     ) => {
-                        self.set_serial_byte_write_delay(duration)?;
+                        self.set_serial_repeated_character_debounce(duration)?;
                     }
                 }
             }
@@ -736,6 +736,10 @@ impl TestVm {
     /// [`Self::wait_for_serial_output`] and returns any text that was buffered
     /// to the serial console after the command was sent.
     pub async fn run_shell_command(&self, cmd: &str) -> Result<String> {
+        // Allow the guest OS to transform the input command into a
+        // guest-specific command sequence. This accounts for the guest's shell
+        // type (which affects e.g. affects how it displays multi-line commands)
+        // and serial console buffering discipline.
         let command_sequence = self.guest_os.shell_command_sequence(cmd);
         for step in command_sequence.0 {
             match step {
@@ -761,10 +765,11 @@ impl TestVm {
             }
         }
 
-        // Once the command has run, the guest should display another prompt.
-        // Treat the unconsumed buffered text before this point as the command
-        // output. (Note again that the command itself was already consumed by
-        // the wait above.)
+        // `shell_command_sequence` promises that the generated command sequence
+        // clears buffer of everything up to and including the input command
+        // before actually issuing the final '\n' that issues the command.
+        // This ensures that the buffer contents returned by this call contain
+        // only the command's output.
         let out = self
             .wait_for_serial_output(
                 self.guest_os.get_shell_prompt(),
@@ -772,54 +777,48 @@ impl TestVm {
             )
             .await?;
 
-        // Trim both ends of the output to get rid of any echoed newlines and/or
-        // whitespace that were inserted when sending '\n' to start processing
-        // the command.
+        // Trim any leading newlines inserted when the command was issued and
+        // any trailing whitespace that isn't actually part of the command
+        // output. Any other embedded whitespace is the caller's problem.
         Ok(out.trim().to_string())
     }
 
+    /// Sends `string` to the guest's serial console worker, then waits for the
+    /// entire string to be sent to the guest before returning.
     pub(crate) async fn send_serial_str(&self, string: &str) -> Result<()> {
         if !string.is_empty() {
-            self.send_serial_bytes_async(Vec::from(string.as_bytes())).await
-        } else {
-            Ok(())
+            self.send_serial_bytes(Vec::from(string.as_bytes()))?.await?;
         }
+        Ok(())
     }
 
-    async fn send_serial_bytes_async(&self, bytes: Vec<u8>) -> Result<()> {
+    fn serial_console(&self) -> Result<&SerialConsole> {
         match &self.state {
-            VmState::Ensured { serial } => {
-                let (done_tx, done_rx) = oneshot::channel();
-                serial.send_bytes(bytes, done_tx)?;
-                done_rx.await?;
-                Ok(())
-            }
+            VmState::Ensured { serial } => Ok(serial),
             VmState::New => Err(VmStateError::InstanceNotEnsured.into()),
         }
     }
 
+    fn send_serial_bytes(
+        &self,
+        bytes: Vec<u8>,
+    ) -> Result<oneshot::Receiver<()>> {
+        self.serial_console()?.send_bytes(bytes)
+    }
+
     fn clear_serial_buffer(&self) -> Result<()> {
-        match &self.state {
-            VmState::Ensured { serial } => serial.clear(),
-            VmState::New => Err(VmStateError::InstanceNotEnsured.into()),
-        }
+        self.serial_console()?.clear()
     }
 
     fn change_serial_buffer_kind(&self, kind: BufferKind) -> Result<()> {
-        match &self.state {
-            VmState::Ensured { serial } => serial.change_buffer_kind(kind),
-            VmState::New => Err(VmStateError::InstanceNotEnsured.into()),
-        }
+        self.serial_console()?.change_buffer_kind(kind)
     }
 
-    fn set_serial_byte_write_delay(
+    fn set_serial_repeated_character_debounce(
         &self,
         delay: std::time::Duration,
     ) -> Result<()> {
-        match &self.state {
-            VmState::Ensured { serial } => serial.set_guest_write_delay(delay),
-            VmState::New => Err(VmStateError::InstanceNotEnsured.into()),
-        }
+        self.serial_console()?.set_repeated_character_debounce(delay)
     }
 
     /// Indicates whether this VM's guest OS has a read-only filesystem.