diff --git a/README.md b/README.md index 028ddbcb..52e96796 100644 --- a/README.md +++ b/README.md @@ -498,12 +498,14 @@ 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 +exited shows its last lines and asks `Restart? [Y/n]`. A lone `n` or `no` +declines, and attach exits 0. At a terminal any other line restarts it, Enter +included. Without a terminal 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. +A revoked managed launch is refused before the question is asked. `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, +by the same rules: 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)). diff --git a/crates/pty-conformance/divergences.toml b/crates/pty-conformance/divergences.toml index 70b08d1d..a5b909a7 100644 --- a/crates/pty-conformance/divergences.toml +++ b/crates/pty-conformance/divergences.toml @@ -359,3 +359,27 @@ 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. """ + +[[divergence]] +test = "attach_no_restart::no_declines_the_restart_prompt" +side = "node" +kind = "intended" +reason = """ +Decision 0020: a lone n or no, in any case, declines attach's dead-session \ +prompt at a terminal and off one, and exits 0. Pinned Node \ +f005432ad5c73b79abd7c5eaa8a0d6eb66ee9194 declines only on a lone n \ +(answer.toLowerCase() === "n"), so no restarts the session. Remove this entry \ +if Node declines on no, or if Rust drops the decision. +""" + +[[divergence]] +test = "restart_prompt::no_leaves_the_running_session_alone" +side = "node" +kind = "intended" +reason = """ +Decision 0020: a lone n or no, in any case, declines restart's running-session \ +prompt at a terminal and off one, exits 0 and leaves the session running. \ +Pinned Node f005432ad5c73b79abd7c5eaa8a0d6eb66ee9194 declines only on a lone \ +n, so no stops and restarts the session. Remove this entry if Node declines \ +on no, 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 8fd77b4f..3f88b1aa 100644 --- a/crates/pty-conformance/tests/attach_no_restart.rs +++ b/crates/pty-conformance/tests/attach_no_restart.rs @@ -302,3 +302,31 @@ fn force_is_not_an_answer_to_the_restart_prompt() { expect_contains(&out.stdout(), &format!("Session \"{id}\" restarted.")); wait_until("second invocation", || invocation_count(&marker) == 2); } + +/// Decision 0020: `no` declines, at a terminal and off one, and exits 0 as +/// `n` does. Node restarts on any line but a lone `n`. +#[test] +fn no_declines_the_restart_prompt() { + let rig = Rig::new(); + let marker = rig.tmp().join("invocations"); + let id = "no-target"; + spawn_retained_once(&rig, id, &marker); + + let mut t = rig.pty_tty_raw(&[], &[], &["attach", id], 30, 100); + assert!( + t.wait_for_text("Restart? [Y/n]", deadline()), + "no prompt: {}", + t.output_str() + ); + t.write(b"no\r"); + let code = t.wait_exit(Duration::from_secs(10)).expect("attach exits"); + assert_eq!(code, 0, "{}", t.output_str()); + + let out = rig.pty_stdin(b"No\n", &["attach", id]); + expect_status(&out, 0); + expect_contains(&out.stdout(), "Restart? [Y/n]"); + + assert_eq!(invocation_count(&marker), 1); + let kept = rig.meta(id).expect("the exited record is kept"); + assert_eq!(kept["exitCode"], 42, "{kept}"); +} diff --git a/crates/pty-conformance/tests/restart_prompt.rs b/crates/pty-conformance/tests/restart_prompt.rs index f4909060..33694d3d 100644 --- a/crates/pty-conformance/tests/restart_prompt.rs +++ b/crates/pty-conformance/tests/restart_prompt.rs @@ -9,6 +9,7 @@ use pty_conformance::*; use std::path::Path; +use std::time::Duration; const OUTER: (&str, &str) = ("PTY_SESSION", "outer"); @@ -104,3 +105,31 @@ fn yes_flag_restarts_without_a_terminal() { expect_contains(&out.stdout(), &format!("Session \"{id}\" restarted.")); wait_until("second start", || start_count(&marker) == 2); } + +/// Decision 0020: `no` declines, at a terminal and off one, exits 0 and +/// leaves the session running. Node restarts on any line but a lone `n`. +#[test] +fn no_leaves_the_running_session_alone() { + let rig = Rig::new(); + let marker = rig.tmp().join("starts"); + let id = "no-running"; + let pid = spawn_running(&rig, id, &marker); + + let mut t = rig.pty_tty_raw(&[OUTER], &[], &["restart", id], 30, 100); + assert!( + t.wait_for_text("Kill and restart? [Y/n]", deadline()), + "no prompt: {}", + t.output_str() + ); + t.write(b"no\r"); + let code = t.wait_exit(Duration::from_secs(10)).expect("restart exits"); + assert_eq!(code, 0, "{}", t.output_str()); + + let out = rig.pty_stdin_env(b"NO\n", &[OUTER], &["restart", id]); + expect_status(&out, 0); + expect_contains(&out.stdout(), "Kill and restart? [Y/n]"); + + 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); +} diff --git a/crates/pty/src/cli/ask.rs b/crates/pty/src/cli/ask.rs index 97a8cb01..1dcf490a 100644 --- a/crates/pty/src/cli/ask.rs +++ b/crates/pty/src/cli/ask.rs @@ -2,9 +2,9 @@ //! stdout, read one line from stdin, and count only an answer someone gave //! (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's `readline` prompt has no tty check, and only a lone `n` declines; +//! here `no` declines too. At EOF Node's question never resolves, and the +//! process exits 0 having done nothing. //! //! node: src/cli.ts:4069-4078 (`ask`), 3922-3926 and 1836-1840 (callers). @@ -12,25 +12,24 @@ use std::io::{BufRead, IsTerminal, Write}; -/// 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. +/// Did the answer decline? A lone `n` or `no`, in any case. Node compares +/// `answer.toLowerCase() === "n"`, so its `no` proceeds (decision 0020). pub fn declined(answer: &str) -> bool { - answer.to_lowercase() == "n" + matches!(answer.to_lowercase().as_str(), "n" | "no") } /// What a restart prompt was told. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Consent { Yes, - /// A lone `n`. + /// A lone `n` or `no`. No, /// Stdin was at EOF, or is not a terminal and did not say `y` or `yes`. NoAnswer, } /// 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 +/// At a terminal, as in Node, anything but a decline 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); @@ -72,14 +71,17 @@ mod tests { use super::{Consent, consent, declined}; #[test] - fn only_a_lone_n_declines() { - assert!(declined("n")); - assert!(declined("N")); - assert!(!declined("")); - assert!(!declined("y")); - assert!(!declined("no")); + fn a_lone_n_or_no_declines() { + for no in ["n", "N", "no", "No", "NO"] { + assert!(declined(no), "{no:?}"); + } + for other in ["", "y", "nope", "no thanks", " n"] { + assert!(!declined(other), "{other:?}"); + } } + /// `attach`'s dead-session prompt and `restart`'s running-session prompt + /// both ask through [`super::ask_consent`], so this covers both. #[test] fn consent_needs_an_answer_and_off_a_terminal_a_yes() { for terminal in [true, false] { @@ -88,7 +90,7 @@ mod tests { Consent::NoAnswer, "terminal={terminal}" ); - for no in ["n", "N"] { + for no in ["n", "N", "no", "No", "NO"] { assert_eq!( consent(Some(no), terminal), Consent::No, @@ -103,7 +105,7 @@ mod tests { ); } } - for other in ["", "no", "future-relay-input"] { + for other in ["", "nope", "future-relay-input"] { assert_eq!( consent(Some(other), true), Consent::Yes, diff --git a/crates/pty/src/cli/attach.rs b/crates/pty/src/cli/attach.rs index 167a0c8c..c60f8c4b 100644 --- a/crates/pty/src/cli/attach.rs +++ b/crates/pty/src/cli/attach.rs @@ -228,6 +228,19 @@ fn handle_dead_session(name: &str, meta: Option, auto_restart: println!("Command was: {command}"); println!(); + // The record is checked unchanged under the locks below, so these + // parameters still describe it there. + let mut params = SpawnParams::new(name, &meta.command, &meta.args); + params.display_command = meta.display_command.clone(); + params.cwd = meta.cwd.clone(); + params.tags = meta.tags.clone().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, as `restart` + // does (decision 0020). + pty_lifecycle::config_for(¶ms).check_publication()?; + if !auto_restart { match ask::ask_consent("Restart? [Y/n] ") { ask::Consent::Yes => {} @@ -259,13 +272,7 @@ fn handle_dead_session(name: &str, meta: Option, auto_restart: if !registry::metadata_matches_observation(&meta, ¤t) { return Err(format!("Session \"{name}\" changed before restart.").into()); } - let mut params = SpawnParams::new(name, &meta.command, &meta.args); - params.display_command = meta.display_command.clone(); - params.cwd = meta.cwd.clone(); - params.tags = meta.tags.clone().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(); + // Again under the locks: the answer can take any time. pty_lifecycle::config_for(¶ms).check_publication()?; let publisher = super::prepare_daemon(&mut params)?; params.creation_lock_delegate = Some(creation_guard.delegate().map_err(|error| error.to_string())?); diff --git a/crates/pty/tests/publication_fence.rs b/crates/pty/tests/publication_fence.rs index 23ce6cd2..f59cb0ff 100644 --- a/crates/pty/tests/publication_fence.rs +++ b/crates/pty/tests/publication_fence.rs @@ -166,6 +166,60 @@ fn restart_refuses_revoked_binding_before_stopping_survivor() { before.assert_unchanged(&rig, "survivor"); } +/// A managed session that ran once, appending a line to `marker`, and ended +/// with its record kept. +fn ended_managed(rig: &Rig, name: &str, root: &str, operation: &str, digest: &str, marker: &std::path::Path) { + let started = rig.run(&["run", "-d", "--force", "--id", name, + "--publication-operation", operation, "--publication-binding", digest, + "--publication-root-id", root, "--tag", "keep=true", + "--tag", &format!("st3.start-operation={operation}"), + "--", "/bin/sh", "-c", "printf 'ran\\n' >> \"$1\"", "sh", marker.to_str().unwrap()]); + assert_eq!(started.code, 0, "{}", started.stderr); + wait_until("the session to end", || { + rig.read_meta(name).is_some_and(|meta| meta.get("exitedAt").is_some()) + && !rig.exists(&format!("{name}.sock")) + }); +} + +fn runs(marker: &std::path::Path) -> usize { + std::fs::read_to_string(marker).map(|s| s.lines().count()).unwrap_or(0) +} + +#[test] +fn attach_refuses_a_revoked_dead_session_before_asking() { + let rig = rig(); + let root = identity(&rig); + let marker = rig.scratch.join("ended-runs"); + ended_managed(&rig, "ended", &root, OLD, DIGEST, &marker); + assert_eq!(runs(&marker), 1); + rig.ok(&install_args("ended", &root)); + let before = std::fs::read(rig.path("ended.json")).unwrap(); + for (label, out) in [ + ("no answer", rig.run(&["attach", "ended"])), + ("y", rig.run_stdin(&["attach", "ended"], "y\n")), + ("-r", rig.run(&["attach", "-r", "ended"])), + ] { + assert_eq!(out.code, 1, "{label}: {}", out.stderr); + assert!(out.stderr.contains("operation-revoked"), "{label}: {}", out.stderr); + assert!(!out.stdout.contains("Restart?"), "{label} was asked: {}", out.stdout); + assert_eq!(std::fs::read(rig.path("ended.json")).unwrap(), before, "{label}"); + assert!(!rig.exists("ended.sock"), "{label} started a daemon"); + } + assert_eq!(runs(&marker), 1); +} + +#[test] +fn attach_still_restarts_a_dead_managed_session_that_is_not_revoked() { + let rig = rig(); + let root = identity(&rig); + let marker = rig.scratch.join("ended-runs"); + ended_managed(&rig, "ended", &root, NEW, NEW_DIGEST, &marker); + let out = rig.run_stdin(&["attach", "ended"], "y\n"); + assert!(out.stdout.contains("Restart? [Y/n]"), "{}", out.stdout); + assert!(out.stdout.contains("Session \"ended\" restarted."), "{}\n{}", out.stdout, out.stderr); + wait_until("the second run", || runs(&marker) == 2); +} + #[test] fn immutable_launch_provenance_cannot_be_forged_through_metadata_or_tags() { let rig = rig(); 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 d5f06751..5a53eac2 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 @@ -75,6 +75,17 @@ A revoked or unverifiable managed launch is refused before the prompt, so checks again before it stops the session, because the answer can take any time. +**Addendum: `no` declines.** At either prompt, a lone `n` or `no`, in any case, +declines, at a terminal and off one, and exits 0. Node declines only on a lone +`n`, so its `no` restarts the session; so did Rust's until this addendum. +Someone who types `no` means no. + +**Addendum: `attach` checks publication first.** `attach` refuses a revoked +or unverifiable managed launch before its `Restart? [Y/n]`, as `restart` does, +so it never asks a question whose yes it would then refuse. With `-r`, with no +answer and with `y`, the refusal is the same. The check runs again under the +locks before the restart, after the answer. + **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 @@ -91,6 +102,13 @@ time. - `a_piped_yes_restarts_a_running_session` and `yes_flag_restarts_without_a_terminal` pass on both. +`no_declines_the_restart_prompt` (attach) and +`no_leaves_the_running_session_alone` (restart) type `no` at a terminal and +pipe it off one. Both fail on Node and are recorded as intended. +`crates/pty/tests/publication_fence.rs`: +`attach_refuses_a_revoked_dead_session_before_asking` and +`attach_still_restarts_a_dead_managed_session_that_is_not_revoked`. + 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. @@ -102,4 +120,5 @@ 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. +`attach` or `-y` to `restart` now. `no` now declines either prompt, as `n` +does. diff --git a/docs/parity.md b/docs/parity.md index 2f4bfd6a..f823fb74 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -383,6 +383,7 @@ Decided on 2026-08-29. Each row records the decision. | `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. | +| `no` at a restart prompt counting as yes | S | Not reproduced, on purpose. A lone `n` or `no`, in any case, declines `attach`'s and `restart`'s prompts and exits 0. Node declines only on a lone `n`. Accepted, decision 0020. | ## 12c. What differs because the kernel differs