Simplify graceful worker shutdown (#71)

Close worker stdin as the sole graceful shutdown request. Remove interpreter shutdown snippets from worker-ready advertising and drop the server-to-worker sideband shutdown command. Add regression coverage and ADR documentation for the simplified lifecycle contract.

Author: t-kalinowski

docs/adr/0001-stdin-close-graceful-shutdown.md

@@ -0,0 +1,56 @@
+# ADR 0001: Use stdin close for graceful worker shutdown
+
+## Status
+
+Accepted
+
+## Date
+
+2026-05-19
+
+## Context
+
+The server previously allowed workers to advertise interpreter-specific
+graceful shutdown text through `worker_ready.graceful_shutdown.stdin`, such as
+R `quit("no")` or Python `exit()`. During reset or shutdown, the server could
+write that text to worker stdin as the graceful request.
+
+That design had three problems:
+
+- It created a race between shutdown text sent on stdin and `session_end`
+  notifications sent on sideband.
+- It allowed user-level input operations, such as R `readline()` or Python
+  `input()`, to consume the graceful shutdown request as ordinary user input.
+- It made the lifecycle contract harder to explain, reason about, and
+  communicate to a model or human because graceful shutdown used a mixture of
+  stdin code, sideband notification, and OS escalation.
+
+## Decision
+
+Graceful worker shutdown is requested only by closing the worker stdin
+transport. The server must not send interpreter shutdown code over stdin and
+must not add or use a sideband shutdown command to deliver graceful shutdown.
+
+Workers must no longer advertise `worker_ready.graceful_shutdown.stdin`.
+Built-in workers do not advertise `quit("no")`, `exit()`, or equivalent
+shutdown text.
+
+On reset, respawn, or server shutdown, the server closes worker stdin and waits
+for the worker to exit naturally within the existing shutdown timeout. If the
+worker is busy, ignores EOF, or otherwise does not exit, the existing
+SIGTERM/SIGKILL or platform-equivalent escalation remains the fallback.
+
+Worker-to-server `session_end` remains a notification that the worker session is
+ending. It is not a shutdown request from the server to the worker.
+
+## Consequences
+
+- There is no race between stdin shutdown code and sideband shutdown delivery.
+- A blocked user prompt cannot receive interpreter shutdown text as an answer.
+- Idle interpreters can still exit normally on EOF and run normal finalization
+  where supported.
+- Graceful finalization is best effort. User code can handle EOF and continue,
+  or remain busy, in which case OS escalation terminates the worker.
+- The server/worker lifecycle is simpler: stdin carries user input and EOF,
+  sideband carries facts observed by the worker, and process control remains
+  server-owned.

docs/index.md

@@ -12,6 +12,8 @@ checked-in execution plans without relying on stale notes.
 - `docs/debugging.md`: debug logs, `--debug-repl`, and wire tracing.
 - `docs/sandbox.md`: sandbox modes, writable roots, and Codex per-tool-call sandbox metadata.
 - `docs/worker_sideband_protocol.md`: server/worker IPC contract.
+- `docs/adr/0001-stdin-close-graceful-shutdown.md`: accepted decision that
+  graceful worker shutdown is requested only by closing worker stdin.
 - `docs/plans/AGENTS.md`: when to write a checked-in execution plan and where it lives.
 - `docs/plans/completed/codex-sandbox-state-meta-migration.md`: completed plan for migrating Codex `--sandbox inherit` from async updates to per-tool-call sandbox metadata.
 
@@ -23,6 +25,7 @@ checked-in execution plans without relying on stale notes.
 - `docs/tool-descriptions/repl_tool_python.md`: Python `repl` behavior for the files-mode oversized-output path.
 - `docs/tool-descriptions/repl_tool_python_pager.md`: Python `repl` behavior for pager mode.
 - `docs/tool-descriptions/repl_reset_tool.md`: `repl_reset` behavior.
+- `docs/adr/`: accepted architecture decision records.
 - `README.md`: user-facing overview and installation guide. Treat it as product documentation, not the engineering source of truth.
 
 ## Exploratory Docs

docs/plans/active/worker-server-protocol-zod.md

@@ -243,9 +243,6 @@ The first worker-to-server message must be:
   "worker": { "name": "zod", "version": "0.1.0" },
   "capabilities": {
     "images": false
-  },
-  "graceful_shutdown": {
-    "stdin": "quit()\n"
   }
 }
 ```
@@ -259,20 +256,9 @@ interpreter-specific steady-state code paths.
 The `worker.name` field is descriptive. It is useful for logs and
 diagnostics, but server request handling must not switch on it.
 
-`graceful_shutdown` is optional. If present, it contains the exact stdin text
-the server may write as a graceful shutdown/reset request when server-owned
-shutdown rules say a stdin request is appropriate. For example, R might ask for
-`quit("no")\n`.
-
 Handshake field notes:
 
 - `capabilities.images`: whether the worker emits `output_image` events.
-- `graceful_shutdown.stdin`: exact text the worker wants sent on stdin
-  when the server asks it to exit gracefully. This is not base64-encoded
-  because it is a small human-readable interpreter command. The worker
-  controls only this text; it does not control whether the command is
-  sent, when interrupts are sent, how long the server waits, or whether
-  OS termination follows.
 
 Raw stdout/stderr capture is not a negotiated capability. The server
 will always capture raw worker stdout/stderr as unowned fallback output,
@@ -520,18 +506,19 @@ delivering OS controls. If a worker exists, the server delivers the
 control. If no worker exists, the server must not spawn a worker solely
 to interrupt or shut down nothing.
 
-Workers cannot opt out of OS controls. The handshake may describe a
-graceful stdin request for shutdown/reset, but OS escalation is common
-across all workers and remains server-owned.
+Workers cannot opt out of OS controls. Graceful shutdown is a server-owned
+stdin close followed by a bounded wait; OS escalation is common across all
+workers and remains server-owned.
 
 For reset and shutdown, if the server considers the worker busy, it
 sends an interrupt first. This is not configurable by the worker. After
 the interrupt, the server waits for the normal bounded server-owned
 grace period for the worker to return to an unsatisfied `readline_start`
-or end the session. If the worker becomes idle and provided a
-`graceful_shutdown.stdin` command, the server may write that command to
-stdin. If the worker does not exit within the server-owned graceful
-shutdown timeout, the server escalates to OS termination.
+or end the session. The server then closes worker stdin and waits for
+natural exit. If the worker does not exit within the server-owned graceful
+shutdown timeout, the server escalates to OS termination. The server must not
+write interpreter shutdown code to stdin and must not use sideband shutdown
+commands to deliver graceful shutdown.
 
 ### Interrupt
 
@@ -586,11 +573,9 @@ Reset means end the current worker session and start a fresh one for the
 next tool call.
 
 If the worker is busy, the server interrupts first. If the worker is
-then known to be waiting at an unsatisfied `readline_start`, and the
-handshake provided a `graceful_shutdown.stdin` command, the server may
-write that command to stdin. If the worker does not exit within the
-server-owned timeout, the server escalates to OS termination. The worker
-controls only the graceful command text.
+then known to be waiting at an unsatisfied `readline_start`, the server closes
+worker stdin and waits for natural exit. If the worker does not exit within
+the server-owned timeout, the server escalates to OS termination.
 
 If a leading reset prefix has a non-empty tail, the server writes the
 tail only after reset has completed and the replacement worker has
@@ -806,11 +791,10 @@ surface with Zod as the worker:
 - Interrupt completion is driven by worker events, not prompt parsing.
 - Worker-owned stderr preserves ordering relative to stdout.
 - Raw stdout/stderr never affects prompt or completion state.
-- Reset interrupts first when the worker is busy, then uses graceful
-  stdin shutdown only after an unsatisfied `readline_start`, then
+- Reset interrupts first when the worker is busy, then closes worker stdin and
   escalates to OS termination if needed.
-- Shutdown follows the same interrupt-then-graceful-stdin-then-OS rule
-  and cannot be opted out of.
+- Shutdown follows the same interrupt-then-stdin-close-then-OS rule and cannot
+  be opted out of.
 - Protocol errors fail fast and do not leave the server in a shadow
   interpreter state.
 

docs/worker_sideband_protocol.md

@@ -26,6 +26,11 @@ Runtime stdin transport is a launch-time worker setting, not a sideband
 negotiation. A worker may use ordinary pipes or a PTY for its C stdio, but the
 server still writes accepted request bytes to worker stdin and relies on
 sideband events for prompt, input, discard, output, and session facts.
+For graceful reset and shutdown, the server closes the worker stdin transport
+and then waits for normal worker exit before escalating to OS termination.
+Workers must not advertise interpreter-specific shutdown text, and the server
+does not send shutdown code or a sideband shutdown command. See
+`docs/adr/0001-stdin-close-graceful-shutdown.md`.
 
 Built-in Unix Python uses PTY-backed C stdin/stdout/stderr so CPython calls
 `PyOS_ReadlineFunctionPointer`. The Python callback emits readline accounting
@@ -42,28 +47,18 @@ facts from that CPython path. Sideband IPC stays separate from the PTY.
 - The worker may emit `readline_discard` for exact active-turn stdin bytes it
   discarded before delivering them to the runtime.
 
-`session_end`
-- `{ "type": "session_end" }`
-- Sent when the server is ending the current session, for example during reset
-  or shutdown.
-- Worker treats this as shutdown intent and stops consuming further stdin
-  payloads.
-
 ## Direction: worker -> server
 
 Worker-to-server messages are strict: unknown fields, invalid enum values,
 invalid base64, and unknown message types are protocol errors.
 
 `worker_ready`
-- `{ "type": "worker_ready", "protocol": { "name": "mcp-repl-worker", "version": 1 }, "worker": { "name": <string>, "version": <string> }, "capabilities": { "images": <bool> }, "graceful_shutdown": { "stdin": <string> } }`
+- `{ "type": "worker_ready", "protocol": { "name": "mcp-repl-worker", "version": 1 }, "worker": { "name": <string>, "version": <string> }, "capabilities": { "images": <bool> } }`
 - Must be the first worker-to-server message for protocol workers.
 - The server rejects unsupported protocol names or versions before sending user
   input.
 - `worker.name` is diagnostic metadata. Server request handling must not branch
   on it.
-- `graceful_shutdown` is optional. If present, the server may write that exact
-  stdin text only when server-owned shutdown/reset rules decide a graceful
-  stdin request is appropriate.
 
 `readline_start`
 - `{ "type": "readline_start", "prompt": <string> }`

src/ipc.rs

@@ -102,7 +102,6 @@ pub enum ServerToWorkerIpcMessage {
         request_generation: u64,
     },
     Interrupt,
-    SessionEnd,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -112,8 +111,6 @@ pub enum WorkerToServerIpcMessage {
         protocol: WorkerProtocol,
         worker: WorkerIdentity,
         capabilities: WorkerCapabilities,
-        #[serde(default)]
-        graceful_shutdown: Option<WorkerGracefulShutdown>,
     },
     BackendInfo {
         #[serde(default)]
@@ -182,12 +179,6 @@ pub struct WorkerCapabilities {
     pub images: bool,
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
-#[serde(deny_unknown_fields)]
-pub struct WorkerGracefulShutdown {
-    pub stdin: String,
-}
-
 #[derive(Default)]
 struct ServerIpcInbox {
     queue: VecDeque<WorkerToServerIpcMessage>,
@@ -1750,11 +1741,7 @@ pub fn emit_plot_image(mime_type: &str, data: &str, is_update: bool, source: Opt
     }
 }
 
-pub fn emit_worker_ready(
-    worker_name: &str,
-    supports_images: bool,
-    graceful_shutdown_stdin: Option<&str>,
-) {
+pub fn emit_worker_ready(worker_name: &str, supports_images: bool) {
     if let Some(ipc) = global_ipc() {
         let _ = ipc.send(WorkerToServerIpcMessage::WorkerReady {
             protocol: WorkerProtocol {
@@ -1768,9 +1755,6 @@ pub fn emit_worker_ready(
             capabilities: WorkerCapabilities {
                 images: supports_images,
             },
-            graceful_shutdown: graceful_shutdown_stdin.map(|stdin| WorkerGracefulShutdown {
-                stdin: stdin.to_string(),
-            }),
         });
     }
 }

src/python_session.rs

@@ -171,16 +171,6 @@ struct PythonRuntime {
     stdin: *mut libc::FILE,
 }
 
-pub fn request_shutdown() -> bool {
-    let Some(state) = SESSION_STATE.get() else {
-        return false;
-    };
-    let mut guard = state.inner.lock().unwrap();
-    guard.shutdown = true;
-    state.cvar.notify_all();
-    true
-}
-
 fn request_exit() {
     let Some(state) = SESSION_STATE.get() else {
         return;
@@ -631,7 +621,7 @@ fn run_session_on_current_thread(init: Arc<SessionInit>) -> Result<(), String> {
     }
 
     init.mark_ready();
-    ipc::emit_worker_ready("python", plot_capable(), Some("exit()\n"));
+    ipc::emit_worker_ready("python", plot_capable());
 
     let result = run_repl(&runtime);
     let finalize_result = finalize_python(api, thread_state);

src/python_worker.rs

@@ -11,14 +11,10 @@ use crate::python_session::{self, PythonSession};
 
 struct WorkerState {
     busy: AtomicBool,
-    shutting_down: AtomicBool,
 }
 
 impl WorkerState {
     fn try_mark_busy(&self) -> bool {
-        if self.is_shutting_down() {
-            return false;
-        }
         self.busy
             .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
             .is_ok()
@@ -27,21 +23,12 @@ impl WorkerState {
     fn mark_idle(&self) {
         self.busy.store(false, Ordering::SeqCst);
     }
-
-    fn begin_shutdown(&self) {
-        self.shutting_down.store(true, Ordering::SeqCst);
-    }
-
-    fn is_shutting_down(&self) -> bool {
-        self.shutting_down.load(Ordering::SeqCst)
-    }
 }
 
 impl Default for WorkerState {
     fn default() -> Self {
         Self {
             busy: AtomicBool::new(false),
-            shutting_down: AtomicBool::new(false),
         }
     }
 }
@@ -128,10 +115,6 @@ fn init_ipc(
                         python_session::interrupt_request_generation(request_generation);
                         emit_python_interrupt_ack();
                     }
-                    Some(ServerToWorkerIpcMessage::SessionEnd) => {
-                        state.begin_shutdown();
-                        let _ = python_session::request_shutdown();
-                    }
                     None => {
                         std::process::exit(0);
                     }
@@ -163,10 +146,6 @@ fn handle_write_stdin(
     state: Arc<WorkerState>,
     request_tx: &mpsc::SyncSender<QueuedRequest>,
 ) {
-    if state.is_shutting_down() {
-        return;
-    }
-
     if !state.try_mark_busy() {
         emit_stderr_message("worker is busy; request already running");
         return;

src/worker.rs

@@ -47,11 +47,11 @@ pub fn run() -> Result<(), Box<dyn std::error::Error>> {
 fn run_r_worker() -> Result<(), Box<dyn std::error::Error>> {
     crate::diagnostics::startup_log("worker: run begin");
     let state = Arc::new(WorkerState::default());
-    init_ipc(state.clone()).map_err(|err| {
+    init_ipc().map_err(|err| {
         eprintln!("worker ipc init error: {err}");
         err
     })?;
-    emit_worker_ready("r", true, Some("quit(\"no\")\n"));
+    emit_worker_ready("r", true);
 
     let stdin_state = state.clone();
     let _stdin_thread = thread::Builder::new()
@@ -82,7 +82,7 @@ fn wait_for_r_session() -> Result<&'static RSession, String> {
     }
 }
 
-fn init_ipc(state: Arc<WorkerState>) -> Result<(), Box<dyn std::error::Error>> {
+fn init_ipc() -> Result<(), Box<dyn std::error::Error>> {
     let conn = connect_from_env(Duration::from_secs(2))?;
     set_global_ipc(conn.clone());
     if let Err(err) = thread::Builder::new()
@@ -100,11 +100,6 @@ fn init_ipc(state: Arc<WorkerState>) -> Result<(), Box<dyn std::error::Error>> {
                     Some(ServerToWorkerIpcMessage::PythonInterrupt { .. }) => {
                         crate::r_session::clear_pending_input();
                     }
-                    Some(ServerToWorkerIpcMessage::SessionEnd) => {
-                        state.begin_shutdown();
-                        crate::r_session::clear_pending_input();
-                        let _ = crate::r_session::request_shutdown();
-                    }
                     None => {
                         // Without IPC, the worker cannot participate in turn accounting (prompt,
                         // request boundaries, etc). Exit immediately so the server can respawn.