From d437d5bb0484ac1a7f89b1b12aec0221556c7848 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Wed, 4 Nov 2026 04:40:50 +0100 Subject: [PATCH 1/2] Leave a running session alone when restart's prompt gets no answer pty restart asks Kill and restart? [Y/n] before it stops a running session, through the same ask that #215 replaced for attach. At EOF that read as yes, so a caller with no terminal stopped live work and started it again although nobody agreed to it. Node leaves the session running there and exits 0. The prompt now uses ask_consent, by decision 0020's rule: - At a terminal, any line but a lone n restarts, as before. - Off a terminal, only y or yes restarts. - EOF never does. With no answer, restart exits 1, leaves the session running and names -y. A lone n still exits 0 and -y still restarts without asking. The old ask has no callers left and is removed. Decision 0020 gains an addendum and parity.md a row. The conformance ledger records the two new tests that fail on Node as intended. One of #215's test lines is reformatted to rustfmt's layout. --- README.md | 5 +- crates/pty-conformance/divergences.toml | 25 +++++ .../tests/attach_no_restart.rs | 5 +- .../pty-conformance/tests/restart_prompt.rs | 106 ++++++++++++++++++ crates/pty/src/cli/ask.rs | 28 ++--- crates/pty/src/cli/restart.rs | 22 +++- ...nswered-restart-prompt-does-not-restart.md | 42 +++++-- docs/parity.md | 1 + 8 files changed, 198 insertions(+), 36 deletions(-) create mode 100644 crates/pty-conformance/tests/restart_prompt.rs diff --git a/README.md b/README.md index 2fd94fe4..028ddbcb 100644 --- a/README.md +++ b/README.md @@ -502,7 +502,10 @@ 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)). +`restart` asks `Kill and restart? [Y/n]` before it stops a running session, +by the same rule: with no answer it exits 1 and leaves the session running, +and `-y` restarts without asking. 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. diff --git a/crates/pty-conformance/divergences.toml b/crates/pty-conformance/divergences.toml index d0a3371d..70b08d1d 100644 --- a/crates/pty-conformance/divergences.toml +++ b/crates/pty-conformance/divergences.toml @@ -334,3 +334,28 @@ 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. """ + +[[divergence]] +test = "restart_prompt::a_running_session_is_left_running_at_eof" +side = "node" +kind = "intended" +reason = """ +Decision 0020: restart's running-session prompt at EOF declines, exits 1, \ +names -y and leaves the session running. Pinned Node \ +f005432ad5c73b79abd7c5eaa8a0d6eb66ee9194 leaves it running too, but its \ +readline question never resolves and it exits 0 after the prompt, measured \ +on Node 22.23.2 on Linux. Remove this entry if Node exits 1 there, or if \ +Rust drops the decision. +""" + +[[divergence]] +test = "restart_prompt::a_running_session_is_left_running_without_a_yes" +side = "node" +kind = "intended" +reason = """ +Decision 0020: with stdin not a terminal, only y or yes lets restart stop a \ +running session. Pinned Node f005432ad5c73b79abd7c5eaa8a0d6eb66ee9194 stops \ +and restarts it on any line but a lone n, a piped empty line included, \ +measured on Node 22.23.2 on Linux. Remove this entry if Node requires an \ +explicit answer, or if Rust drops the decision. +""" diff --git a/crates/pty-conformance/tests/attach_no_restart.rs b/crates/pty-conformance/tests/attach_no_restart.rs index cfb9f9c9..8fd77b4f 100644 --- a/crates/pty-conformance/tests/attach_no_restart.rs +++ b/crates/pty-conformance/tests/attach_no_restart.rs @@ -221,7 +221,10 @@ fn auto_restart_relaunches_without_prompting() { 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(), + &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"); diff --git a/crates/pty-conformance/tests/restart_prompt.rs b/crates/pty-conformance/tests/restart_prompt.rs new file mode 100644 index 00000000..f4909060 --- /dev/null +++ b/crates/pty-conformance/tests/restart_prompt.rs @@ -0,0 +1,106 @@ +//! `pty restart`'s `Kill and restart? [Y/n]` for a running session, with no +//! terminal. Only `y` or `yes` restarts there, and EOF never does. Without an +//! answer the session is left running and the command exits 1, which departs +//! from Node (decision 0020). `-y` restarts without asking, as before. +//! +//! Every case sets `PTY_SESSION`, so a restart that goes ahead skips the +//! attach that would follow it. The prompt comes before that choice, and a +//! binary that restarts by mistake then exits instead of attaching. + +use pty_conformance::*; +use std::path::Path; + +const OUTER: (&str, &str) = ("PTY_SESSION", "outer"); + +fn start_count(marker: &Path) -> usize { + std::fs::read_to_string(marker) + .map(|s| s.lines().filter(|l| !l.is_empty()).count()) + .unwrap_or(0) +} + +/// A running session that appends a line to `marker` each time it starts. +/// Returns its daemon pid. +fn spawn_running(rig: &Rig, id: &str, marker: &Path) -> i32 { + let script = format!( + "printf 'started\\n' >> '{}'; exec sleep 300", + marker.display() + ); + let d = rig.daemon(id, &["sh", "-c", &script], DaemonOpts::no_display_name()); + wait_until("first start", || start_count(marker) == 1); + d.pid() +} + +/// A restart that the prompt declined for want of an answer: exit 1, a way +/// to restart named, and the same daemon still running. +fn expect_left_running(rig: &Rig, out: &Out, id: &str, marker: &Path, pid: i32) { + expect_status(out, 1); + expect_contains(&out.stdout(), "Kill and restart? [Y/n]"); + expect_contains( + &out.stderr(), + &format!("Session \"{id}\" was not restarted"), + ); + expect_contains(&out.stderr(), "pass -y"); + assert!(pid_alive(pid), "daemon {pid} was stopped"); + assert_eq!(rig.pid(id), Some(pid), "the session has a new daemon"); + assert_eq!(start_count(marker), 1); + let entry = rig.list_entry(id).expect("the session is still listed"); + assert_eq!(entry["status"], "running", "{entry}"); +} + +/// Decision 0020: stdin at EOF answers nothing, so the running session is +/// not stopped. +#[test] +fn a_running_session_is_left_running_at_eof() { + let rig = Rig::new(); + let marker = rig.tmp().join("starts"); + let id = "eof-running"; + let pid = spawn_running(&rig, id, &marker); + + let out = rig.pty_env(&[OUTER], &["restart", id]); + expect_left_running(&rig, &out, id, &marker, pid); +} + +/// Decision 0020: off a terminal, an empty line, which says yes at a +/// terminal, does not stop the session, and neither does relayed input. +#[test] +fn a_running_session_is_left_running_without_a_yes() { + let rig = Rig::new(); + let marker = rig.tmp().join("starts"); + let id = "piped-running"; + let pid = spawn_running(&rig, id, &marker); + + for input in [&b"\n"[..], b"future-relay-input\n"] { + let out = rig.pty_stdin_env(input, &[OUTER], &["restart", id]); + expect_left_running(&rig, &out, id, &marker, pid); + } +} + +/// A piped `y` is an answer: the session is stopped and started again. +#[test] +fn a_piped_yes_restarts_a_running_session() { + let rig = Rig::new(); + let marker = rig.tmp().join("starts"); + let id = "yes-running"; + let pid = spawn_running(&rig, id, &marker); + + let out = rig.pty_stdin_env(b"y\n", &[OUTER], &["restart", id]); + expect_status(&out, 0); + expect_contains(&out.stdout(), &format!("Session \"{id}\" restarted.")); + wait_until("second start", || start_count(&marker) == 2); + assert!(process_exited(pid), "old daemon {pid} still runs"); +} + +/// `-y` restarts without a terminal and without asking. +#[test] +fn yes_flag_restarts_without_a_terminal() { + let rig = Rig::new(); + let marker = rig.tmp().join("starts"); + let id = "flag-running"; + spawn_running(&rig, id, &marker); + + let out = rig.pty_env(&[OUTER], &["restart", "-y", id]); + expect_status(&out, 0); + expect_not_contains(&out.stdout(), "Kill and restart?"); + expect_contains(&out.stdout(), &format!("Session \"{id}\" restarted.")); + wait_until("second start", || start_count(&marker) == 2); +} diff --git a/crates/pty/src/cli/ask.rs b/crates/pty/src/cli/ask.rs index bac22253..97a8cb01 100644 --- a/crates/pty/src/cli/ask.rs +++ b/crates/pty/src/cli/ask.rs @@ -1,13 +1,10 @@ -//! The `[Y/n]` prompts: write the question to stdout, read one line from -//! stdin. +//! The `[Y/n]` prompts in `attach` and `restart`: write the question to +//! stdout, read one line from stdin, and count only an answer someone gave +//! (decision 0020). //! -//! [`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's `readline` prompt has no tty check, and only a lone `n` declines. +//! At EOF its question never resolves, and the process exits 0 having done +//! nothing. //! //! node: src/cli.ts:4069-4078 (`ask`), 3922-3926 and 1836-1840 (callers). @@ -15,12 +12,6 @@ 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. @@ -28,7 +19,7 @@ pub fn declined(answer: &str) -> bool { answer.to_lowercase() == "n" } -/// What the dead-session restart prompt was told. +/// What a restart prompt was told. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Consent { Yes, @@ -38,7 +29,7 @@ pub enum Consent { NoAnswer, } -/// Print `prompt` and read one line, like [`ask`], but count only an answer. +/// Print `prompt` (no newline) and read one line, counting 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 { @@ -58,7 +49,8 @@ fn consent(answer: Option<&str>, terminal: bool) -> Consent { } } -/// Print `prompt` and read one line. `None` at EOF or on a read error. +/// Print `prompt` and read one line without its line terminator. `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()); diff --git a/crates/pty/src/cli/restart.rs b/crates/pty/src/cli/restart.rs index 8d04435b..9141d672 100644 --- a/crates/pty/src/cli/restart.rs +++ b/crates/pty/src/cli/restart.rs @@ -105,13 +105,23 @@ pub fn run(args: &[String]) -> CliResult { return Ok(1); } - if session.status == SessionStatus::Running - && !yes - && ask::declined(&ask::ask(&format!( + if session.status == SessionStatus::Running && !yes { + match ask::ask_consent(&format!( "Session \"{name}\" is running. Kill and restart? [Y/n] " - ))) - { - return Ok(0); + )) { + ask::Consent::Yes => {} + ask::Consent::No => return Ok(0), + // Nobody said yes, so the running session is left alone + // (decision 0020). + ask::Consent::NoAnswer => { + println!(); + eprintln!( + "pty restart: Session \"{name}\" was not restarted: the restart prompt got no answer." + ); + eprintln!(" Answer y, or pass -y to restart it without asking."); + return Ok(1); + } + } } let mut params = SpawnParams::new(&name, &meta.command, &meta.args); params.display_command = meta.display_command.clone(); 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 index 7e9cd68e..3134d025 100644 --- a/docs/decisions/0020-an-unanswered-restart-prompt-does-not-restart.md +++ b/docs/decisions/0020-an-unanswered-restart-prompt-does-not-restart.md @@ -53,10 +53,22 @@ happened and what to do. - 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. +**Addendum: `pty restart`.** The same rule covers `restart`'s prompt for a +running session, `Session "" is running. Kill and restart? [Y/n]`. +Node's `cmdRestart` asks it through the same `ask`. Measured the same way: + +| stdin | Node | Rust before | +| --- | --- | --- | +| `" was not restarted: the restart prompt got no answer.` +and ` Answer y, or pass -y to restart it without asking.`, and exits 1. +`-y` restarts without asking, as before. Stopping live work without consent is +the worse case of the two, so the same rule applies. The old `ask`, which +proceeded at EOF, is gone. **Test.** `crates/pty-conformance/tests/attach_no_restart.rs`: - `a_restart_prompt_at_eof_declines`, @@ -66,13 +78,23 @@ this decision and is left for a separate change. - `a_piped_yes_restarts` and `auto_restart_needs_no_terminal` pass on both. - The terminal cases before them are unchanged and pass on both. +`crates/pty-conformance/tests/restart_prompt.rs`: +- `a_running_session_is_left_running_at_eof` and + `a_running_session_is_left_running_without_a_yes` fail on Node and are + recorded as intended. Each checks that the same daemon still runs and the + command did not start again. +- `a_piped_yes_restarts_a_running_session` and + `yes_flag_restarts_without_a_terminal` 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. +terminal, or `--no-restart` never to restart one. Pass `-y` to `restart` a +running session without a terminal. Release note: `pty attach` and `pty +restart` no longer restart a session when their restart prompt gets no +answer. That covers stdin at EOF, and stdin that is not a terminal unless it +gives `y` or `yes`. They exit 1 instead: `attach` without running the stored +command, `restart` without stopping the running session. Any script or seat +that relied on either restarting non-interactively must pass `-r` to +`attach` or `-y` to `restart` now. diff --git a/docs/parity.md b/docs/parity.md index c2cb44f8..2f4bfd6a 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -382,6 +382,7 @@ Decided on 2026-08-29. Each row records the decision. | `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. | +| `restart`'s running-session prompt stopping and restarting with no terminal on any line but `n` | S | Not reproduced, on purpose. The same rule as `attach`'s prompt: with no answer, `restart` exits 1 and leaves the session running, and names `-y`. Node exits 0 at EOF and restarts on any other line. Accepted, decision 0020. | ## 12c. What differs because the kernel differs From e2e4964c88dd7744288df74ec2c2630db8faee3c Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Wed, 4 Nov 2026 05:00:38 +0100 Subject: [PATCH 2/2] Refuse a revoked restart before asking, not after restart checked the publication binding only after its prompt. It asked Kill and restart? and then refused a revoked or unverifiable managed launch, and with no terminal the refusal could not be reached at all: publication_fence's restart_refuses_revoked_binding_before_stopping_survivor runs restart --force without -y and got the no-answer message instead. The check now runs before the prompt. After a yes it runs again before the session is stopped, since an answer can take any time. Decision 0020's addendum says so. --- crates/pty/src/cli/restart.rs | 25 +++++++++++-------- ...nswered-restart-prompt-does-not-restart.md | 5 ++++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/crates/pty/src/cli/restart.rs b/crates/pty/src/cli/restart.rs index 9141d672..c300739b 100644 --- a/crates/pty/src/cli/restart.rs +++ b/crates/pty/src/cli/restart.rs @@ -105,11 +105,24 @@ pub fn run(args: &[String]) -> CliResult { return Ok(1); } + let mut params = SpawnParams::new(&name, &meta.command, &meta.args); + params.display_command = meta.display_command.clone(); + params.cwd = meta.cwd.clone(); + // An explicit retry drops flapping bookkeeping, never launch provenance. + params.tags = registry::strip_gc_bookkeeping(meta.tags.as_ref()).unwrap_or_default(); + params.display_name = meta.display_name.clone(); + apply_persisted_launch_options(&mut params, &meta); + params.scrub_env = RESTART_SCRUBBED_ENV.iter().map(|s| s.to_string()).collect(); + // Refuse a revoked or unverifiable launch before asking, and so before + // stopping the survivor. + pty_lifecycle::config_for(¶ms).check_publication()?; + if session.status == SessionStatus::Running && !yes { match ask::ask_consent(&format!( "Session \"{name}\" is running. Kill and restart? [Y/n] " )) { - ask::Consent::Yes => {} + // An answer can take any time, so check again before the stop. + ask::Consent::Yes => pty_lifecycle::config_for(¶ms).check_publication()?, ask::Consent::No => return Ok(0), // Nobody said yes, so the running session is left alone // (decision 0020). @@ -123,16 +136,6 @@ pub fn run(args: &[String]) -> CliResult { } } } - let mut params = SpawnParams::new(&name, &meta.command, &meta.args); - params.display_command = meta.display_command.clone(); - params.cwd = meta.cwd.clone(); - // An explicit retry drops flapping bookkeeping, never launch provenance. - params.tags = registry::strip_gc_bookkeeping(meta.tags.as_ref()).unwrap_or_default(); - params.display_name = meta.display_name.clone(); - apply_persisted_launch_options(&mut params, &meta); - params.scrub_env = RESTART_SCRUBBED_ENV.iter().map(|s| s.to_string()).collect(); - // Refuse a revoked or unverifiable launch before stopping the survivor. - pty_lifecycle::config_for(¶ms).check_publication()?; let publisher = super::prepare_daemon(&mut params)?; if session.status == SessionStatus::Running { 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 index 3134d025..d5f06751 100644 --- a/docs/decisions/0020-an-unanswered-restart-prompt-does-not-restart.md +++ b/docs/decisions/0020-an-unanswered-restart-prompt-does-not-restart.md @@ -70,6 +70,11 @@ and ` Answer y, or pass -y to restart it without asking.`, and exits 1. the worse case of the two, so the same rule applies. The old `ask`, which proceeded at EOF, is gone. +A revoked or unverifiable managed launch is refused before the prompt, so +`restart` never asks a question whose yes it would then refuse. After a yes it +checks again before it stops the session, because the answer can take any +time. + **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