fix: wait for new pane shell readiness (#2774)

* fix: wait for new pane shell readiness

refs #2773

* refactor: share agent startup bounds

refs #2773

* fix: reconcile agent names after startup timeout

refs #2773

* fix: cover agent cleanup scheduling skew

refs #2773

* fix: bound pane shell readiness retries

refs #2773

---------

Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: Ogulcan Celik <ogulcancelik@gmail.com>

2 files changed, 315 insertions, 18 deletions

diff --git a/tests/cli/agent_transport.rs b/tests/cli/agent_transport.rs
index 85419e90..a052f697 100644
--- a/tests/cli/agent_transport.rs
+++ b/tests/cli/agent_transport.rs
@@ -8,6 +8,24 @@ fn agent_start_waits_through_unknown_then_rejects_blocked() {
     let listener = UnixListener::bind(&socket_path).unwrap();
 
     let server = thread::spawn(move || {
+        let (mut pane_stream, pane_line) = accept_fake_cli_operation(&listener);
+        let pane: serde_json::Value = serde_json::from_str(&pane_line).unwrap();
+        assert_eq!(pane["method"], "pane.get");
+        assert_eq!(pane["params"]["pane_id"], "w1:p1");
+        writeln!(
+            pane_stream,
+            "{}",
+            serde_json::json!({
+                "id": pane["id"],
+                "result": {
+                    "type": "pane_info",
+                    "pane": { "terminal_id": "term_1" }
+                }
+            })
+        )
+        .unwrap();
+        pane_stream.flush().unwrap();
+
         let (mut start_stream, start_line) = accept_fake_cli_operation(&listener);
         let start: serde_json::Value = serde_json::from_str(&start_line).unwrap();
         assert_eq!(start["method"], "agent.start");
@@ -99,6 +117,106 @@ fn agent_start_waits_through_unknown_then_rejects_blocked() {
 }
 
 #[test]
+fn agent_start_does_not_retry_after_the_target_terminal_changes() {
+    let base = unique_test_dir();
+    fs::create_dir_all(&base).unwrap();
+    let socket_path = base.join("herdr.sock");
+    let listener = UnixListener::bind(&socket_path).unwrap();
+
+    let server = thread::spawn(move || {
+        for _ in 0..2 {
+            let (mut stream, line) = accept_fake_cli_operation(&listener);
+            let request: serde_json::Value = serde_json::from_str(&line).unwrap();
+            assert_eq!(request["method"], "pane.get");
+            writeln!(
+                stream,
+                "{}",
+                serde_json::json!({
+                    "id": request["id"],
+                    "result": {
+                        "type": "pane_info",
+                        "pane": { "terminal_id": "term_1" }
+                    }
+                })
+            )
+            .unwrap();
+            stream.flush().unwrap();
+
+            let (mut stream, line) = accept_fake_cli_operation(&listener);
+            let request: serde_json::Value = serde_json::from_str(&line).unwrap();
+            if request["method"] == "agent.start" {
+                writeln!(
+                    stream,
+                    "{}",
+                    serde_json::json!({
+                        "id": request["id"],
+                        "error": {
+                            "code": "agent_pane_busy",
+                            "message": "agent target pane w1:p1 is not an available shell"
+                        }
+                    })
+                )
+                .unwrap();
+                stream.flush().unwrap();
+            } else {
+                assert_eq!(request["method"], "pane.process_info");
+                writeln!(
+                    stream,
+                    "{}",
+                    serde_json::json!({
+                        "id": request["id"],
+                        "result": {
+                            "type": "pane_process_info",
+                            "process_info": {
+                                "pane_id": "w1:p1",
+                                "shell_pid": 10,
+                                "foreground_process_group_id": 10,
+                                "foreground_processes": [
+                                    { "pid": 10, "name": "bash" },
+                                    { "pid": 11, "name": "startup-helper" }
+                                ]
+                            }
+                        }
+                    })
+                )
+                .unwrap();
+                stream.flush().unwrap();
+            }
+        }
+
+        let (mut stream, line) = accept_fake_cli_operation(&listener);
+        let request: serde_json::Value = serde_json::from_str(&line).unwrap();
+        assert_eq!(request["method"], "pane.get");
+        writeln!(
+            stream,
+            "{}",
+            serde_json::json!({
+                "id": request["id"],
+                "result": {
+                    "type": "pane_info",
+                    "pane": { "terminal_id": "term_2" }
+                }
+            })
+        )
+        .unwrap();
+        stream.flush().unwrap();
+    });
+
+    let started = run_cli(
+        &socket_path,
+        &[
+            "agent", "start", "reviewer", "--kind", "pi", "--pane", "w1:p1",
+        ],
+    );
+    assert_eq!(started.status.code(), Some(1));
+    let error: serde_json::Value = serde_json::from_slice(&started.stderr).unwrap();
+    assert_eq!(error["error"]["code"], "agent_pane_busy");
+
+    server.join().unwrap();
+    cleanup_test_base(&base);
+}
+
+#[test]
 fn prompt_wait_is_sent_as_one_agent_request() {
     let base = unique_test_dir();
     fs::create_dir_all(&base).unwrap();
diff --git a/tests/cli/agents.rs b/tests/cli/agents.rs
index 37c8d557..f5affc87 100644
--- a/tests/cli/agents.rs
+++ b/tests/cli/agents.rs
@@ -1,5 +1,183 @@
 use super::harness::*;
 
+fn write_delayed_shell_and_fake_pi(
+    base: &Path,
+    shell_delay_seconds: &str,
+) -> (PathBuf, PathBuf, PathBuf) {
+    use std::os::unix::fs::PermissionsExt;
+
+    let bin = base.join("bin");
+    let delayed_shell = bin.join("delayed-shell");
+    let fake_pi = bin.join("pi");
+    let invocations = base.join("pi-invocations");
+    fs::create_dir_all(&bin).unwrap();
+    fs::write(
+        &delayed_shell,
+        format!("#!/bin/sh\n/bin/sleep {shell_delay_seconds}\nexec /bin/sh\n"),
+    )
+    .unwrap();
+    fs::write(
+        &fake_pi,
+        format!(
+            "#!/bin/sh\nprintf '%s\\n' \"$@\" >> '{}'\nexport HERDR_AGENT=pi\n'{}' pane report-agent \"$HERDR_PANE_ID\" --source custom:delayed-shell-pi --agent pi --state idle >/dev/null\nwhile IFS= read -r _prompt; do :; done\n",
+            invocations.display(),
+            env!("CARGO_BIN_EXE_herdr"),
+        ),
+    )
+    .unwrap();
+    fs::set_permissions(&delayed_shell, fs::Permissions::from_mode(0o755)).unwrap();
+    fs::set_permissions(&fake_pi, fs::Permissions::from_mode(0o755)).unwrap();
+    (bin, delayed_shell, invocations)
+}
+
+#[test]
+fn agent_start_waits_for_a_new_pane_shell_to_finish_initializing() {
+    let base = unique_test_dir();
+    let config_home = base.join("config");
+    let runtime_dir = base.join("runtime");
+    let socket_path = runtime_dir.join("herdr.sock");
+    let (bin, delayed_shell, invocations) = write_delayed_shell_and_fake_pi(&base, "0.4");
+    let config = format!(
+        "onboarding = false\n[terminal]\ndefault_shell = {:?}\nshell_mode = \"non_login\"\n",
+        delayed_shell.to_str().unwrap()
+    );
+    let herdr = spawn_herdr_with_config(
+        &config_home,
+        &runtime_dir,
+        &socket_path,
+        Some(&bin),
+        &config,
+    );
+    wait_for_socket(&socket_path, Duration::from_secs(5));
+    let seed = run_cli_json(
+        &socket_path,
+        &["workspace", "create", "--cwd", base.to_str().unwrap()],
+    );
+    let seed_workspace = seed["result"]["workspace"]["workspace_id"]
+        .as_str()
+        .unwrap();
+    let created = run_cli_json(
+        &socket_path,
+        &[
+            "workspace",
+            "create",
+            "--cwd",
+            base.to_str().unwrap(),
+            "--no-focus",
+        ],
+    );
+    let pane_id = created["result"]["root_pane"]["pane_id"].as_str().unwrap();
+    let terminal_id = created["result"]["root_pane"]["terminal_id"]
+        .as_str()
+        .unwrap();
+    assert!(!created["result"]["root_pane"]["focused"].as_bool().unwrap());
+
+    let started = run_cli_json(
+        &socket_path,
+        &[
+            "agent",
+            "start",
+            "worker",
+            "--kind",
+            "pi",
+            "--pane",
+            pane_id,
+            "--timeout",
+            "8000",
+            "--",
+            "--no-context-files",
+            "--no-skills",
+            "--no-extensions",
+        ],
+    );
+
+    assert_eq!(started["result"]["agent"]["terminal_id"], terminal_id);
+    assert_eq!(started["result"]["agent"]["pane_id"], pane_id);
+    assert_eq!(started["result"]["agent"]["interactive_ready"], true);
+    assert_eq!(
+        fs::read_to_string(&invocations).unwrap(),
+        "--no-context-files\n--no-skills\n--no-extensions\n"
+    );
+    assert_eq!(
+        run_cli_json(&socket_path, &["workspace", "list"])["result"]["workspaces"]
+            .as_array()
+            .unwrap()
+            .iter()
+            .find(|workspace| workspace["workspace_id"] == seed_workspace)
+            .unwrap()["focused"],
+        true
+    );
+
+    cleanup_spawned_herdr(herdr, base);
+}
+
+#[test]
+fn agent_start_stops_retrying_when_the_pane_shell_stays_busy() {
+    let base = unique_test_dir();
+    let config_home = base.join("config");
+    let runtime_dir = base.join("runtime");
+    let socket_path = runtime_dir.join("herdr.sock");
+    let (bin, delayed_shell, invocations) = write_delayed_shell_and_fake_pi(&base, "2.3");
+    let config = format!(
+        "onboarding = false\n[terminal]\ndefault_shell = {:?}\nshell_mode = \"non_login\"\n",
+        delayed_shell.to_str().unwrap()
+    );
+    let herdr = spawn_herdr_with_config(
+        &config_home,
+        &runtime_dir,
+        &socket_path,
+        Some(&bin),
+        &config,
+    );
+    wait_for_socket(&socket_path, Duration::from_secs(5));
+    let created = run_cli_json(
+        &socket_path,
+        &["workspace", "create", "--cwd", base.to_str().unwrap()],
+    );
+    let pane_id = created["result"]["root_pane"]["pane_id"].as_str().unwrap();
+
+    let started_at = Instant::now();
+    let unavailable = run_cli(
+        &socket_path,
+        &[
+            "agent",
+            "start",
+            "worker",
+            "--kind",
+            "pi",
+            "--pane",
+            pane_id,
+            "--timeout",
+            "8000",
+        ],
+    );
+    assert_eq!(unavailable.status.code(), Some(1));
+    let error: serde_json::Value = serde_json::from_slice(&unavailable.stderr).unwrap();
+    assert_eq!(error["error"]["code"], "agent_pane_busy");
+    assert!(started_at.elapsed() >= Duration::from_secs(2));
+    assert!(started_at.elapsed() < Duration::from_secs(4));
+    assert!(!invocations.exists());
+
+    let retried = run_cli_json(
+        &socket_path,
+        &[
+            "agent",
+            "start",
+            "worker",
+            "--kind",
+            "pi",
+            "--pane",
+            pane_id,
+            "--timeout",
+            "8000",
+        ],
+    );
+    assert_eq!(retried["result"]["type"], "agent_started");
+    assert_eq!(fs::read_to_string(&invocations).unwrap(), "\n");
+
+    cleanup_spawned_herdr(herdr, base);
+}
+
 #[test]
 fn agent_start_command_works() {
     use std::os::unix::fs::PermissionsExt;
@@ -75,7 +253,7 @@ fn agent_start_command_works() {
         assert_eq!(error["error"]["code"], "invalid_agent_argument");
     }
 
-    for invalid_timeout in ["3000", "300001"] {
+    for invalid_timeout in ["3000", "300001", "18446744073709551615"] {
         let rejected = run_cli(
             &socket_path,
             &[
@@ -259,7 +437,7 @@ fn agent_start_rejects_a_shell_replaced_by_a_foreground_program() {
             "--pane",
             &pane_id,
             "--timeout",
-            "1000",
+            "4000",
         ],
     );
     assert_eq!(started.status.code(), Some(1));
@@ -310,6 +488,23 @@ fn agent_start_timeout_releases_the_name_for_reuse() {
         .unwrap()
         .to_string();
 
+    assert!(run_cli(
+        &socket_path,
+        &[
+            "pane",
+            "report-agent",
+            &reuse_pane_id,
+            "--source",
+            "custom:reuse",
+            "--agent",
+            "pi",
+            "--state",
+            "idle",
+        ],
+    )
+    .status
+    .success());
+
     let started = run_cli(
         &socket_path,
         &[
@@ -328,22 +523,6 @@ fn agent_start_timeout_releases_the_name_for_reuse() {
     let error: serde_json::Value = serde_json::from_slice(&started.stderr).unwrap();
     assert_eq!(error["error"]["code"], "timeout");
 
-    assert!(run_cli(
-        &socket_path,
-        &[
-            "pane",
-            "report-agent",
-            &reuse_pane_id,
-            "--source",
-            "custom:reuse",
-            "--agent",
-            "pi",
-            "--state",
-            "idle",
-        ],
-    )
-    .status
-    .success());
     let reused = run_cli(&socket_path, &["agent", "rename", &reuse_pane_id, "worker"]);
     assert!(
         reused.status.success(),