Close the pty write-after-respawn race and bound the pty PATH resolve stall

examples/terminal/src/main.zig

@@ -230,7 +230,7 @@ fn spawnShell(model: *Model, fx: *Fx) void {
     // session that just ended. (A no-op on the first spawn.)
     model.session.reset();
     model.session.refreshScreenText();
-    fx.ptySpawn(.{
+    _ = fx.ptySpawn(.{
         .key = shell_key,
         .argv = default_shell_argv,
         .cols = model.cols,

examples/workbench/src/main.zig

@@ -135,7 +135,7 @@ pub fn boot(model: *Model, fx: *Effects) void {
         model.history_index = 0;
         model.address_field.set(home_url);
     }
-    fx.ptySpawn(.{
+    _ = fx.ptySpawn(.{
         .key = shell_effect_key,
         .argv = default_shell_argv,
         .cols = 80,

src/runtime/effects.zig

@@ -2911,6 +2911,20 @@ pub fn Effects(comptime Msg: type) type {
             on_event: PtyMsgFn,
         };
 
+        /// A versioned reference to one `ptySpawn` occupancy, returned
+        /// by `ptySpawn` and accepted by `ptyWriteChecked`. `key` alone
+        /// is reusable (a later spawn may claim the same key once this
+        /// occupancy's exit has delivered and its slot retired);
+        /// `generation` is stamped fresh on every spawn and is what
+        /// `ptyWriteChecked` actually pins a write to. Prefer this over
+        /// the bare-key `ptyWrite` whenever a write could otherwise
+        /// race a respawn under the same key (see `ptyWrite`'s
+        /// key-reuse-misdelivery note).
+        pub const PtyHandle = struct {
+            key: u64,
+            generation: u64,
+        };
+
         /// A recorded fake-pty spawn request, exposed for test
         /// assertions (`pendingPtyAt`). Strings borrow the slot's
         /// storage — valid until the slot retires.
@@ -6840,10 +6854,11 @@ pub fn Effects(comptime Msg: type) type {
         /// per-drain Msgs (and journal records), never per-read ones.
         /// Under session replay nothing spawns — the journaled batches
         /// and exit ARE the session (`feedPtyOutput`/`feedPtyExit`).
-        pub fn ptySpawn(self: *Self, options: PtySpawnOptions) void {
+        pub fn ptySpawn(self: *Self, options: PtySpawnOptions) ?PtyHandle {
             self.reclaimSlots();
             if (options.argv.len == 0 or options.argv.len > max_effect_argv) {
-                return self.rejectPty(options.key, options.on_event, true);
+                self.rejectPty(options.key, options.on_event, true);
+                return null;
             }
             var total_bytes: usize = 0;
             for (options.argv) |arg| {
@@ -6853,26 +6868,46 @@ pub fn Effects(comptime Msg: type) type {
                 // a cut argument (validated here so the fake executor
                 // and replay refuse identically, before the real
                 // transport's own guard).
-                if (std.mem.indexOfScalar(u8, arg, 0) != null) return self.rejectPty(options.key, options.on_event, true);
+                if (std.mem.indexOfScalar(u8, arg, 0) != null) {
+                    self.rejectPty(options.key, options.on_event, true);
+                    return null;
+                }
+            }
+            if (total_bytes > max_effect_argv_bytes) {
+                self.rejectPty(options.key, options.on_event, true);
+                return null;
+            }
+            if (options.cols == 0 or options.rows == 0) {
+                self.rejectPty(options.key, options.on_event, true);
+                return null;
             }
-            if (total_bytes > max_effect_argv_bytes) return self.rejectPty(options.key, options.on_event, true);
-            if (options.cols == 0 or options.rows == 0) return self.rejectPty(options.key, options.on_event, true);
             if (options.term.len == 0 or options.term.len > max_effect_pty_term_bytes) {
-                return self.rejectPty(options.key, options.on_event, true);
+                self.rejectPty(options.key, options.on_event, true);
+                return null;
             }
             // TERM rides the child environment; an embedded NUL would
             // truncate it at the C boundary, so a spawn that "succeeded"
             // would hand the child a different TERM than requested.
-            if (std.mem.indexOfScalar(u8, options.term, 0) != null) return self.rejectPty(options.key, options.on_event, true);
-            if (self.keyOccupiedUntilDelivery(options.key)) return self.rejectPty(options.key, options.on_event, true);
-            const slot_index = self.findIdlePtySlot() orelse return self.rejectPty(options.key, options.on_event, true);
+            if (std.mem.indexOfScalar(u8, options.term, 0) != null) {
+                self.rejectPty(options.key, options.on_event, true);
+                return null;
+            }
+            if (self.keyOccupiedUntilDelivery(options.key)) {
+                self.rejectPty(options.key, options.on_event, true);
+                return null;
+            }
+            const slot_index = self.findIdlePtySlot() orelse {
+                self.rejectPty(options.key, options.on_event, true);
+                return null;
+            };
             // Table capacity obeys the replay-hold invariant, the
             // channel argument verbatim: executor-truth start failures
             // park a real slot under replay until the journaled
             // terminal feeds, so live holds the open counted through
             // the staged window too.
             if (self.idlePtySlotCount() <= self.stagedPtyReservationCount()) {
-                return self.rejectPty(options.key, options.on_event, true);
+                self.rejectPty(options.key, options.on_event, true);
+                return null;
             }
             // A build without a pty transport (a libc-free Linux
             // build; a target outside the support matrix) refuses up
@@ -6887,7 +6922,10 @@ pub fn Effects(comptime Msg: type) type {
             // spawn), instead of the replayed spawn parking with no
             // journaled terminal to retire it.
             if (comptime !pty_transport.supported) {
-                if (self.executor != .fake) return self.rejectPty(options.key, options.on_event, false);
+                if (self.executor != .fake) {
+                    self.rejectPty(options.key, options.on_event, false);
+                    return null;
+                }
             }
 
             const slot = &self.pty_slots[slot_index];
@@ -6911,6 +6949,7 @@ pub fn Effects(comptime Msg: type) type {
             }
             @memcpy(slot.term_storage[0..options.term.len], options.term);
             slot.term_len = options.term.len;
+            const handle: PtyHandle = .{ .key = options.key, .generation = generation };
 
             if (slot.fake) {
                 // Session replay: PARK — the fake-slot discipline. The
@@ -6922,9 +6961,16 @@ pub fn Effects(comptime Msg: type) type {
                     slot.park_seq = self.nextPendingSeq();
                     slot.park_state = .reserved;
                 }
-                return;
+                return handle;
             }
             self.startRealPty(slot, options);
+            // `startRealPty` may fail synchronously and release the slot
+            // (staging `.spawn_failed` instead) — the returned handle is
+            // still correct either way: `ptyWriteChecked` re-validates
+            // the generation against whatever currently occupies `key`
+            // at write time, so a handle for an already-released slot
+            // simply reads back as "unknown" rather than misdelivering.
+            return handle;
         }
 
         /// Write bytes toward the pty child's stdin, all-or-nothing.
@@ -6950,6 +6996,43 @@ pub fn Effects(comptime Msg: type) type {
         /// exact: a verdict is journaled (and consumed) iff the key names
         /// a live occupancy and the payload is non-empty — both
         /// deterministic across the replayed dispatch stream.
+        // NOTE(key-reuse misdelivery): `findPtySlot` below matches purely
+        // on the bare `u64` key. Once a key's slot is retired
+        // (`retirePtySlot`, state -> `.idle`) it becomes eligible for
+        // reuse by a *new* `ptySpawn` under the same key, so a `ptyWrite`
+        // intended for the OLD occupant but issued after a NEW occupant
+        // has already spawned under that same key silently lands on the
+        // new occupant's stdin instead of being dropped.
+        //
+        // `ts_core_host.zig`'s `issuePtySpawn`/`runPtyWrite` owns both the
+        // spawn and the write for its keys, so it now stamps and reuses
+        // `PtyHandle.generation` via `ptyWriteChecked` below — closing the
+        // gap outright rather than just relying on its `entry.used`
+        // liveness gate.
+        //
+        // `ui_app.zig`'s `terminalGatewayWrite` (the built-in `<terminal>`
+        // element's write path, via `terminal_session.zig`'s
+        // `TerminalSessions`) still uses this bare-key form. That module
+        // does not own the spawn — the embedding app calls `ptySpawn` and
+        // merely binds the resulting key to a `<terminal>` element, and
+        // deliberately keeps the SAME session object across a respawn
+        // under the same key (`notePtyEvent`'s `.output` handler calls
+        // `resetForRespawn` rather than allocating a new session) — so
+        // adopting `ptyWriteChecked` there would mean threading a
+        // generation through the public `EffectPtyEvent` contract
+        // `session_journal.zig`/`session_replay.zig` serialize, a wire-
+        // format change well beyond this fix's scope. It remains safe
+        // under the same single-threaded-dispatch argument verified for
+        // `ts_core_host.zig`, via a different mechanism: `keyEvent`/
+        // `textInput` (the only two paths that call `gateway.write`) both
+        // gate on `Session.acceptsInput()` (`!session.ended`), and
+        // `session.ended` is set at the same exit-delivery instant that
+        // frees the key for reuse, only clearing again once the new
+        // spawn's first output batch arrives — so no write can reach the
+        // gateway for a key between an old occupant's exit and a new
+        // occupant's first output. New callers that own both the spawn
+        // and the write should prefer `ptyWriteChecked` over this
+        // bare-key form.
         pub fn ptyWrite(self: *Self, key: u64, bytes: []const u8) bool {
             const slot = self.findPtySlot(key) orelse {
                 // A spawn whose transport failed SYNCHRONOUSLY released
@@ -6985,6 +7068,26 @@ pub fn Effects(comptime Msg: type) type {
             return accepted;
         }
 
+        /// `ptyWrite`, but pinned to the exact occupancy `handle` names
+        /// instead of whatever currently holds `handle.key`. Closes the
+        /// key-reuse-misdelivery gap `ptyWrite` documents: if `handle`'s
+        /// occupancy has already retired and the key was reused by a
+        /// newer `ptySpawn`, `handle.generation` no longer matches the
+        /// slot's current generation, so the write is refused exactly
+        /// like an unknown key rather than landing on the new occupant.
+        /// The mismatch case is deliberately NOT counted into
+        /// `dropped_writes` (that tally belongs to the CURRENT occupant,
+        /// which never saw this write) and is not journaled — from
+        /// `handle`'s perspective this occupancy is simply gone, the same
+        /// silent-refuse contract `ptyWrite` already gives a key with no
+        /// spawn at all.
+        pub fn ptyWriteChecked(self: *Self, handle: PtyHandle, bytes: []const u8) bool {
+            if (self.findPtySlot(handle.key)) |slot| {
+                if (slot.generation != handle.generation) return false;
+            }
+            return self.ptyWrite(handle.key, bytes);
+        }
+
         /// The admission decision behind `ptyWrite` — the live half the
         /// journaled verdict records.
         fn ptyWriteAdmit(self: *Self, slot: *PtySlot, bytes: []const u8) bool {

src/runtime/effects_host_tests.zig

@@ -6,6 +6,13 @@
 //! pin: a session driving requests and fx timers through a full UiApp
 //! journals `.host` results and replays to identical state without a
 //! host call.
+//!
+//! The real-executor cases include the genuinely async path
+//! `HostCallBinding`'s own doc comment calls out: `request_fn` returns
+//! without answering, and `feedHostResult` arrives LATER from a separate,
+//! host-marshaled call (outside `request_fn`'s own call stack) — plus
+//! that path racing a `cancel_fn` notice, mirroring the fake executor's
+//! "cancelHostRequest drops silently" coverage for the real binding.
 
 const std = @import("std");
 const geometry = @import("geometry");
@@ -450,6 +457,80 @@ test "real-mode host calls ride the binding and answer through the feed" {
     try std.testing.expectEqual(@as(u32, 1), h.app_state.model.result_count);
 }
 
+test "real-mode: request_fn defers the answer, which arrives later and can race a cancel" {
+    var h = try Harness.create();
+    defer h.destroy();
+    const fx = &h.app_state.effects;
+
+    const Stub = struct {
+        var bound: ?*HostEffects = null;
+        var deferred_key: ?u64 = null;
+        var cancel_count: usize = 0;
+        var last_cancelled: u64 = 0;
+        fn send(context: *anyopaque, name: []const u8, payload: []const u8) void {
+            _ = context;
+            _ = name;
+            _ = payload;
+        }
+        // Neither branch answers inline: both requests stay in flight
+        // until the test itself feeds a result from outside this
+        // function's call stack, standing in for a host that marshals
+        // the answer back through its own event loop instead of
+        // resolving synchronously.
+        fn request(context: *anyopaque, name: []const u8, key: u64, payload: []const u8) void {
+            _ = context;
+            _ = name;
+            _ = payload;
+            deferred_key = key;
+        }
+        fn cancelNotice(context: *anyopaque, key: u64) void {
+            _ = context;
+            cancel_count += 1;
+            last_cancelled = key;
+        }
+    };
+    Stub.bound = fx;
+    Stub.deferred_key = null;
+    Stub.cancel_count = 0;
+    var context: u8 = 0;
+    fx.bindHostCalls(.{
+        .context = &context,
+        .send_fn = Stub.send,
+        .request_fn = Stub.request,
+        .cancel_fn = Stub.cancelNotice,
+    });
+
+    // The request returns with no answer yet: nothing has delivered.
+    test_payload = "hello";
+    try h.app_state.dispatch(&h.harness.runtime, 1, .ask);
+    try h.wake();
+    try std.testing.expectEqual(ask_key, Stub.deferred_key.?);
+    try std.testing.expectEqual(@as(u32, 0), h.app_state.model.result_count);
+
+    // The host answers later, off `request_fn`'s own call stack: the
+    // deferred result still delivers on the next wake.
+    try fx.feedHostResult(ask_key, true, "deferred-answer");
+    try h.wake();
+    try std.testing.expectEqual(@as(u32, 1), h.app_state.model.result_count);
+    try std.testing.expectEqual(@as(u32, 1), h.app_state.model.ok_count);
+    try std.testing.expectEqualStrings("deferred-answer", h.app_state.model.bytesPrefix());
+
+    // A second deferred request races a cancel: the host is notified,
+    // and a late answer that arrives after the cancel notice reports
+    // `error.EffectNotFound` for the real executor too (the fake
+    // executor's equivalent is pinned by "cancelHostRequest drops
+    // silently, and the generic cancel routes to it" above).
+    try h.app_state.dispatch(&h.harness.runtime, 1, .ask_other);
+    try h.wake();
+    try std.testing.expectEqual(other_key, Stub.deferred_key.?);
+    fx.cancelHostRequest(other_key);
+    try h.wake();
+    try std.testing.expectEqual(@as(usize, 1), Stub.cancel_count);
+    try std.testing.expectEqual(other_key, Stub.last_cancelled);
+    try std.testing.expectError(error.EffectNotFound, fx.feedHostResult(other_key, true, "too-late"));
+    try std.testing.expectEqual(@as(u32, 1), h.app_state.model.result_count);
+}
+
 test "real-mode requests without bound host services reject through the err route" {
     var h = try Harness.create();
     defer h.destroy();