fleetd #360 review round 2: extend SystemdUnitSafetyTest to herdr-inner.sh

herdr.service's ExecStart only names deploy/herdr-inner.sh, so that script was the only
place herdr's login-shell and pty-size properties lived, and nothing was reading it -- the
same silent-at-startup shape the ticket was about, one file further down the chain. A
mutation dropping both the login shell and the stty sizing left SystemdUnitSafetyTest green.

Add herdrInnerScriptUsesALoginShell and herdrInnerScriptSetsANonZeroPtySize, extend the
vacuity guard to cover herdr-inner.sh too, and tighten execStartUsesALoginShell's check from
a bare contains("-lc") substring match to the same login-shell-invocation regex the new
checks use.

fleetd/src/test/java/dev/ltms/fleet/deploy/SystemdUnitSafetyTest.java

@@ -43,9 +43,22 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  *       member that cannot open a pull request.
  * </ul>
  *
- * <p>None of these three failures makes the unit fail to start or makes {@code /healthz} report
- * unhealthy, so nothing short of reading the unit file catches a regression. This test is that
- * read.
+ * <p>Review round 2 on fleetd #360 found the same shape one file further down the chain:
+ * {@code herdr.service}'s {@code ExecStart} only names {@code deploy/herdr-inner.sh}, so that
+ * script is the ONLY place two more of these properties live, and nothing was reading it.
+ *
+ * <ul>
+ *   <li>If the script does not exec a login shell, herdr -- and every member pane it spawns as a
+ *       child -- starts with none of the secrets only a login shell sources, the same silent
+ *       empty-credentials failure as fleetd.service's {@code ExecStart}, one process further away.
+ *   <li>If the script does not set a real, non-zero pty size before starting herdr (with
+ *       {@code stty}), herdr reports a 0x0 window and every pane spawn fails with
+ *       "ghostty error -2" -- which surfaces as a fleetd spawn failure, not a herdr one.
+ * </ul>
+ *
+ * <p>None of these five failures makes the unit (or the script) fail to start, or makes
+ * {@code /healthz} report unhealthy, so nothing short of reading the files catches a regression.
+ * This test is that read.
  *
  * <p><b>It checks source text, not behaviour</b> -- it cannot start systemd or fork an mount
  * namespace in a build sandbox. It parses the same two files an operator would install and fails
