A pty/ctl exec that cannot start its shell keeps the one running

Each host closed the running shell before it forked the new one, so an exec whose shell failed left the pane with none and later runs answered error shell gone. A shell not there is now refused up front, and every host starts the new shell first, replacing the old only once the close-on-exec pipe says it ran; a failure there is only said (restartFailed).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

9 files changed, 103 insertions, 22 deletions

diff --git a/docs/fs.md b/docs/fs.md
index 4e3428d1..68cc38a9 100644
--- a/docs/fs.md
+++ b/docs/fs.md
@@ -958,7 +958,9 @@ its directory: one that is gone is refused before anything runs, `exec:
 there, not executable, a script whose interpreter is not there) fails the
 write with why -- `shell: shell not found`, or `shell: interpreter
 /no/such/interp not found` for a script whose `#!` names a program that is
-not there, ENOENT -- keeping the terminal; a `Tty` naming such a shell or
+not there, ENOENT -- keeping the terminal and its running shell: a shell
+not there is refused before anything runs, and the host starts the new one
+before the old goes, so one that cannot start leaves the old be; a `Tty` naming such a shell or
 script is refused before anything runs (`Tty: interpreter ... not found`,
 its `err` the only record), and one whose shell cannot start fails the
 same way and leaves no pane. The host knows before it answers: the child reports a failed exec
