Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
37 changes: 37 additions & 0 deletions crates/pty-conformance/divergences.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
86 changes: 86 additions & 0 deletions crates/pty-conformance/tests/attach_no_restart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
110 changes: 96 additions & 14 deletions crates/pty/src/cli/ask.rs
Original file line number Diff line number Diff line change
@@ -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<String> {
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() {
Expand All @@ -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"
);
}
}
}
17 changes: 15 additions & 2 deletions crates/pty/src/cli/attach.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,21 @@ fn handle_dead_session(name: &str, meta: Option<SessionMetadata>, 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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 |
| --- | --- | --- |
| `</dev/null` | exit 0, not restarted | restarted |
| an empty line | restarted | restarted |
| `future-input` | restarted | restarted |
| `y` or `yes` | restarted | restarted |
| `n` | exit 0, not restarted | exit 0, not restarted |
| `-r`, `</dev/null` | restarted | restarted |
| `--force`, `</dev/null` | exit 0, not restarted | restarted |

**Rust behavior.** The dead-session prompt in `attach` counts only an answer
that someone gave (`ask::ask_consent`):
- At a terminal, as in Node, any line but a lone `n` restarts, an empty line
included.
- Anywhere else, only `y` or `yes`, in any case, restarts.
- EOF is never an answer, at a terminal or not.
- With no answer, attach ends the prompt line and prints two lines to standard
error. It exits 1 and keeps the exited record:

```
pty attach: Session "<name>" 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.
Loading
Loading