diff --git a/fleetd/src/test/java/dev/ltms/fleet/deploy/SystemdUnitSafetyTest.java b/fleetd/src/test/java/dev/ltms/fleet/deploy/SystemdUnitSafetyTest.java
index db916c2..f27f3cf 100644
--- a/fleetd/src/test/java/dev/ltms/fleet/deploy/SystemdUnitSafetyTest.java
+++ b/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.
*
*
- *
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.
+ *
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.
+ *
+ *
+ * - 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.
+ *
- 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.
+ *
+ *
+ * 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.
*
*
It checks source text, not behaviour -- 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 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 bothUnits() {
return Stream.of(FLEETD_SERVICE, HERDR_SERVICE);
}
+ private static boolean invokesALoginShell(List 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 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 activeLines(Path unit) throws Exception {
List 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 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 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) {