diff --git a/src/detached/server.zig b/src/detached/server.zig
index a08a0342..676b5b95 100644
--- a/src/detached/server.zig
+++ b/src/detached/server.zig
@@ -485,9 +485,11 @@ pub const Session = struct {
     fn spawn(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void {
         const s = of(ctx);
         if (pane >= s.ptys.len) return; // the core indexes its own panes
-        s.closePty(pane);
         s.harvest();
-        if (s.ptys[pane].pid != 0) return s.core.reportError(pane, "shell", error.ShellClosing);
+        // The new shell starts before a running one goes: one that cannot
+        // start leaves the running one be (pty/ctl's exec).
+        const running = s.ptys[pane].fd >= 0;
+        if (!running and s.ptys[pane].pid != 0) return s.core.reportError(pane, "shell", error.ShellClosing);
         for (s.retired_shells) |shell| {
             if (shell.pid == 0) break;
         } else return s.core.reportError(pane, "shell", error.ShellClosing);
@@ -500,7 +502,17 @@ pub const Session = struct {
             s.core.screen_h,
             s.core.screen_w,
             s.ninep,
-        ) catch |err| return s.core.shellFailed(pane, err);
+        ) catch |err| {
+            if (running) return s.core.restartFailed(pane, err);
+            return s.core.shellFailed(pane, err);
+        };
+        s.closePty(pane);
+        s.harvest();
+        if (s.ptys[pane].pid != 0) {
+            _ = libc.close(child.file.handle);
+            host_io.retireShell(child.pid);
+            return s.core.reportError(pane, "shell", error.ShellClosing);
+        }
         const command = if (s.core.panes[pane]) |pn| pn.command != null else false;
         s.ptys[pane] = .{ .fd = child.file.handle, .pid = child.pid, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } };
         setNonblock(child.file.handle);
diff --git a/src/gui/gui.zig b/src/gui/gui.zig
index cb3bdd97..851218bb 100644
--- a/src/gui/gui.zig
+++ b/src/gui/gui.zig
@@ -4553,10 +4553,20 @@ fn gridPostPresent(ctx: ?*anyopaque) void {
 fn spawnPane(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void {
     const s = shellOf(ctx);
     s.reap();
+    // The new shell starts before a running one goes: one that cannot
+    // start leaves the running one be (pty/ctl's exec).
+    const running = s.ptys[pane] != null;
+    const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| {
+        if (running) return s.core.restartFailed(pane, err);
+        return s.core.shellFailed(pane, err);
+    };
     s.closePty(pane);
-    if (s.ptys[pane] != null) return s.core.reportError(pane, "shell", error.WorkersBusy);
+    if (s.ptys[pane] != null) {
+        _ = libc.close(child.file.handle);
+        host_io.retireShell(child.pid);
+        return s.core.reportError(pane, "shell", error.WorkersBusy);
+    }
     s.gens[pane] +%= 1;
-    const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| return s.core.shellFailed(pane, err);
     const command = s.core.panes[pane].?.command != null;
     const pt: Pty = .{ .fd = child.file.handle, .pid = child.pid, .serial = s.core.panes[pane].?.serial, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } };
     s.ptys[pane] = pt;
diff --git a/src/macos.zig b/src/macos.zig
index 9b17803b..e839816c 100644
--- a/src/macos.zig
+++ b/src/macos.zig
@@ -2437,11 +2437,17 @@ fn hostState(ctx: ?*anyopaque) *State {
 fn spawnShell(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void {
     const st = hostState(ctx);
     const core = st.core;
+    // The new shell starts before a running one goes: one that cannot
+    // start leaves the running one be (pty/ctl's exec).
+    const running = st.ptys[pane] != null;
+    const child = host_io.forkShell(core, pane, &st.prompt_rcs, core.shellBin(), cwd, core.screen_h, core.screen_w, st.ninep) catch |err| {
+        if (running) return core.restartFailed(pane, err);
+        return core.shellFailed(pane, err);
+    };
     reap(st, pane);
     st.gens[pane] +%= 1;
     const gen = st.gens[pane];
 
-    const child = host_io.forkShell(core, pane, &st.prompt_rcs, core.shellBin(), cwd, core.screen_h, core.screen_w, st.ninep) catch |err| return core.shellFailed(pane, err);
     st.ptys[pane] = .{
         .file = child.file,
         .pid = child.pid,
diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig
index 31382279..92342525 100644
--- a/src/ninep/ctl.zig
+++ b/src/ninep/ctl.zig
@@ -1996,7 +1996,7 @@ test "pty/ctl exec in a directory that is gone fails ENOENT; a shell that cannot
     try testing.expectEqualStrings("shell: access denied", p.fs.late_failure[0..p.fs.late_failure_len]);
 }
 
-test "a script whose interpreter is not there: Tty refuses it up front, only an err logged" {
+test "a script whose interpreter is not there: Tty refuses it up front, only an err logged, and pty/ctl exec keeps the running shell" {
     const p = try th.withTerm(testing.allocator);
     defer p.deinit();
     const Starting = struct {
@@ -2023,6 +2023,20 @@ test "a script whose interpreter is not there: Tty refuses it up front, only an
     try testing.expect(th.logHas(p, "Tty: interpreter /no/such/interp not found\n"));
     try testing.expect(!th.logHas(p, "\nmsg "));
     try testing.expect(!th.logHas(p, "\ndel "));
+    // The shell to start again is that script: refused before the running
+    // one goes.
+    const id = p.paneBySerial(serial).?;
+    p.panes[id].?.shell = try p.gpa.dupe(u8, try std.fmt.bufPrint(&line, "{s}/bad", .{dir}));
+    p.setCwd(id, dir);
+    const exec_refused = wr(p, Node.of(serial, .pty_ctl), "exec\n");
+    try testing.expectEqual(E.NOENT, exec_refused.errno());
+    try testing.expectEqualStrings("exec: interpreter /no/such/interp not found", exec_refused.reply.ename);
+    // One the host found could not start (the script changed after): said
+    // with the interpreter's name, and the running shell is not gone.
+    p.fs.late_failure_len = 0;
+    p.restartFailed(@intCast(id), error.InterpreterNotFound);
+    try testing.expectEqualStrings("shell: interpreter /no/such/interp not found", p.fs.late_failure[0..p.fs.late_failure_len]);
+    try testing.expect(!p.panes[id].?.shell_failed);
 }
 
 test "every EINVAL a write gets says why, in its err record too; DEL is a control character in a line" {
diff --git a/src/ninep/pty.zig b/src/ninep/pty.zig
index aa0f499a..bf5f432c 100644
--- a/src/ninep/pty.zig
+++ b/src/ninep/pty.zig
@@ -67,6 +67,14 @@ pub fn writeCtl(p: *Pardes, req: Req, id: usize) Reply {
                 const dir = pane.cwdSlice();
                 return tree.failText(req.tag, E.NOENT, std.fmt.bufPrint(&p.fs.ename, "exec: {s}: no such directory", .{dir[0..@min(dir.len, 256)]}) catch "exec: no such directory");
             };
+            // Nor is a shell that is not there, or a script whose
+            // interpreter is not: refused, and the running shell kept.
+            if (comptime pardes.hosted) if (!apply and std.mem.eql(u8, line, "exec")) if (p.panes[id]) |pane| {
+                const bin = pane.shell orelse p.shellBin();
+                var why: [320]u8 = undefined;
+                if (@import("../host_io.zig").Shell.refusal(bin[0..@min(bin.len, 200)], &why)) |refused|
+                    return tree.failText(req.tag, E.NOENT, std.fmt.bufPrint(&p.fs.ename, "exec: {s}", .{refused}) catch "exec: no such shell");
+            };
             if (!verb(p, id, line, apply)) return tree.failText(req.tag, E.INVAL, if (outOfRange(line)) e_winsize_range else e_bad_pty_ctl);
         }
     }
diff --git a/src/pardes.zig b/src/pardes.zig
index d2fd795b..76ec2176 100644
--- a/src/pardes.zig
+++ b/src/pardes.zig
@@ -5353,6 +5353,13 @@ pub const Pardes = struct {
         }
     }
 
+    /// A shell started again in a terminal whose shell runs (pty/ctl's
+    /// exec) did not start: the host kept the running one, so only said.
+    pub fn restartFailed(p: *Pardes, id: u8, err: anyerror) void {
+        if (p.panes[id] == null) return;
+        p.sayShellFailure(id, err);
+    }
+
     fn sayShellFailure(p: *Pardes, id: u8, err: anyerror) void {
         const pane = p.panes[id] orelse return;
         if (err == error.FileNotFound or err == error.NotDir) {
diff --git a/src/tty/tty.zig b/src/tty/tty.zig
index 3c8c90bc..a6f6bdfc 100644
--- a/src/tty/tty.zig
+++ b/src/tty/tty.zig
@@ -1253,9 +1253,15 @@ const Shell = struct {
 
     fn spawn(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void {
         const s = of(ctx);
-        closePty(ctx, pane); // a shell still in the slot goes first, reaped
+        // The new shell starts before a running one goes: one that cannot
+        // start leaves the running one be (pty/ctl's exec).
+        const running = s.ptys[pane] != null;
+        const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| {
+            if (running) return s.core.restartFailed(pane, err);
+            return s.core.shellFailed(pane, err);
+        };
+        closePty(ctx, pane); // a shell still in the slot goes, reaped
         s.gens[pane] +%= 1;
-        const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| return s.core.shellFailed(pane, err);
         const command = if (s.core.panes[pane]) |pn| pn.command != null else false;
         s.ptys[pane] = .{ .file = child.file, .pid = child.pid, .reader = .{ .any_future = null, .result = {} }, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } };
         var lbuf: [pardes.memory.limits.host_path_cap + 1]u8 = undefined;
diff --git a/test/fs.py b/test/fs.py
index 977b3e45..6edfeb0f 100644
--- a/test/fs.py
+++ b/test/fs.py
@@ -442,6 +442,27 @@ def run_file(binary):
             assert run(client, term, b'true\n') == b'exit 0\n'
             assert client.read(f'/pane/{term}/pty/ctl').split()[2] == b'2', client.read(f'/pane/{term}/pty/ctl')
             client.write(f'/pane/{term}/pty/ctl', b'winsize 80 24\n')
+            # A shell started again that cannot start (its directory is
+            # there but cannot be entered) fails the write and leaves the
+            # running shell be: runs still work.
+            locked = root / 'locked'
+            locked.mkdir()
+            assert run(client, term, f'cd {locked}\n'.encode()) == b'exit 0\n'
+            deadline = time.monotonic() + 5
+            while str(locked).encode() not in client.read('/index'):
+                assert time.monotonic() < deadline, client.read('/index')
+                time.sleep(.05)
+            locked.chmod(0o600)
+            try:
+                client.write(f'/pane/{term}/pty/ctl', b'exec\n')
+                raise AssertionError('pty/ctl exec of a shell that cannot start was taken')
+            except OSError as refused:
+                assert 'shell' in str(refused), refused
+            finally:
+                locked.chmod(0o755)
+            kept = run(client, term, b'true\n')
+            assert kept == b'exit 0\n', (kept, client.read('/log')[-600:])
+            assert run(client, term, f'cd {root}\n'.encode()) == b'exit 0\n'
             with Client(address) as other:
                 slow = []
                 waiter = threading.Thread(target=lambda: slow.append(run(other, term, b'sleep 1\n')), daemon=True)
@@ -522,20 +543,15 @@ def new_terminals_named_once(binary):
             bad.write_bytes(b'#!/nonexistent/interp\n')
             bad.chmod(0o755)
             panes = client.read('/index')
-            try:
-                client.write('/pane/1/ctl', f'Tty {bad}\n'.encode())
-                raise AssertionError('Tty of a shell that cannot start was taken')
-            except OSError as refused:
-                assert 'shell' in str(refused), refused
+            for where, line in (('/pane/1/ctl', f'Tty {bad}\n'), ('/ctl', f'Shell {bad}\n')):
+                try:
+                    client.write(where, line.encode())
+                    raise AssertionError(f'{line} of a shell that cannot start was taken')
+                except OSError as refused:
+                    assert 'interpreter /nonexistent/interp not found' in str(refused), refused
             time.sleep(.3)
             assert client.read('/index').count(b'\n') == panes.count(b'\n'), client.read('/index')
-            client.write('/ctl', f'Shell {bad}\n'.encode())
-            try:
-                client.write(f'/pane/{made[0]}/pty/ctl', b'exec\n')
-                raise AssertionError('pty/ctl exec of a shell that cannot start was taken')
-            except OSError as refused:
-                assert 'shell' in str(refused), refused
-            client.write('/ctl', b'Shell\n')
+            assert b'shell: shell not found' not in client.read('/log'), client.read('/log')
             assert str(made[0]).encode() in b' '.join(r.split()[0] for r in client.read('/index').splitlines())