fix: a CLI run ends with the CLI, and list cells take pasted JSON

crates/agent_panel/src/gateway_rules.rs

@@ -544,13 +544,14 @@ mod tests {
                 Some(json!({"query": query, "name": name, "branch": AGENT})),
             )
         };
-        for held in [
+        let held_cases = [
             "query add($n: String) as commit { insert Person { name: $n } }",
             "query add() @description(\"one\") as /* held */ commit {\n insert Person { name: \"a\" }\n}",
             "query add() as unattached branch { insert Person { name: \"a\" } }",
             "query add($b: String) as unattached branch on $b { insert Person { name: \"a\" } }",
             "set x = 1;\nquery add() as unattached branch on \"01J\" { delete Person where name = \"a\" }",
-        ] {
+        ];
+        for held in held_cases {
             let refusal = mutation(held, None);
             assert!(
                 matches!(&refusal, Verdict::Refuse(text) if text.contains("as commit")),
@@ -563,17 +564,39 @@ mod tests {
         assert!(refused(&mutation(two, None)), "unnamed, either may run");
         assert!(refused(&mutation(two, Some("missing"))));
         // The words elsewhere than the head are not the clause.
-        for written in [
+        let written_cases = [
             "query keep() { insert Person { name: \"as commit\" } }",
             "// as commit\nquery keep() { insert Person { name: \"a\" } }",
             "query keep() /* as commit */ { insert Person { name: \"a\" } }",
-        ] {
+        ];
+        for written in written_cases {
             assert!(
                 matches!(mutation(written, None), Verdict::Write(_)),
                 "{written}"
             );
         }
         assert!(matches!(mutation(two, Some("keep")), Verdict::Write(_)));
+        // The compiler reads the clause too (`declared_write`, from its parse). The gateway reads
+        // tokens alone, so text the compiler cannot parse is not let through unread; wherever the
+        // compiler finds the clause the gateway refuses, and where neither does, both let the
+        // write through.
+        use gq_language::check::{Publication, declared_write};
+        let mut compiler_held = 0;
+        for held in held_cases {
+            if declared_write(held, "add").publication != Publication::Branch {
+                compiler_held += 1;
+                assert!(held_off_the_branch(held, Some("add")), "{held}");
+            }
+        }
+        assert!(compiler_held >= 3, "the compiler parses most of them");
+        for written in written_cases {
+            assert_eq!(
+                declared_write(written, "keep").publication,
+                Publication::Branch,
+                "{written}"
+            );
+            assert!(!held_off_the_branch(written, Some("keep")), "{written}");
+        }
     }
 
     #[test]

crates/gq_language/src/input.rs

@@ -75,9 +75,32 @@ pub fn edit_text(value: &Value) -> String {
 /// How an empty list is written.
 const EMPTY_LIST: &str = "[]";
 
+/// The items of a list as typed, each as text for its type to read: JSON (`["a", "b"]`, as a
+/// list is copied) when it starts with `[`, else [`list_items`]. A cell, a form field and a list
+/// parameter read one alike.
+pub fn list_texts(text: &str) -> Result<Vec<String>, String> {
+    let trimmed = text.trim();
+    if !trimmed.starts_with('[') {
+        return list_items(trimmed);
+    }
+    let value: Value =
+        serde_json::from_str(trimmed).map_err(|error| format!("Not a JSON list: {error}"))?;
+    let Value::Array(items) = value else {
+        return Err("Not a list".into());
+    };
+    items
+        .into_iter()
+        .map(|item| match item {
+            Value::String(text) => Ok(text),
+            Value::Null => Err("A list cannot hold an empty item".into()),
+            other => Ok(other.to_string()),
+        })
+        .collect()
+}
+
 /// The items of a list as typed: separated by commas, each trimmed unless quoted; a quoted item
 /// keeps its text, a doubled quote standing for one. Items left empty between commas are
-/// dropped. A cell, a list parameter and a list filter read one alike.
+/// dropped. A list filter reads one so, and [`list_texts`] reads text that is not JSON so.
 pub fn list_items(text: &str) -> Result<Vec<String>, String> {
     let mut items = Vec::new();
     let mut chars = text.chars().peekable();
@@ -365,15 +388,12 @@ pub fn parse_input(property: &Property, text: &str) -> Result<Value, String> {
         };
     }
     if property.list {
-        if text.trim() == EMPTY_LIST {
-            return Ok(Value::Array(Vec::new()));
-        }
         let element = Property {
             list: false,
             nullable: false,
             ..property.clone()
         };
-        return list_items(text)?
+        return list_texts(text)?
             .iter()
             .map(|item| parse_scalar(&element, item))
             .collect::<Result<Vec<_>, _>>()
@@ -403,7 +423,8 @@ mod tests {
 
     /// A list reads back as it was written: an item holding a comma or a quote, an empty one,
     /// one starting with `[`, and the empty list itself, apart from no value. A list parameter
-    /// reads the same text back, though it takes text starting with `[` for JSON.
+    /// reads the same text back, and both take a list as it is copied, in JSON. Before, a cell
+    /// refused the JSON a parameter took.
     #[test]
     fn a_list_reads_back_as_it_was_written() {
         let tags = Property {
@@ -438,6 +459,9 @@ mod tests {
         );
         // Typed by hand: spaces around unquoted items go, empty ones are dropped.
         assert_eq!(parse_input(&tags, " a ,, b ,"), Ok(json!(["a", "b"])));
+        // Pasted as it is copied, in JSON.
+        assert_eq!(parse_input(&tags, r#"["a", "b"]"#), Ok(json!(["a", "b"])));
+        assert!(parse_input(&tags, r#"["a", null]"#).is_err());
         assert!(parse_input(&tags, "\"open").is_err());
         assert!(parse_input(&tags, "\"a\" b").is_err());
         // A list that must hold one is emptied by clearing it.

crates/gq_language/src/params.rs

@@ -46,26 +46,10 @@ fn scalar_or_list(name: &str, type_name: &str, text: &str) -> Result<Value, Stri
 }
 
 /// A list is JSON (`["a", "b"]`) or comma-separated items (`a, b`), quoted as a cell's are
-/// when one holds a comma (`"Smith, John", x`).
+/// when one holds a comma (`"Smith, John", x`), read as a cell reads one
+/// ([`crate::input::list_texts`]).
 fn list(name: &str, item_type: &str, text: &str) -> Result<Value, String> {
-    let trimmed = text.trim();
-    if trimmed.starts_with('[') {
-        let value: Value = serde_json::from_str(trimmed)
-            .map_err(|error| format!("${name} is not a JSON list: {error}"))?;
-        let Value::Array(items) = value else {
-            return Err(format!("${name} must be a list"));
-        };
-        return items
-            .into_iter()
-            .map(|item| match item {
-                Value::String(s) => scalar(name, item_type, &s),
-                Value::Null => Err(format!("${name} cannot hold a null item")),
-                other => scalar(name, item_type, &other.to_string()),
-            })
-            .collect::<Result<Vec<_>, _>>()
-            .map(Value::Array);
-    }
-    crate::input::list_items(trimmed)
+    crate::input::list_texts(text)
         .map_err(|error| format!("${name}: {error}"))?
         .iter()
         .map(|item| scalar(name, item_type, item))

crates/graph_store/src/cli_tokens.rs

@@ -1221,6 +1221,10 @@ async fn run_with(
     }
 }
 
+/// How long a command's pipes are read after it ends: what it wrote is in them by then, and a
+/// program it started that holds them open is not waited for.
+const PIPES_AFTER_EXIT: Duration = Duration::from_millis(250);
+
 /// [`run_with`], once.
 async fn run_once(
     command: std::process::Command,
@@ -1254,18 +1258,47 @@ async fn run_once(
     let (Some(stdout), Some(stderr)) = (child.stdout.take(), child.stderr.take()) else {
         return Err(Unrun::Failed("its output could not be read".to_owned()));
     };
-    // Each pipe is closed once read to its bound, so a program that writes more stops.
-    let read = |pipe: Box<dyn futures::AsyncRead + Send + Unpin>, bound: usize| async move {
-        let mut read = Vec::new();
-        pipe.take(bound as u64).read_to_end(&mut read).await?;
-        Ok::<_, std::io::Error>(read)
-    };
     let work = async move {
-        let (written, out, err) = futures::join!(
-            write,
-            read(Box::new(stdout), limit),
-            read(Box::new(stderr), MAX_ANSWER_BYTES),
-        );
+        // Each pipe is closed once read to its bound, so a program that writes more stops.
+        let (mut stdout, mut stderr) = (
+            stdout.take(limit as u64),
+            stderr.take(MAX_ANSWER_BYTES as u64),
+        );
+        let (mut out, mut err) = (Vec::new(), Vec::new());
+        let (written, exited) = {
+            let reads = futures::future::join3(
+                write,
+                stdout.read_to_end(&mut out),
+                stderr.read_to_end(&mut err),
+            )
+            .fuse();
+            let exit = child.status().fuse();
+            futures::pin_mut!(reads, exit);
+            futures::select! {
+                (written, read_out, read_err) = reads => {
+                    read_out.map_err(|error| error.to_string())?;
+                    read_err.map_err(|error| error.to_string())?;
+                    (written, None)
+                }
+                // Ended: what it wrote is in its pipes, which a program it started may hold
+                // open after it (a credential helper's background process). They are read a
+                // moment more, not until that program ends, which would hold the CLI's turn to
+                // the timeout.
+                status = exit => {
+                    let grace = smol::Timer::after(PIPES_AFTER_EXIT).fuse();
+                    futures::pin_mut!(grace);
+                    let written = futures::select! {
+                        (written, read_out, read_err) = reads => {
+                            read_out.map_err(|error| error.to_string())?;
+                            read_err.map_err(|error| error.to_string())?;
+                            written
+                        }
+                        _ = grace => Ok(()),
+                    };
+                    (written, Some(status))
+                }
+            }
+        };
         // A command that stopped reading its input (it exited, refused a lock) says why in its
         // output and how it ended: those decide, not the pipe it closed.
         written
@@ -1274,11 +1307,11 @@ async fn run_once(
                 _ => Err(error),
             })
             .map_err(|error| error.to_string())?;
-        let (out, err) = (
-            out.map_err(|error| error.to_string())?,
-            err.map_err(|error| error.to_string())?,
-        );
-        let status = child.status().await.map_err(|error| error.to_string())?;
+        let status = match exited {
+            Some(status) => status,
+            None => child.status().await,
+        }
+        .map_err(|error| error.to_string())?;
         Ok::<_, String>(Ran {
             status,
             stdout: out,
@@ -2432,6 +2465,35 @@ exit 64
         };
         assert!(!ran.status.success());
         assert!(String::from_utf8_lossy(&ran.stderr).contains("lock refused"));
+
+        // One that leaves a program holding its output open (a credential helper's background
+        // process) is done when it ends, with what it wrote. Before, the run waited for that
+        // program, holding the CLI's turn up to the timeout.
+        let leaving = dir.join("leaving");
+        std::fs::write(
+            &leaving,
+            "#!/bin/sh
+(sleep 5) &
+echo token
+",
+        )
+        .unwrap();
+        std::fs::set_permissions(&leaving, std::fs::Permissions::from_mode(0o755)).unwrap();
+        let started = std::time::Instant::now();
+        let ran = smol::block_on(run_once(
+            std::process::Command::new(&leaving),
+            None,
+            Duration::from_secs(10),
+            1024,
+        ))
+        .unwrap_or_else(|_| panic!("the command's own answer"));
+        assert!(
+            started.elapsed() < Duration::from_secs(3),
+            "{:?}",
+            started.elapsed()
+        );
+        assert!(ran.status.success());
+        assert_eq!(String::from_utf8_lossy(&ran.stdout), "token\n");
         std::fs::remove_dir_all(&dir).ok();
     }