diff --git a/README.md b/README.md index 9b66b73b..2fd94fe4 100644 --- a/README.md +++ b/README.md @@ -497,6 +497,13 @@ A legacy record that already has an exit while its daemon is still finishing is refused the same way, before any restart prompt: a restart waits for proof that the daemon is gone, and a live legacy daemon cannot give it. +**A dead session restarts only on an answer.** `attach` on a session that has +exited shows its last lines and asks `Restart? [Y/n]`. At a terminal, Enter +restarts it. Without one, only `y` or `yes` does. At EOF, or on any other +input, attach exits 1 and the stored command does not run. `-r` restarts +without asking, `--no-restart` never restarts, and `--force` answers nothing. +The Node tool differs here ([decision 0020](docs/decisions/0020-an-unanswered-restart-prompt-does-not-restart.md)). + `run` and `restart` are different and deliberately so: from inside a session `run` runs the command directly and `restart` restarts without attaching. Both did what was asked, so both exit 0. diff --git a/crates/pty-conformance/divergences.toml b/crates/pty-conformance/divergences.toml index e4f57e13..d0a3371d 100644 --- a/crates/pty-conformance/divergences.toml +++ b/crates/pty-conformance/divergences.toml @@ -297,3 +297,40 @@ f005432ad5c73b79abd7c5eaa8a0d6eb66ee9194 on Node 22 exceeded the five-second \ harness deadline in CI run 40986814385. Remove when Node honors the quiet \ session-first timeout. """ + +[[divergence]] +test = "attach_no_restart::a_restart_prompt_at_eof_declines" +side = "node" +kind = "intended" +reason = """ +Decision 0020: attach's dead-session prompt at EOF declines, exits 1 and \ +names -r. Pinned Node f005432ad5c73b79abd7c5eaa8a0d6eb66ee9194 restarts \ +nothing there either, but its readline question never resolves and it exits \ +0 after the prompt, measured on Node 22.23.2 and 24.18.0 on Linux. Remove \ +this entry if Node exits 1 there, or if Rust drops the decision. +""" + +[[divergence]] +test = "attach_no_restart::a_restart_prompt_off_a_terminal_needs_a_yes" +side = "node" +kind = "intended" +reason = """ +Decision 0020: with stdin not a terminal, only y or yes restarts a dead \ +session from attach's prompt. Pinned Node \ +f005432ad5c73b79abd7c5eaa8a0d6eb66ee9194 restarts on any line but a lone n, \ +so a piped empty line or relayed input reruns the stored command, measured \ +on Node 22.23.2 on Linux. Remove this entry if Node requires an explicit \ +answer, or if Rust drops the decision. +""" + +[[divergence]] +test = "attach_no_restart::force_is_not_an_answer_to_the_restart_prompt" +side = "node" +kind = "intended" +reason = """ +Decision 0020: --force answers nothing, so attach --force at EOF declines, \ +exits 1 and names -r. Pinned Node f005432ad5c73b79abd7c5eaa8a0d6eb66ee9194 \ +also restarts nothing there but exits 0 after the prompt, measured on Node \ +22.23.2 on Linux. The test's piped-y half restarts on both. Remove this \ +entry with the EOF entry above. +""" diff --git a/crates/pty-conformance/tests/attach_no_restart.rs b/crates/pty-conformance/tests/attach_no_restart.rs index 59fcae6d..cfb9f9c9 100644 --- a/crates/pty-conformance/tests/attach_no_restart.rs +++ b/crates/pty-conformance/tests/attach_no_restart.rs @@ -4,6 +4,8 @@ //! default policy prompts `Restart? [Y/n]` and `--auto-restart` re-launches //! without asking. The terminal cases run the CLI inside a real pty //! (`Rig::pty_tty_raw`) with the same 30×100 geometry the Node test uses. +//! The cases after them have no terminal: there the prompt restarts only on a +//! `y`, which departs from Node (decision 0020). use pty_conformance::*; use serde_json::Value; @@ -213,3 +215,87 @@ fn auto_restart_relaunches_without_prompting() { assert_eq!(invocation_count(&marker), 2); assert_eq!(event_count(&rig, id, "session_start"), 1); } + +/// An attach that the prompt declined for want of an answer: exit 1, a way to +/// restart named, and the old record left in place for a later answer. +fn expect_unanswered(rig: &Rig, out: &Out, id: &str, marker: &Path) { + expect_status(out, 1); + expect_contains(&out.stdout(), "Restart? [Y/n]"); + expect_contains(&out.stderr(), &format!("Session \"{id}\" was not restarted")); + expect_contains(&out.stderr(), "pass -r"); + assert_eq!(invocation_count(marker), 1); + let kept = rig.meta(id).expect("the exited record is kept"); + assert_eq!(kept["exitCode"], 42, "{kept}"); +} + +/// Decision 0020: stdin at EOF answers nothing, so the stored command does +/// not run again. +#[test] +fn a_restart_prompt_at_eof_declines() { + let rig = Rig::new(); + let marker = rig.tmp().join("invocations"); + let id = "eof-target"; + spawn_retained_once(&rig, id, &marker); + + let out = rig.pty(&["attach", id]); + expect_unanswered(&rig, &out, id, &marker); +} + +/// Decision 0020: off a terminal, only `y` or `yes` restarts. An empty line, +/// which says yes at a terminal, does not. +#[test] +fn a_restart_prompt_off_a_terminal_needs_a_yes() { + let rig = Rig::new(); + let marker = rig.tmp().join("invocations"); + let id = "piped-target"; + spawn_retained_once(&rig, id, &marker); + + for input in [&b"\n"[..], b"future-relay-input\n"] { + let out = rig.pty_stdin(input, &["attach", id]); + expect_unanswered(&rig, &out, id, &marker); + } +} + +/// A piped `y` is an answer: the session restarts, as at a terminal. +#[test] +fn a_piped_yes_restarts() { + let rig = Rig::new(); + let marker = rig.tmp().join("invocations"); + let id = "yes-target"; + spawn_retained_once(&rig, id, &marker); + + let out = rig.pty_stdin(b"y\n", &["attach", id]); + expect_contains(&out.stdout(), &format!("Session \"{id}\" restarted.")); + wait_until("second invocation", || invocation_count(&marker) == 2); +} + +/// `-r` restarts without a terminal and without asking. +#[test] +fn auto_restart_needs_no_terminal() { + let rig = Rig::new(); + let marker = rig.tmp().join("invocations"); + let id = "auto-piped-target"; + spawn_retained_once(&rig, id, &marker); + + let out = rig.pty(&["attach", "-r", id]); + expect_not_contains(&out.stdout(), "Restart?"); + expect_contains(&out.stdout(), &format!("Session \"{id}\" restarted.")); + wait_until("second invocation", || invocation_count(&marker) == 2); +} + +/// `--force` only allows nesting. It answers nothing, and a `y` after it still +/// restarts. +#[test] +fn force_is_not_an_answer_to_the_restart_prompt() { + let rig = Rig::new(); + let marker = rig.tmp().join("invocations"); + let id = "force-target"; + spawn_retained_once(&rig, id, &marker); + + let out = rig.pty(&["attach", "--force", id]); + expect_unanswered(&rig, &out, id, &marker); + + let out = rig.pty_stdin(b"y\n", &["attach", "--force", id]); + expect_contains(&out.stdout(), &format!("Session \"{id}\" restarted.")); + wait_until("second invocation", || invocation_count(&marker) == 2); +} diff --git a/crates/pty/src/cli/ask.rs b/crates/pty/src/cli/ask.rs index 75ab4507..bac22253 100644 --- a/crates/pty/src/cli/ask.rs +++ b/crates/pty/src/cli/ask.rs @@ -1,38 +1,83 @@ -//! The `[Y/n]` prompt: write the question to stdout, read one line from -//! stdin. No tty check, exactly like Node's `readline` prompt; a closed -//! stdin reads as an empty answer, which proceeds. +//! The `[Y/n]` prompts: write the question to stdout, read one line from +//! stdin. +//! +//! [`ask`] has no tty check, like Node's `readline` prompt, and only a lone +//! `n` declines. A closed stdin reads as an empty answer, which proceeds. +//! Node differs there: its question never resolves at EOF, and the process +//! exits 0 having done nothing. `pty restart` still asks this way. +//! +//! [`ask_consent`] asks for the dead-session restart in `attach`, where only an +//! answer someone gave counts (decision 0020). //! //! node: src/cli.ts:4069-4078 (`ask`), 3922-3926 and 1836-1840 (callers). #![allow(dead_code)] // callers: the restart / dead-session prompts -use std::io::{BufRead, Write}; +use std::io::{BufRead, IsTerminal, Write}; /// Print `prompt` (no newline) and return the answer line without its /// line terminator. pub fn ask(prompt: &str) -> String { + read_answer(prompt).unwrap_or_default() +} + +/// Did the answer decline? Node compares `answer.toLowerCase() === "n"` +/// (the restart prompts) — only a lone `n`/`N` says no; anything else, +/// including an empty line, proceeds. +pub fn declined(answer: &str) -> bool { + answer.to_lowercase() == "n" +} + +/// What the dead-session restart prompt was told. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Consent { + Yes, + /// A lone `n`. + No, + /// Stdin was at EOF, or is not a terminal and did not say `y` or `yes`. + NoAnswer, +} + +/// Print `prompt` and read one line, like [`ask`], but count only an answer. +/// At a terminal, as in Node, anything but a lone `n` says yes, an empty line +/// included. Anywhere else only `y` or `yes` does. EOF is never an answer. +pub fn ask_consent(prompt: &str) -> Consent { + let answer = read_answer(prompt); + consent(answer.as_deref(), std::io::stdin().is_terminal()) +} + +fn consent(answer: Option<&str>, terminal: bool) -> Consent { + match answer { + None => Consent::NoAnswer, + Some(answer) if declined(answer) => Consent::No, + Some(_) if terminal => Consent::Yes, + Some(answer) if answer.eq_ignore_ascii_case("y") || answer.eq_ignore_ascii_case("yes") => { + Consent::Yes + } + Some(_) => Consent::NoAnswer, + } +} + +/// Print `prompt` and read one line. `None` at EOF or on a read error. +fn read_answer(prompt: &str) -> Option { let mut out = std::io::stdout().lock(); let _ = out.write_all(prompt.as_bytes()); let _ = out.flush(); drop(out); let mut line = String::new(); - let _ = std::io::stdin().lock().read_line(&mut line); + match std::io::stdin().lock().read_line(&mut line) { + Ok(0) | Err(_) => return None, + Ok(_) => {} + } while line.ends_with('\n') || line.ends_with('\r') { line.pop(); } - line -} - -/// Did the answer decline? Node compares `answer.toLowerCase() === "n"` -/// (the restart prompts) — only a lone `n`/`N` says no; anything else, -/// including an empty line, proceeds. -pub fn declined(answer: &str) -> bool { - answer.to_lowercase() == "n" + Some(line) } #[cfg(test)] mod tests { - use super::declined; + use super::{Consent, consent, declined}; #[test] fn only_a_lone_n_declines() { @@ -42,4 +87,41 @@ mod tests { assert!(!declined("y")); assert!(!declined("no")); } + + #[test] + fn consent_needs_an_answer_and_off_a_terminal_a_yes() { + for terminal in [true, false] { + assert_eq!( + consent(None, terminal), + Consent::NoAnswer, + "terminal={terminal}" + ); + for no in ["n", "N"] { + assert_eq!( + consent(Some(no), terminal), + Consent::No, + "{no:?} terminal={terminal}" + ); + } + for yes in ["y", "Y", "yes", "YES"] { + assert_eq!( + consent(Some(yes), terminal), + Consent::Yes, + "{yes:?} terminal={terminal}" + ); + } + } + for other in ["", "no", "future-relay-input"] { + assert_eq!( + consent(Some(other), true), + Consent::Yes, + "{other:?} at a terminal" + ); + assert_eq!( + consent(Some(other), false), + Consent::NoAnswer, + "{other:?} off a terminal" + ); + } + } } diff --git a/crates/pty/src/cli/attach.rs b/crates/pty/src/cli/attach.rs index 0cbd3186..167a0c8c 100644 --- a/crates/pty/src/cli/attach.rs +++ b/crates/pty/src/cli/attach.rs @@ -228,8 +228,21 @@ fn handle_dead_session(name: &str, meta: Option, auto_restart: println!("Command was: {command}"); println!(); - if !auto_restart && super::ask::declined(&ask::ask("Restart? [Y/n] ")) { - return Ok(0); + if !auto_restart { + match ask::ask_consent("Restart? [Y/n] ") { + ask::Consent::Yes => {} + ask::Consent::No => return Ok(0), + // Nobody said yes, so the stored command does not run again + // (decision 0020). + ask::Consent::NoAnswer => { + println!(); + eprintln!( + "pty attach: Session \"{name}\" was not restarted: the restart prompt got no answer." + ); + eprintln!(" Answer y, or pass -r to restart it without asking."); + return Ok(1); + } + } } // An exited session's daemon can still be finishing, and it needs the diff --git a/docs/decisions/0020-an-unanswered-restart-prompt-does-not-restart.md b/docs/decisions/0020-an-unanswered-restart-prompt-does-not-restart.md new file mode 100644 index 00000000..7e9cd68e --- /dev/null +++ b/docs/decisions/0020-an-unanswered-restart-prompt-does-not-restart.md @@ -0,0 +1,78 @@ +# 0020 — An unanswered restart prompt does not restart + +**Status:** accepted + +**Node behavior.** `handleDeadSession` (`src/cli.ts`) asks `Restart? [Y/n] ` +through `readline/promises`, with no terminal check. Any line but a lone `n` +restarts the session, an empty line included. When stdin reaches EOF first, +the question never resolves: the process exits 0 after the prompt, and +nothing restarts. + +Measured with no terminal, against the pinned Node reference on Node 22.23.2 +and 24.18.0, and against Rust before this change: + +| stdin | Node | Rust before | +| --- | --- | --- | +| `" was not restarted: the restart prompt got no answer. + Answer y, or pass -r to restart it without asking. + ``` +- A lone `n` still exits 0. `-r` still restarts without asking. `--force` + still only allows attaching from inside another session, and it answers + nothing. + +**Why.** Agents and scripts call `attach` without a terminal (#214). A prompt +that nobody answered is not consent to run a stored command again, and a +rerun repeats whatever the command does. Off a terminal, an empty line or +relayed input is no more a person's answer than EOF is, so only `y` counts +there. Node already restarts nothing at EOF, but it exits 0, the same as a +declined prompt. Exit 1, with the flag that restarts, tells the caller what +happened and what to do. + +**Client effect.** +- A script or seat that relied on `attach` restarting a dead session without a + terminal must pass `-r` now, or pipe `y`. +- At EOF, `attach` exits 1 where Node exits 0. Neither restarts. +- Off a terminal, any line but `y`, `yes` or `n` exits 1 where Node restarts. +- At a terminal nothing changes, except that EOF (Ctrl+D) exits 1. + +**Not changed.** `pty restart` asks `Kill and restart? [Y/n]` through the old +`ask`, so at EOF it still stops a running session and starts it again. Node +exits 0 there and leaves the session running. That is a parity defect outside +this decision and is left for a separate change. + +**Test.** `crates/pty-conformance/tests/attach_no_restart.rs`: +- `a_restart_prompt_at_eof_declines`, + `a_restart_prompt_off_a_terminal_needs_a_yes` and + `force_is_not_an_answer_to_the_restart_prompt` fail on Node. + `crates/pty-conformance/divergences.toml` records all three as intended. +- `a_piped_yes_restarts` and `auto_restart_needs_no_terminal` pass on both. +- The terminal cases before them are unchanged and pass on both. + +The unit test `consent_needs_an_answer_and_off_a_terminal_a_yes` in +`crates/pty/src/cli/ask.rs` covers each answer at a terminal and off one. + +**Migration / negotiation.** Pass `-r` to restart a dead session without a +terminal, or `--no-restart` never to restart one. Release note: `pty attach` +no longer restarts a dead session when the restart prompt gets no answer. +That covers stdin at EOF, and stdin that is not a terminal unless it gives `y` +or `yes`. It exits 1 instead, without running the stored command. Any +script or seat that relied on `attach` restarting a dead session +non-interactively must pass `-r` now. diff --git a/docs/parity.md b/docs/parity.md index 2a891d9b..c2cb44f8 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -381,6 +381,7 @@ Decided on 2026-08-29. Each row records the decision. | `up`/`down` and `pty.toml` | S | Kept. Already ported; the binding rule needs the tag pair. | | `queryStats` waiting out its 2 s timeout on a daemon that closes without STATUS, and `peek -f` hanging on a plain close | S | Not reproduced, on purpose. Both end promptly instead. Deliberate improvements, not gaps — accepted, decision 0006. | | `peek --wait` missing a pattern that scrolls off between two 200 ms screen reads | S | Not reproduced, on purpose. The wait also watches the output stream, and a timeout says when the pattern is in the scrollback from before the wait. Accepted, decision 0019. | +| `attach`'s dead-session prompt restarting with no terminal on any line but `n` | S | Not reproduced, on purpose. Off a terminal only `y` or `yes` restarts, and EOF never does. With no answer, attach exits 1 and names `-r`, where Node exits 0 at EOF and restarts on any other line. Accepted, decision 0020. | ## 12c. What differs because the kernel differs