Commit d588268
Eric Bower
·
2026-05-20 10:41:34 -0400 EDT
parent bcf57a1
refactor(run): when creating session will always run `/bin/bash` BREAKING CHANGE: `zmx run` now requires the target shell to support `$?` exit status tracking
5 files changed,
+94,
-75
+7,
-0
| ... | ... | @@ -4,6 +4,13 @@ Use spec: https://common-changelog.org/ | |
| 4 | 4 | ||
| 5 | 5 | ## Staged | |
| 6 | 6 | ||
| 7 | + | ### Changed | |
| 8 | + | ||
| 9 | + | - *BREAKING* `zmx run` when creating session it runs `/bin/bash` instead of `$SHELL` | |
| 10 | + | - There are just too many edge cases with tracking exit status in other shells which makes | |
| 11 | + | `zmx run` much less useful for task management. | |
| 12 | + | - This means when using `zmx run` the target shell must have support for `$?` exit code tracking | |
| 13 | + | ||
| 7 | 14 | ## v0.6.0 - 2026-05-16 | |
| 8 | 15 | ||
| 9 | 16 | ### Added |
+1,
-1
| ... | ... | @@ -78,7 +78,7 @@ Usage: zmx <command> [args...] | |
| 78 | 78 | ||
| 79 | 79 | Commands: | |
| 80 | 80 | [a]ttach <name> [command...] Attach to session, creating if needed | |
| 81 | - | [r]un <name> [-d] [--fish] [command...] Send command without attaching | |
| 81 | + | [r]un <name> [-d] [command...] Send command without attaching | |
| 82 | 82 | [s]end <name> <text...> Send raw input to session PTY | |
| 83 | 83 | [p]rint <name> <text...> Inject text into session display | |
| 84 | 84 | [wr]ite <name> <file_path> Write stdin to file_path through the session |
+0,
-57
| ... | ... | @@ -5,7 +5,6 @@ const posix = std.posix; | |
| 5 | 5 | pub const c = switch (builtin.os.tag) { | |
| 6 | 6 | .macos => @cImport({ | |
| 7 | 7 | @cInclude("sys/ioctl.h"); // ioctl and constants | |
| 8 | - | @cInclude("sys/sysctl.h"); // sysctl for process name lookup | |
| 9 | 8 | @cInclude("termios.h"); | |
| 10 | 9 | @cInclude("stdlib.h"); | |
| 11 | 10 | @cInclude("unistd.h"); |
| ... | ... | @@ -31,59 +30,3 @@ pub const forkpty = if (builtin.os.tag == .macos) | |
| 31 | 30 | }.forkpty | |
| 32 | 31 | else | |
| 33 | 32 | c.forkpty; | |
| 34 | - | ||
| 35 | - | /// Returns the basename of the foreground process running on the given PTY fd. | |
| 36 | - | /// Writes into `buf` and returns a slice of it, or null on failure. | |
| 37 | - | pub fn getForegroundProcessName(pty_fd: i32, buf: []u8) ?[]const u8 { | |
| 38 | - | const pgid = c.tcgetpgrp(pty_fd); | |
| 39 | - | if (pgid <= 0) return null; | |
| 40 | - | ||
| 41 | - | switch (builtin.os.tag) { | |
| 42 | - | .macos => { | |
| 43 | - | // Use KERN_PROC_PGRP to find the process in the foreground group. | |
| 44 | - | // We walk the process list and find the first process whose pgid matches. | |
| 45 | - | var mib = [_]c_int{ c.CTL_KERN, c.KERN_PROC, c.KERN_PROC_PGRP, @intCast(pgid) }; | |
| 46 | - | var size: usize = 0; | |
| 47 | - | if (c.sysctl(&mib, mib.len, null, &size, null, 0) != 0) return null; | |
| 48 | - | if (size == 0) return null; | |
| 49 | - | ||
| 50 | - | // kinfo_proc is large; allocate on heap to avoid blowing the stack | |
| 51 | - | const kinfo_size = @sizeOf(c.struct_kinfo_proc); | |
| 52 | - | const count = size / kinfo_size; | |
| 53 | - | if (count == 0) return null; | |
| 54 | - | ||
| 55 | - | // Use a stack buffer for small lists (usually 1-3 procs), heap otherwise. | |
| 56 | - | var stack_buf: [8 * @sizeOf(c.struct_kinfo_proc)]u8 align(@alignOf(c.struct_kinfo_proc)) = undefined; | |
| 57 | - | const heap_needed = size > stack_buf.len; | |
| 58 | - | const proc_buf: []u8 = if (heap_needed) | |
| 59 | - | std.heap.c_allocator.alloc(u8, size) catch return null | |
| 60 | - | else | |
| 61 | - | stack_buf[0..size]; | |
| 62 | - | defer if (heap_needed) std.heap.c_allocator.free(proc_buf); | |
| 63 | - | ||
| 64 | - | if (c.sysctl(&mib, mib.len, proc_buf.ptr, &size, null, 0) != 0) return null; | |
| 65 | - | ||
| 66 | - | const procs: []c.struct_kinfo_proc = @alignCast(std.mem.bytesAsSlice(c.struct_kinfo_proc, proc_buf[0..size])); | |
| 67 | - | if (procs.len == 0) return null; | |
| 68 | - | ||
| 69 | - | // p_comm is a null-terminated fixed-length field | |
| 70 | - | const comm: [*:0]const u8 = @ptrCast(&procs[0].kp_proc.p_comm); | |
| 71 | - | const name = std.mem.sliceTo(comm, 0); | |
| 72 | - | const copy_len = @min(name.len, buf.len); | |
| 73 | - | @memcpy(buf[0..copy_len], name[0..copy_len]); | |
| 74 | - | return buf[0..copy_len]; | |
| 75 | - | }, | |
| 76 | - | .linux => { | |
| 77 | - | // /proc/<pid>/comm contains just the process name + newline | |
| 78 | - | var path_buf: [64]u8 = undefined; | |
| 79 | - | const path = std.fmt.bufPrint(&path_buf, "/proc/{d}/comm", .{pgid}) catch return null; | |
| 80 | - | const file = std.fs.openFileAbsolute(path, .{}) catch return null; | |
| 81 | - | defer file.close(); | |
| 82 | - | const n = file.read(buf) catch return null; | |
| 83 | - | // strip trailing newline | |
| 84 | - | const end = if (n > 0 and buf[n - 1] == '\n') n - 1 else n; | |
| 85 | - | return buf[0..end]; | |
| 86 | - | }, | |
| 87 | - | else => return null, | |
| 88 | - | } | |
| 89 | - | } |
+5,
-17
| ... | ... | @@ -594,7 +594,6 @@ const Daemon = struct { | |
| 594 | 594 | is_task_mode: bool = false, // flag for when session is run as a task | |
| 595 | 595 | task_exit_code: ?u8 = null, // null = running or n/a, set when task completes | |
| 596 | 596 | task_ended_at: ?u64 = null, // timestamp when task exited | |
| 597 | - | is_fish: bool = false, // true if session shell is fish (affects exit code variable) | |
| 598 | 597 | pty_fd: i32 = -1, // set by daemonLoop so handleRun can probe the foreground process | |
| 599 | 598 | pty_write_buf: std.ArrayList(u8) = .empty, | |
| 600 | 599 |
| ... | ... | @@ -683,7 +682,7 @@ const Daemon = struct { | |
| 683 | 682 | std.posix.exit(1); | |
| 684 | 683 | } | |
| 685 | 684 | ||
| 686 | - | const shell = util.detectShell(); | |
| 685 | + | const shell: [:0]const u8 = if (self.is_task_mode) "/bin/bash" else util.detectShell(); | |
| 687 | 686 | // Use "-shellname" as argv[0] to signal login shell (traditional method) | |
| 688 | 687 | const login_shell = try std.fmt.allocPrintSentinel( | |
| 689 | 688 | alloc, |
| ... | ... | @@ -1139,30 +1138,19 @@ const Daemon = struct { | |
| 1139 | 1138 | ||
| 1140 | 1139 | if (payload.len == 0) return; | |
| 1141 | 1140 | ||
| 1142 | - | // Auto-detect the foreground process on the PTY to determine shell type. | |
| 1143 | - | if (self.pty_fd >= 0) { | |
| 1144 | - | var name_buf: [64]u8 = undefined; | |
| 1145 | - | if (cross.getForegroundProcessName(self.pty_fd, &name_buf)) |name| { | |
| 1146 | - | self.is_fish = std.mem.eql(u8, name, "fish"); | |
| 1147 | - | std.log.debug("foreground process={s} is_fish={}", .{ name, self.is_fish }); | |
| 1148 | - | } | |
| 1149 | - | } | |
| 1150 | 1141 | const cmd = payload; | |
| 1151 | 1142 | ||
| 1152 | - | // Daemon appends the task marker so the client never injects | |
| 1153 | - | // shell-specific syntax, keeping Ctrl-C recovery clean. | |
| 1154 | - | const marker = if (self.is_fish) | |
| 1155 | - | "; echo ZMX_TASK_COMPLETED:$status" | |
| 1156 | - | else | |
| 1157 | - | "; echo ZMX_TASK_COMPLETED:$?"; | |
| 1143 | + | // Daemon appends the task marker so we know when a task is done with | |
| 1144 | + | // exit status | |
| 1145 | + | const marker = "\necho ZMX_TASK_COMPLETED:$?\r"; | |
| 1158 | 1146 | ||
| 1159 | 1147 | if (cmd.len > 0 and cmd[cmd.len - 1] == '\r') { | |
| 1160 | 1148 | self.queuePtyInput(cmd[0 .. cmd.len - 1]); | |
| 1161 | 1149 | } else { | |
| 1162 | 1150 | self.queuePtyInput(cmd); | |
| 1163 | 1151 | } | |
| 1164 | - | self.queuePtyInput(marker); | |
| 1165 | 1152 | self.queuePtyInput("\r"); | |
| 1153 | + | self.queuePtyInput(marker); | |
| 1166 | 1154 | ||
| 1167 | 1155 | try ipc.appendMessage(self.alloc, &client.write_buf, .Ack, ""); | |
| 1168 | 1156 | client.has_pending_output = true; |
+81,
-0
| ... | ... | @@ -0,0 +1,81 @@ | |
| 1 | + | #!/usr/bin/env bats | |
| 2 | + | # Tests for stdin piped to `zmx run` sessions. | |
| 3 | + | # | |
| 4 | + | # Verifies that `zmx run` sessions always use bash (not the user's $SHELL), | |
| 5 | + | # so piping commands via stdin works without quoting issues regardless of | |
| 6 | + | # the user's default shell. | |
| 7 | + | ||
| 8 | + | load test_helper | |
| 9 | + | ||
| 10 | + | # ============================================================================ | |
| 11 | + | # Session shell is always bash | |
| 12 | + | # ============================================================================ | |
| 13 | + | ||
| 14 | + | @test "run: session uses bash regardless of SHELL env" { | |
| 15 | + | run timeout 10 env SHELL=/usr/bin/fish "$ZMX" run test-shell-check echo 'hello' | |
| 16 | + | [ "$status" -eq 0 ] | |
| 17 | + | ||
| 18 | + | sleep 0.3 | |
| 19 | + | run "$ZMX" history test-shell-check | |
| 20 | + | # Task marker uses $? (bash syntax), not $status (fish syntax) | |
| 21 | + | [[ "$output" == *'ZMX_TASK_COMPLETED:'* ]] | |
| 22 | + | [[ "$output" == *'$?'* ]] | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | # ============================================================================ | |
| 26 | + | # Stdin piped to run | |
| 27 | + | # ============================================================================ | |
| 28 | + | ||
| 29 | + | @test "run: stdin pipe executes command" { | |
| 30 | + | run bash -c 'printf "echo stdin-marker-abc123\n" | timeout 10 "$0" run test-stdin-basic' "$ZMX" | |
| 31 | + | [ "$status" -eq 0 ] | |
| 32 | + | ||
| 33 | + | sleep 0.3 | |
| 34 | + | run "$ZMX" history test-stdin-basic | |
| 35 | + | [[ "$output" == *"stdin-marker-abc123"* ]] | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | @test "run: stdin with special characters passes through unmangled" { | |
| 39 | + | run bash -c 'printf "echo '\''hello \$USER \$(whoami) \\\"double\\\" ; # comment'\''\n" | timeout 10 "$0" run test-stdin-special' "$ZMX" | |
| 40 | + | [ "$status" -eq 0 ] | |
| 41 | + | ||
| 42 | + | sleep 0.3 | |
| 43 | + | run "$ZMX" history test-stdin-special | |
| 44 | + | [[ "$output" == *'$USER'* ]] | |
| 45 | + | [[ "$output" == *'$(whoami)'* ]] | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | @test "run: multiline stdin script executes all lines" { | |
| 49 | + | local script | |
| 50 | + | script=$(printf 'echo line-one-aaa\necho line-two-bbb\necho line-three-ccc\n') | |
| 51 | + | run bash -c 'printf "%s" "$1" | timeout 10 "$0" run test-stdin-multi' "$ZMX" "$script" | |
| 52 | + | [ "$status" -eq 0 ] | |
| 53 | + | ||
| 54 | + | sleep 0.5 | |
| 55 | + | run "$ZMX" history test-stdin-multi | |
| 56 | + | [[ "$output" == *"line-one-aaa"* ]] | |
| 57 | + | [[ "$output" == *"line-two-bbb"* ]] | |
| 58 | + | [[ "$output" == *"line-three-ccc"* ]] | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | @test "run: stdin with heredoc in script" { | |
| 62 | + | # Heredoc delimiter as the last line of stdin should work now that | |
| 63 | + | # the task marker is sent on its own line. | |
| 64 | + | local script | |
| 65 | + | script=$(printf "cat <<'EOF'\nThis has \"double\" and 'single' quotes\nand \$variables that should not expand\nEOF\n") | |
| 66 | + | run bash -c 'printf "%s" "$1" | timeout 10 "$0" run test-stdin-heredoc' "$ZMX" "$script" | |
| 67 | + | [ "$status" -eq 0 ] | |
| 68 | + | ||
| 69 | + | sleep 0.5 | |
| 70 | + | run "$ZMX" history test-stdin-heredoc | |
| 71 | + | [[ "$output" == *'$variables that should not expand'* ]] | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | @test "run: args-only still works" { | |
| 75 | + | run timeout 10 env SHELL=/bin/bash "$ZMX" run test-args-only echo args-only-marker-999 | |
| 76 | + | [ "$status" -eq 0 ] | |
| 77 | + | ||
| 78 | + | sleep 0.3 | |
| 79 | + | run "$ZMX" history test-args-only | |
| 80 | + | [[ "$output" == *"args-only-marker-999"* ]] | |
| 81 | + | } |