fix(cli): close the gaps review found in wait, --key and pane close

Five things the first pass got wrong, in the order they bite.

`--until free --changed` waited on a command it had already missed: the
"something ran" edge is only set by a poll that catches the pane busy, and a
command that starts and finishes inside one 500ms interval never is. That is
indistinguishable from a command that never ran, so the timeout now names both
doors instead of letting a finished build read as a hang.

`free` also outranked the agent ladder, which is backwards. A pane whose depth-0
process *is* the agent — the tree cannot tell that apart from a shell at its
prompt — reads free for its whole turn, so a `waiting` the caller explicitly
asked for could be overwritten by a process-tree fact and then withheld by the
`--changed` rule that comes with it. `free` is now consulted only when none of
the requested agent states answered, which is both cheaper and what the docs
already claimed. An empty process tree is "we could not see in" rather than
"free" for the same reason `no-agent` exists.

`--key M-X` sent `ESC x`: the whole spelling was folded to lowercase, which is
free for Ctrl (the C0 rule clears the case anyway) and wrong for Alt, where the
character rides through as itself.

`send --help` listed the key vocabulary by hand next to the table it is a list
of; it had already drifted by one alias. It is generated now.

And a `pane close` batch that could not close everything raised an error, which
left `--json` holding prose exactly when a cleanup script needs to know which
panes are still its problem. It exits 1 with `{"closed":[…],"failed":[…]}`, with
the complaint still on stderr so `-q` reports it.
This commit is contained in:
l0ng-ai
2026-08-11 20:00:14 +08:00
parent f3b10a76ec
commit 420431d5e2
8 changed files with 283 additions and 33 deletions
+6 -2
View File
@@ -68,7 +68,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
pane is back to its bare shell, which is what a `cargo test` running in a
pane has instead of an agent status. With `--changed` it means "something ran
and then finished", the shape you want on the line after a `send`. It costs a
second request per poll and so is only checked when you name it.
second request per poll and so is only checked when you name it — and only
when none of the agent states you named answered first, so `waiting,done,free`
on a pane of unknown kind cannot lose you a `waiting`.
- **`tty7 send --key` presses keys instead of typing characters** — `C-c` to
stop a runaway build, `escape` to close a TUI, `up`/`down`/`enter` to answer
@@ -92,7 +94,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- **`tty7 pane close --json` now reports `{"closed": [ids]}`** rather than a
single `{"closed": id}`, because the verb takes more than one pane.
single `{"closed": id}`, because the verb takes more than one pane. A batch
that could not close everything exits 1 with `{"closed": […], "failed": […]}`
and the complaint on stderr, so a retry knows what is left.
### Fixed
+4 -8
View File
@@ -68,16 +68,12 @@ pub enum Command {
#[command(about = "Split a pane (= tty7 pane split)")]
Split(SplitArgs),
// The key list is built from the table it is a list *of*, rather than
// written out here: a hand-copied vocabulary drifts the first time a key
// is added, and this is the text a caller reaches for to learn the names.
#[command(
about = "Type text into a pane, or send it keystrokes with --key",
long_about = "Types TEXT into the pane exactly as a keyboard would.\n\n\
--key sends a keystroke rather than characters, which is what a pane \
wants once something is already running in it: answering a prompt that \
only takes arrow keys, closing a TUI with escape, stopping a build with \
C-c. Repeat it for a sequence.\n\n\
Keys: enter, escape, tab, backtab, space, backspace, delete, up, down, \
right, left, home, end, pageup, pagedown, C-<char> (Ctrl), M-<char> \
(Alt). Aliases: return, cr, esc, del, bs, shift-tab, pgup, pgdn."
long_about = crate::keys::send_long_help()
)]
Send(SendArgs),
+154 -10
View File
@@ -826,12 +826,24 @@ fn pane_close(
}
if !failures.is_empty() {
bail!(
"closed {} pane(s); {} could not be closed — {}",
// Structured even here, for the reason `wait` is: the caller was
// cleaning up, and what they need next is which panes are still theirs
// to deal with — an anyhow error would leave `--json` holding prose.
// The complaint goes to stderr all the same, so `-q` still reports it
// and the exit code is not the only thing that says so.
eprintln!(
"tty7: closed {} pane(s); {} could not be closed — {}",
closed.len(),
failures.len(),
failures.join("; ")
);
return Ok(Outcome::Exit(
1,
Report {
human: String::new(),
json: json!({ "closed": closed, "failed": failures }),
},
));
}
let human = match closed.as_slice() {
// The single-pane case is the overwhelming one and has always been
@@ -973,10 +985,12 @@ fn wait(args: WaitArgs, ctx: &Context, backend: &mut dyn Backend) -> Result<Outc
let mut changed = cursor != baseline;
// `free` is a fact about the process tree, not the agent ladder, so it
// is asked separately and only when requested. It outranks the ladder
// when true: a pane whose foreground command has exited is free
// whatever a hook last said about it.
if watch_free && current != WaitState::Exit {
// is asked separately, only when requested, and only once the ladder
// has failed to answer. A state the caller listed is their answer:
// overwriting a real `waiting` with a process-tree fact would strand a
// pane whose depth-0 process *is* the agent (see `pane_is_free`), where
// the tree reads free for the whole turn.
if watch_free && current != WaitState::Exit && !args.until.contains(&current) {
if pane_is_free(backend, pane)? {
current = WaitState::Free;
changed = seen_busy;
@@ -1055,6 +1069,17 @@ fn wait(args: WaitArgs, ctx: &Context, backend: &mut dyn Backend) -> Result<Outc
status hook",
);
}
// `--changed` needs to have *seen* the pane busy, and a command
// that starts and finishes inside one interval never is. That
// looks exactly like "the command never ran", so say both, rather
// than let a finished command read as a timeout.
if current == WaitState::Free && !seen_busy {
human.push_str(
"\nnothing was ever seen running here — either the command never started, \
or it finished inside one --interval. Poll faster (--interval 100) or drop \
--changed",
);
}
return Ok(Outcome::Exit(
124,
Report {
@@ -1078,8 +1103,18 @@ fn wait(args: WaitArgs, ctx: &Context, backend: &mut dyn Backend) -> Result<Outc
/// Depth, not count: the pane's own shell sits at depth 0 and everything it
/// launched hangs below, so "nothing deeper than the shell" holds however many
/// shells the pane ended up with, and does not have to guess at process names.
/// It is also the portable question — Windows has no foreground process group
/// to ask about, so `ProcEntry::foreground` is never true there.
///
/// What it cannot see, both by construction: a pane whose depth-0 process *is*
/// the command — which is what `tty7 run` spawns — reads free for as long as it
/// runs, and a backgrounded job keeps a pane busy after the foreground command
/// is long gone. An empty tree is "we could not see in" rather than "free":
/// answering free there would be the same false success `no-agent` exists to
/// remove.
fn pane_is_free(backend: &mut dyn Backend, pane: u64) -> Result<bool> {
Ok(backend.procs(pane)?.procs.iter().all(|p| p.depth == 0))
let procs = backend.procs(pane)?.procs;
Ok(!procs.is_empty() && procs.iter().all(|p| p.depth == 0))
}
/// Whether the daemon still has a live pane behind this id. Absent from the
@@ -2020,6 +2055,10 @@ mod tests {
/// A batch keeps going after a failure. Stopping at the first one would
/// leave the rest of the leak exactly where it was — while still reporting
/// the failure, because a half-done cleanup that claims success is worse.
///
/// Reported as an exit code carrying a report, not as an error: the caller
/// was cleaning up, and the useful answer is which panes are still theirs
/// to deal with. An anyhow error would leave `--json` with prose.
#[test]
fn pane_close_reports_failures_without_abandoning_the_batch() {
let mut backend = mock();
@@ -2030,13 +2069,30 @@ mod tests {
];
backend.kill_failures = vec![78];
let err = execute(
let out = execute(
cli(&["tty7", "pane", "close", "--orphans"]),
&Context::default(),
&mut backend,
)
.expect_err("a pane that could not be closed has to be reported");
assert!(err.to_string().contains("%78"), "{err}");
.expect("a partial cleanup is an exit code, not an error");
let Outcome::Exit(1, r) = out else {
panic!("a pane that could not be closed has to be reported");
};
assert_eq!(
r.json["closed"],
serde_json::json!([77, 79]),
"the survivors of the batch are what a retry needs: {}",
r.json
);
assert!(
r.json["failed"]
.as_array()
.expect("the failures are a list")
.iter()
.any(|f| f.as_str().is_some_and(|f| f.contains("%78"))),
"{}",
r.json
);
assert_eq!(
backend.killed,
vec![77, 78, 79],
@@ -2866,6 +2922,94 @@ mod tests {
assert_eq!(json["stale"], false);
}
/// A command that starts and finishes between two polls is never *seen*
/// busy, which is indistinguishable from one that never ran — so the
/// timeout has to name both doors instead of letting a finished command
/// read as "still going".
#[test]
fn wait_changed_free_says_why_it_saw_nothing_run() {
let mut backend = mock();
for _ in 0..2 {
backend.replies.push_back(ReplyOk::AgentStates(Vec::new()));
}
backend.procs_reply = idle_procs();
let out = execute(
cli(&[
"tty7",
"wait",
"%3",
"--until",
"free",
"--changed",
"--timeout",
"0",
]),
&Context::default(),
&mut backend,
)
.expect("a timeout is an exit code, not an error");
let Outcome::Exit(124, r) = out else {
panic!("a pane that was free all along has not run anything");
};
assert!(
r.human.contains("--interval") && r.human.contains("--changed"),
"the timeout should name the two ways out: {}",
r.human
);
}
/// `free` answers for a pane the agent ladder cannot, so it must not answer
/// *over* it. A pane whose depth-0 process is the agent itself reads free
/// for its whole turn; letting that outrank a `waiting` the caller asked
/// for would strand exactly the delegation loop the verb exists for.
#[test]
fn wait_free_does_not_overrule_a_state_the_caller_asked_for() {
use tty7_core::core::cli_agent::AgentStatus;
let mut backend = mock();
backend
.replies
.push_back(ReplyOk::AgentStates(vec![agent_state(
3,
AgentStatus::Waiting,
)]));
// The agent is the pane's only process, so the tree reads "free".
backend.procs_reply = idle_procs();
let json = json_of(run_cli(
&["tty7", "wait", "%3", "--until", "waiting,free"],
&Context::default(),
&mut backend,
));
assert_eq!(json["status"], "waiting", "the ladder answered first");
assert_eq!(json["matched"], true);
assert!(
backend.procs_calls.is_empty(),
"and the process tree was never asked"
);
}
/// An unreadable process tree is not an idle one. Answering `free` on an
/// empty reply would be the same false success `no-agent` was added to
/// remove, one layer down.
#[test]
fn wait_free_does_not_read_an_empty_process_tree_as_finished() {
let mut backend = mock();
backend.replies.push_back(ReplyOk::AgentStates(Vec::new()));
backend.procs_reply = tty7_core::daemon::protocol::PaneProcs::default();
let out = execute(
cli(&["tty7", "wait", "%3", "--until", "free", "--timeout", "0"]),
&Context::default(),
&mut backend,
)
.expect("a timeout is an exit code, not an error");
assert!(
matches!(out, Outcome::Exit(124, _)),
"nothing was seen, so nothing can be claimed"
);
}
/// Watching `free` must not cost anything for callers who did not ask:
/// the process tree is a second round trip per poll on top of the agent
/// snapshot, and the default wait is for agents.
+72 -3
View File
@@ -68,17 +68,36 @@ const ALIASES: &[(&str, &str)] = &[
("pgdown", "pagedown"),
];
/// What `--help` prints, and what an unknown name is answered with.
/// Every spelling `--key` accepts, in one line: what an unknown name is
/// answered with, and half of what `tty7 send --help` prints.
pub fn vocabulary() -> String {
let named: Vec<&str> = NAMED.iter().map(|(name, _)| *name).collect();
format!("{}, C-<char> (Ctrl), M-<char> (Alt)", named.join(", "))
}
/// `tty7 send --help`. Assembled from the tables above so that adding a key or
/// an alias cannot leave the help text describing the vocabulary of an older
/// build — the drift nobody notices until a caller is told a key exists and it
/// does not, or the reverse.
pub fn send_long_help() -> String {
let aliases: Vec<&str> = ALIASES.iter().map(|(from, _)| *from).collect();
format!(
"Types TEXT into the pane exactly as a keyboard would.\n\n\
--key sends a keystroke rather than characters, which is what a pane wants once \
something is already running in it: answering a prompt that only takes arrow keys, \
closing a TUI with escape, stopping a build with C-c. Repeat it for a sequence.\n\n\
Keys: {}. Aliases: {}.",
vocabulary(),
aliases.join(", ")
)
}
/// clap's `value_parser` for `--key`, so an unknown name is a usage error
/// caught before a single byte reaches the pane — sending half a key sequence
/// and then failing would leave the pane in a state nobody asked for.
pub fn parse(spelling: &str) -> Result<Key, String> {
let folded = spelling.trim().to_ascii_lowercase();
let trimmed = spelling.trim();
let folded = trimmed.to_ascii_lowercase();
let canonical = ALIASES
.iter()
.find_map(|(from, to)| (*from == folded).then_some(*to))
@@ -90,6 +109,8 @@ pub fn parse(spelling: &str) -> Result<Key, String> {
bytes: bytes.to_vec(),
});
}
// Ctrl reads the folded spelling: the C0 rule clears the top three bits, so
// C-c and C-C are the same byte and always were.
if let Some(rest) = strip_modifier(canonical, &["c-", "ctrl-", "control-"]) {
return control(rest).map(|byte| Key {
name: format!("c-{rest}"),
@@ -98,7 +119,14 @@ pub fn parse(spelling: &str) -> Result<Key, String> {
}
// Alt is a prefixed ESC — the encoding every Unix terminal has used for it
// since long before there was a modifier-reporting protocol to do better.
if let Some(rest) = strip_modifier(canonical, &["m-", "alt-", "meta-"]) {
// Which means the character rides through as itself, so unlike Ctrl this
// one has to read the spelling as written: M-X is not M-x.
// Stripped from `folded` rather than `canonical`: only that one is the
// caller's own spelling with the case knocked out of it, which is what
// makes the tail recoverable from `trimmed` by length.
if let Some(rest) =
strip_modifier(&folded, &["m-", "alt-", "meta-"]).map(|rest| as_written(trimmed, rest))
{
let mut chars = rest.chars();
return match (chars.next(), chars.next()) {
(Some(ch), None) => {
@@ -128,6 +156,15 @@ fn strip_modifier<'a>(name: &'a str, prefixes: &[&str]) -> Option<&'a str> {
.filter(|rest| !rest.is_empty())
}
/// The same tail of the spelling the caller wrote, before it was folded.
///
/// Safe to index by length: `to_ascii_lowercase` is byte-for-byte, and every
/// modifier prefix is ASCII, so a suffix of the folded form is a suffix of the
/// original at the same offset and on the same char boundary.
fn as_written<'a>(original: &'a str, folded_rest: &str) -> &'a str {
&original[original.len() - folded_rest.len()..]
}
/// The C0 control byte a Ctrl-chord produces. This is the ASCII table's own
/// rule — clear the top three bits — which is why the range runs past the
/// letters and into `[ \ ] ^ _`, and why Ctrl-? is the odd one out at 0x7f.
@@ -205,6 +242,38 @@ mod tests {
assert_eq!(bytes("alt-b"), vec![0x1b, b'b']);
}
/// Ctrl can be folded and Alt cannot: the ASCII rule throws the case away
/// either way for a control byte, while Alt carries the character through
/// as itself, so `M-X` and `M-x` are two different keys and must stay so.
#[test]
fn alt_keeps_the_case_the_caller_wrote() {
assert_eq!(bytes("M-X"), vec![0x1b, b'X']);
assert_eq!(bytes("Meta-X"), vec![0x1b, b'X']);
assert_eq!(bytes("M-x"), vec![0x1b, b'x']);
assert_eq!(parse("M-X").unwrap().name, "m-X");
// Non-ASCII rides through as its own UTF-8, and the prefix arithmetic
// must not land mid-character doing it.
assert_eq!(bytes("M-ä"), vec![0x1b, 0xc3, 0xa4]);
}
/// The help text is generated from the tables, so it cannot describe a
/// vocabulary the parser does not have. This is the assertion that the
/// generating is real rather than a second copy that happens to agree.
#[test]
fn the_help_text_lists_every_key_the_parser_takes() {
let help = send_long_help();
for (name, _) in NAMED {
assert!(help.contains(name), "`{name}` is missing from --help");
}
for (alias, _) in ALIASES {
assert!(help.contains(alias), "`{alias}` is missing from --help");
}
assert!(
help.contains("C-<char>") && help.contains("M-<char>"),
"{help}"
);
}
/// An unknown name has to fail before anything is written: a key sequence
/// half-delivered into a live pane is worse than one not delivered at all.
/// So the error names the vocabulary rather than just refusing.
+17 -1
View File
@@ -78,7 +78,17 @@ tty7 wait "$PANE" --until free --changed --timeout 900
cat /tmp/t.rc /tmp/t.log
```
`free` costs one extra request per poll, so it is only checked when you name it.
`free` costs one extra request per poll, so it is only checked when you name it
and only once the agent ladder has not already answered, so pairing it with
`waiting,done` never costs you a state you asked for.
<Warning>
`free` is read off the process tree, which has two blind spots. A pane whose
own root process *is* the command — what `tty7 run` spawns — looks free the
whole time it runs; wait on `tty7 run` itself instead, it already blocks. And
a backgrounded job (`… &`) keeps the pane busy after the foreground command
has finished.
</Warning>
<Note>
A pane with no agent reports `no-agent`, **not** `idle`. That distinction is
@@ -101,6 +111,12 @@ free ends up where it started, so there is no new state to compare against.
There `--changed` means "something ran while I was watching", which is exactly
what you want in the line after a `send`.
That does mean a command which starts *and* finishes between two polls is never
seen running, and the wait sits there until it times out. If the thing you are
waiting on can be that quick, poll faster (`--interval 100`) or drop `--changed`
and let a sentinel file carry the answer. The timeout says as much when it
happens.
## Answering a prompt
A worker that stops at `waiting` is usually showing something that keystrokes,
+7 -3
View File
@@ -161,8 +161,10 @@ by its [hooks](/agents/status); the last three are facts about the pane:
`free` is how you wait for a **command** rather than an agent, and it is the
one state that costs a second request per poll — so it is only checked when you
name it. With `--changed` it means "something ran and then finished", which is
what you want directly after a `send`.
name it, and only when none of the agent states you asked for already matched.
With `--changed` it means "something ran and then finished", which is what you
want directly after a `send`; a command quick enough to finish inside one
`--interval` is never seen running, and the timeout says so.
The reply carries the agent's message and native session id. The JSON's `stale`
flag says whether the answer might belong to the previous turn.
@@ -268,7 +270,9 @@ holds it. An interrupted `run` and a removed workspace both leave orphans here.
lists as orphaned and nothing else — panes a workspace holds are untouched —
and reports an empty list rather than an error when there is nothing to clean
up, so a script does not have to guard it. A pane that cannot be closed does
not abandon the rest of the batch; the failure is reported at the end.
not abandon the rest of the batch: the rest are still attempted, the complaint
goes to stderr, and the verb exits 1 with `{"closed":[…],"failed":[…]}` — the
list a retry needs.
<Warning>
`--orphans` closes every orphan on the machine, and an orphan can still be
+9
View File
@@ -186,6 +186,11 @@ Exit codes are built for this: `0` means a state you asked for was reached,
`124` means the timeout ran out (the `timeout(1)` convention, so "not yet" is
distinguishable from "broken"), `1` means the pane died first.
One trap in `--changed`: a command that finishes inside a single poll (500ms by
default) is never *seen* running, so the wait keeps going until it times out.
For something that quick, `--interval 100`, or drop `--changed` and read the
`.rc` file. The timeout message says so when it happens.
If you want the process tree itself — "what is running in there", "which port is
this pane serving" — that is `tty7 procs %83`: indented by depth, `*` on the
foreground process, then the ports those processes are listening on.
@@ -226,6 +231,10 @@ understanding.
a pane running a build is `no-agent`, never `idle`, so `--until idle` is never
the way to ask "is the command finished". That is `free`.
Mixing the two is safe: `--until waiting,done,free` covers a pane whose kind you
don't know, because `free` is only consulted when none of the agent states you
named matched first.
### `--changed` is not optional in a loop
The status is a **level, not an event**: `done` stands until the next turn
+14 -6
View File
@@ -202,11 +202,18 @@ Notes that decide whether a loop works:
- **`idle` is not "the command finished".** It is something an *agent* says
about itself. A pane running a build has no agent and reports `no-agent`.
Use `free` for commands.
- **`free` costs a second request per poll**, so it is only checked when named.
- **`free` costs a second request per poll**, so it is only checked when named,
and only if none of the agent states you asked for matched first — pairing
`waiting,done,free` never loses you a `waiting`.
- **`--changed` means something different for `free`**: a shell goes free →
busy → free and ends where it started, so there is no new state to compare
against. There it means "something ran while I watched" — exactly what you
want on the line after a `send`.
want on the line after a `send`. A command fast enough to finish inside one
`--interval` is never seen running, so it times out instead; use
`--interval 100` for those, or drop `--changed` and read a sentinel file.
- **`free` reads the process tree**, so a pane whose root process is the command
itself (a `tty7 run` pane) looks free while it runs, and a backgrounded job
keeps a pane busy after the foreground command is gone.
### `tty7 events`
Streams server events until interrupted, one per line — pane exits, agent
@@ -303,10 +310,11 @@ still tell a real name from a stand-in.
`orphan: true` means no workspace holds it. An interrupted `tty7 run` is what
leaves them.
`close` takes several ids at once and keeps going after a failure, reporting
what could not be closed at the end rather than abandoning the rest. `--orphans`
closes exactly what `pane ls --all` marks orphaned, and reports an empty list
instead of an error when there is nothing to do.
`close` takes several ids at once and keeps going after a failure: the rest are
still attempted, and it exits 1 with `{"closed":[...],"failed":[...]}` so you
know what is left. `--orphans` closes exactly what `pane ls --all` marks
orphaned, and reports an empty list instead of an error when there is nothing
to do.
**`--orphans` is the user's broom, not yours.** It closes every abandoned pane
on the machine, and an abandoned pane may still be running someone's command.