@@ -57,14 +70,38 @@ class SystemdUnitSafetyTest {
     /** Tests run with the module directory (fleetd/) as cwd; deploy/ is the repo-root sibling. */
     private static final Path FLEETD_SERVICE = Path.of("../deploy/fleetd.service");
     private static final Path HERDR_SERVICE = Path.of("../deploy/herdr.service");
+    private static final Path HERDR_INNER_SCRIPT = Path.of("../deploy/herdr-inner.sh");
 
     private static final List<String> NAMESPACING_DIRECTIVES = List.of(
             "ProtectSystem", "ProtectHome", "ProtectKernelTunables", "ProtectControlGroups");
 
+    /**
+     * A shell invoked with a login flag, e.g. {@code zsh -lc '...'} or {@code /bin/sh -l}. The
+     * property under test is the {@code -l}, not the specific shell or the rest of its flags, so
+     * this matches a token ending in "sh" followed by a flag cluster containing "l" -- tighter
+     * than a bare {@code contains("-lc")}, which a "-lc" anywhere in the line would also satisfy.
+     */
+    private static final Pattern LOGIN_SHELL_INVOCATION =
+            Pattern.compile("(?:^|\\s)\\S*sh\\s+-[A-Za-z]*l[A-Za-z]*\\b");
+
+    private static final Pattern POSITIVE_ROWS = Pattern.compile("\\brows\\s+([1-9]\\d*)\\b");
+    private static final Pattern POSITIVE_COLS = Pattern.compile("\\bcols\\s+([1-9]\\d*)\\b");
+
     static Stream<Path> bothUnits() {
         return Stream.of(FLEETD_SERVICE, HERDR_SERVICE);
     }
 
+    private static boolean invokesALoginShell(List<String> activeLines) {
+        return activeLines.stream().anyMatch(l -> LOGIN_SHELL_INVOCATION.matcher(l).find());
+    }
+
+    /** An active {@code stty} line naming both a positive row count and a positive column count. */
+    private static boolean setsANonZeroPtySize(List<String> activeLines) {
+        return activeLines.stream()
+                .filter(l -> l.startsWith("stty"))
+                .anyMatch(l -> POSITIVE_ROWS.matcher(l).find() && POSITIVE_COLS.matcher(l).find());
+    }
+
     /** Lines that are actually in force: comments and blank lines don't count. */
     private static List<String> activeLines(Path unit) throws Exception {
         List<String> active = new ArrayList<>();
@@ -125,23 +162,51 @@ class SystemdUnitSafetyTest {
                         + execStartLines.size() + ": " + execStartLines);
 
         String execStart = execStartLines.get(0);
-        assertTrue(execStart.contains("-lc"),
-                FLEETD_SERVICE + "'s ExecStart (" + execStart + ") does not run a login shell (\"-lc\"). "
-                        + "Every secret fleetd needs (AI_GATEWAY_TOKEN, WORKER_GITEA_TOKEN, LAVINMQ_URI, "
-                        + "COORD_AMQP_URI) lives in a file only the login shell sources; systemd runs no "
-                        + "login shell on its own. Running java directly boots fine with every credential "
-                        + "empty, and the failure surfaces hours later as a member that cannot open a "
-                        + "pull request (fleetd #360).");
+        assertTrue(LOGIN_SHELL_INVOCATION.matcher(execStart).find(),
+                FLEETD_SERVICE + "'s ExecStart (" + execStart + ") does not run a login shell (a shell "
+                        + "invoked with a \"-l\" flag, e.g. \"zsh -lc\"). Every secret fleetd needs "
+                        + "(AI_GATEWAY_TOKEN, WORKER_GITEA_TOKEN, LAVINMQ_URI, COORD_AMQP_URI) lives in a "
+                        + "file only the login shell sources; systemd runs no login shell on its own. "
+                        + "Running java directly boots fine with every credential empty, and the failure "
+                        + "surfaces hours later as a member that cannot open a pull request (fleetd #360).");
+    }
+
+    @Test
+    @DisplayName("[SOURCE TEXT] herdr-inner.sh execs a login shell -- otherwise herdr and every member it spawns start with empty credentials")
+    void herdrInnerScriptUsesALoginShell() throws Exception {
+        List<String> active = activeLines(HERDR_INNER_SCRIPT);
+
+        assertTrue(invokesALoginShell(active),
+                HERDR_INNER_SCRIPT + " does not exec a login shell (a shell invoked with a \"-l\" flag, "
+                        + "e.g. \"zsh -lc\"). herdr.service's ExecStart only names this script, so this "
+                        + "is the ONLY place herdr's login-shell property lives. Without it, herdr -- and "
+                        + "every member pane it spawns as a child of herdr -- starts with none of the "
+                        + "secrets that only a login shell sources (this host: ~/.fleet/secrets.sh via "
+                        + "~/.zprofile), and the failure surfaces hours later as a member with no "
+                        + "credentials at all, not just fleetd (fleetd #360).");
+    }
+
+    @Test
+    @DisplayName("[SOURCE TEXT] herdr-inner.sh sets a non-zero pty size before starting herdr -- otherwise every pane spawn fails with \"ghostty error -2\"")
+    void herdrInnerScriptSetsANonZeroPtySize() throws Exception {
+        List<String> active = activeLines(HERDR_INNER_SCRIPT);
+
+        assertTrue(setsANonZeroPtySize(active),
+                HERDR_INNER_SCRIPT + " does not run \"stty rows N cols M\" with both N and M positive "
+                        + "before starting herdr. Without a real pty size, herdr reports a 0x0 window and "
+                        + "every pane spawn fails with \"ghostty error -2\" -- which surfaces as a fleetd "
+                        + "spawn failure, not a herdr one, and gives no hint that the actual cause is this "
+                        + "script (fleetd #360).");
     }
 
     /**
      * The denominator guard, same shape as {@code McpContractDocTest}'s: a check that scans for a
-     * forbidden pattern passes trivially if it is handed nothing to scan. Pin that both files exist,
-     * are non-trivial, and that the DO-NOT block's commented mentions are still there -- so the
-     * "comment survives, directive doesn't" distinction above is actually being exercised.
+     * forbidden pattern passes trivially if it is handed nothing to scan. Pin that all three files
+     * exist, are non-trivial, and that the DO-NOT block's commented mentions are still there -- so
+     * the "comment survives, directive doesn't" distinction above is actually being exercised.
      */
     @Test
-    @DisplayName("[SOURCE TEXT] the namespacing check is not vacuous -- both files exist and the DO-NOT block still mentions every forbidden directive in a comment")
+    @DisplayName("[SOURCE TEXT] the namespacing check is not vacuous -- all three files exist and the DO-NOT block still mentions every forbidden directive in a comment")
     void theCheckActuallyHasSomethingToCheck() throws Exception {
         assertTrue(Files.size(FLEETD_SERVICE) > 200,
                 FLEETD_SERVICE + " is missing or unexpectedly small -- the checks above would pass "
@@ -149,6 +214,11 @@ class SystemdUnitSafetyTest {
         assertTrue(Files.size(HERDR_SERVICE) > 200,
                 HERDR_SERVICE + " is missing or unexpectedly small -- the checks above would pass "
                         + "vacuously against an empty or absent file");
+        assertTrue(Files.size(HERDR_INNER_SCRIPT) > 50,
+                HERDR_INNER_SCRIPT + " is missing or unexpectedly small -- the login-shell and "
+                        + "pty-size checks above would either pass vacuously or fail with an opaque "
+                        + "\"file not found\" instead of naming the actual consequence (empty "
+                        + "credentials / \"ghostty error -2\") against an empty or absent file");
 
         String fleetdService = Files.readString(FLEETD_SERVICE);
         for (String directive : NAMESPACING_DIRECTIVES) {