Merge pull request #6774 from Hmbown/fix/bh2-cli-and-npm-wrapper

fix(cli,npm): exit codes, hidden key prompt, config and alias routing, wrapper signals and download timeouts

crates/app-server/src/lib.rs

@@ -1359,8 +1359,8 @@ async fn handle_stdio_thread_message<W: AsyncWrite + Unpin>(
     Ok(result)
 }
 
-/// Resuming or forking a thread the runtime reports as `missing` must fail
-/// with a named not-found error. Recording the null model/workspace of that
+/// Resuming, forking, archiving or unarchiving a thread the runtime reports
+/// as `missing` must fail with a named not-found error. Recording the null model/workspace of that
 /// response as a stdio hint would clobber any previously cached hint for
 /// the same thread id (#5171).
 fn ensure_thread_found(response: &ThreadResponse) -> std::result::Result<(), JsonRpcError> {
@@ -2316,6 +2316,7 @@ async fn dispatch_stdio_request_with_writer<W: AsyncWrite + Unpin>(
                 },
             )
             .await?;
+            ensure_thread_found(&response)?;
             StdioDispatchResult {
                 result: serde_json::to_value(response)
                     .map_err(|err| JsonRpcError::internal(err.to_string()))?,
@@ -2331,6 +2332,7 @@ async fn dispatch_stdio_request_with_writer<W: AsyncWrite + Unpin>(
                 },
             )
             .await?;
+            ensure_thread_found(&response)?;
             StdioDispatchResult {
                 result: serde_json::to_value(response)
                     .map_err(|err| JsonRpcError::internal(err.to_string()))?,
@@ -3538,6 +3540,26 @@ mod tests {
         assert_eq!(hint.workspace.as_deref(), Some(workspace.as_path()));
     }
 
+    #[tokio::test]
+    async fn stdio_archive_of_missing_thread_fails_instead_of_reporting_success() {
+        let tmp = tempfile::tempdir().expect("tempdir");
+        let config_path = tmp.path().join("config.toml");
+        fs::write(&config_path, "").expect("write config");
+        let state = build_state(Some(config_path), None).expect("state");
+        for method in ["thread/archive", "thread/unarchive"] {
+            let err =
+                dispatch_stdio_request(&state, method, json!({ "thread_id": "ghost-thread" }))
+                    .await
+                    .expect_err("an unknown thread id must fail");
+            assert_eq!(err.code, -32004, "{method}");
+            assert!(
+                err.message.contains("ghost-thread"),
+                "{method}: {}",
+                err.message
+            );
+        }
+    }
+
     fn sse_frame(event: &str, payload: Value) -> String {
         format!("event: {event}\ndata: {payload}\n\n")
     }

crates/cli/src/lib.rs

@@ -829,7 +829,8 @@ struct LaneStartRequest {
 
 fn start_lane(request: LaneStartRequest) -> Result<()> {
     use codewhale_lane::{
-        LaneRegistry, LaneStartSpec, RuntimeBackendKind, WorktreeProvision, resolve_backend,
+        LaneRegistry, LaneStartSpec, LaneStatus, RuntimeBackendKind, WorktreeProvision,
+        resolve_backend,
     };
 
     let LaneStartRequest {
@@ -890,6 +891,18 @@ fn start_lane(request: LaneStartRequest) -> Result<()> {
     if let Some(attach) = backend.attach_command(&record) {
         println!("attach:  {attach}");
     }
+    // The inline runtime runs the command to completion inside `start`, so its
+    // terminal status is final here. A lane that did not complete must fail
+    // the command, or `workflow run --runtime inline` gates in CI pass on a
+    // failed run. Tmux lanes are still running, so their status says nothing.
+    if kind == RuntimeBackendKind::Inline && record.status != LaneStatus::Completed {
+        bail!(
+            "lane {} {} (log: {})",
+            record.id,
+            record.status.as_str(),
+            record.log_path.display()
+        );
+    }
     Ok(())
 }
 
@@ -1761,7 +1774,7 @@ enum ModelCommand {
         #[arg(long, value_parser = parse_catalog_route)]
         provider: Option<ProviderKind>,
     },
-    /// Set the default model (e.g. "pro", "flash", "deepseek-v4-pro").
+    /// Set the default model (e.g. "deepseek-v4-pro"; "pro"/"flash" on routes that serve DeepSeek).
     Set { model: String },
 }
 
@@ -2438,12 +2451,14 @@ fn run() -> Result<()> {
         }
         Some(Commands::Sandbox(args)) => run_sandbox_command(args.command),
         Some(Commands::AppServer(args)) => {
-            // The HTTP/mobile runtime API is delegated to the mature `serve` path
-            // in the TUI binary, which reads the *global* --config. app-server has
-            // historically taken a subcommand-level --config, so bridge it before
-            // resolving runtime options (provider/keyring) for the delegated run.
-            if (args.http || args.mobile) && cli.config.is_none() && args.config.is_some() {
-                cli.config = args.config.clone();
+            // Every transport loads the same file: the subcommand's --config,
+            // else the global one. The HTTP/mobile runtime API is delegated to
+            // the `serve` path in the TUI binary, which reads only the *global*
+            // --config, and runtime options (provider/keyring) resolve from it
+            // too, so bridge the choice there before resolving them.
+            let config_path = app_server_config_path(&cli, &args);
+            if config_path != cli.config {
+                cli.config = config_path;
                 store = ConfigStore::load(cli.config.clone())?;
             }
             let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides);
@@ -2842,31 +2857,33 @@ fn no_keyring_secrets() -> Secrets {
     ))
 }
 
+/// Clear one provider's credential. `Ok(Some(message))` means the config leg
+/// was saved but the secret store kept the key; the caller must exit non-zero.
 fn clear_auth_provider(
     store: &mut ConfigStore,
     secrets: &Secrets,
     provider: ProviderKind,
-) -> Result<()> {
+) -> Result<Option<String>> {
     if provider == ProviderKind::Antigravity {
-        return clear_legacy_antigravity_config(store, secrets);
+        return clear_legacy_antigravity_config(store, secrets).map(|()| None);
     }
     let outcome = codewhale_config::credentials::clear_provider_api_key(store, secrets, provider)?;
     let slot = outcome.slot;
     // The secret-store leg used to fail silently here, which meant `auth clear`
-    // could print success while the key was still in the keyring. Say so
-    // instead; the config no longer advertises a key the backend may hold.
+    // could print success while the key was still in the keyring. The config
+    // no longer advertises a key the backend may hold, but the credential is
+    // not revoked, so this is a failure rather than a note on stdout.
     if let Some(error) = &outcome.secret_store_error {
-        println!(
-            "cleared API key for {slot} from config, but the secret store refused the delete: {error}"
-        );
-        return Ok(());
+        return Ok(Some(format!(
+            "cleared API key for {slot} from config, but the secret store refused the delete: {error}; the key may still be stored there"
+        )));
     }
     if provider == ProviderKind::Xai {
         println!("cleared xAI credentials from config, secret store, and owned OAuth storage");
     } else {
         println!("cleared API key for {slot} from config and secret store");
     }
-    Ok(())
+    Ok(None)
 }
 
 /// Remove only Codewhale-owned state for the retired Antigravity route.
@@ -4428,7 +4445,7 @@ fn run_auth_command_with_secrets_and_runtime(
             let api_key = match (api_key, api_key_stdin) {
                 (Some(v), _) => v,
                 (None, true) => read_api_key_from_stdin()?,
-                (None, false) => prompt_api_key(slot)?,
+                (None, false) => prompt_api_key(provider)?,
             };
             let mut credential_store =
                 codewhale_config::credentials::credential_metadata_store(store)?;
@@ -4467,13 +4484,20 @@ fn run_auth_command_with_secrets_and_runtime(
             })
         }
         AuthCommand::Clear { provider } => {
-            if provider == ProviderKind::Xai {
+            let incomplete = if provider == ProviderKind::Xai {
                 codewhale_config::with_xai_oauth_revocation_transaction(|| {
                     clear_auth_provider(store, secrets, provider)
-                })
+                })?
             } else {
-                clear_auth_provider(store, secrets, provider)
+                clear_auth_provider(store, secrets, provider)?
+            };
+            // Reported after the xAI transaction commits: the config leg is
+            // saved, so failing inside it would roll back a revocation that
+            // already happened.
+            if let Some(message) = incomplete {
+                bail!(message);
             }
+            Ok(())
         }
         AuthCommand::List => {
             for line in auth_list_lines_with_runtime(store, secrets, runtime_overrides) {
@@ -4569,18 +4593,71 @@ fn keyring_status_short(state: Option<bool>) -> &'static str {
     }
 }
 
-fn prompt_api_key(slot: &str) -> Result<String> {
-    use std::io::{IsTerminal, Write};
-    eprint!("Enter API key for {slot}: ");
-    io::stderr().flush().ok();
-    if !io::stdin().is_terminal() {
-        // Non-interactive: read directly without prompting twice.
-        return read_api_key_from_stdin();
+fn prompt_api_key(provider: ProviderKind) -> Result<String> {
+    use std::io::IsTerminal;
+    read_prompted_api_key(
+        provider.as_str(),
+        io::stdin().is_terminal(),
+        |prompt| {
+            // The help promises the key is not echoed: a plain `read_line`
+            // would leave it on screen, in scrollback, and in recordings.
+            // `read_secure_line` returns "" on a stream that is not a
+            // terminal, so prompt on whichever of stderr/stdout is one.
+            let term = match hidden_prompt_stream(
+                io::stderr().is_terminal(),
+                io::stdout().is_terminal(),
+            ) {
+                Some(PromptStream::Stderr) => console::Term::stderr(),
+                Some(PromptStream::Stdout) => console::Term::stdout(),
+                None => {
+                    return Err(io::Error::other(
+                        "both stdout and stderr are redirected, so the key cannot be read \
+                         without echo; pipe it on stdin instead",
+                    ));
+                }
+            };
+            term.write_str(prompt)?;
+            // Ends the line itself once the key is read.
+            term.read_secure_line()
+        },
+        read_api_key_from_stdin,
+    )
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum PromptStream {
+    Stderr,
+    Stdout,
+}
+
+fn hidden_prompt_stream(
+    stderr_is_terminal: bool,
+    stdout_is_terminal: bool,
+) -> Option<PromptStream> {
+    if stderr_is_terminal {
+        Some(PromptStream::Stderr)
+    } else if stdout_is_terminal {
+        Some(PromptStream::Stdout)
+    } else {
+        None
     }
-    let mut buf = String::new();
-    io::stdin()
-        .read_line(&mut buf)
-        .context("failed to read API key from stdin")?;
+}
+
+fn read_prompted_api_key(
+    provider_id: &str,
+    stdin_is_terminal: bool,
+    read_hidden_line: impl FnOnce(&str) -> io::Result<String>,
+    read_piped: impl FnOnce() -> Result<String>,
+) -> Result<String> {
+    use std::io::Write;
+    let prompt = format!("Enter API key for {provider_id}: ");
+    if !stdin_is_terminal {
+        // Non-interactive: read directly without prompting twice.
+        eprint!("{prompt}");
+        io::stderr().flush().ok();
+        return read_piped();
+    }
+    let buf = read_hidden_line(&prompt).context("failed to read API key from the terminal")?;
     let key = buf.trim().to_string();
     if key.is_empty() {
         bail!("empty API key provided");
@@ -5119,6 +5196,39 @@ fn canonical_model_for_set(model: &str) -> &str {
     }
 }
 
+/// The provider (and its endpoint) whose model `model set` writes.
+///
+/// A home config gets `[providers.<saved route>] model`, so the saved route in
+/// that file decides. A workspace-scoped config gets a root `model` that
+/// applies to whatever route is in effect, which that file alone may not name.
+fn model_set_route(
+    store: &ConfigStore,
+    resolved_runtime: &ResolvedRuntimeOptions,
+) -> Result<Option<(ProviderKind, String)>> {
+    if codewhale_config::config_path_is_workspace_scoped(store.path()) {
+        return Ok(Some((
+            resolved_runtime.provider,
+            resolved_runtime.base_url.clone(),
+        )));
+    }
+    let (route, _, _) = codewhale_tui::route_preferences::selected_route(store.path())?;
+    Ok(ProviderKind::parse_config_identity(&route).map(|provider| {
+        let base_url = store
+            .config
+            .providers
+            .for_provider(provider)
+            .base_url
+            .clone()
+            .filter(|base| !base.trim().is_empty())
+            .unwrap_or_else(|| {
+                codewhale_config::provider::provider_for_kind(provider)
+                    .default_base_url()
+                    .to_string()
+            });
+        (provider, base_url)
+    }))
+}
+
 fn run_model_command(
     store: &mut ConfigStore,
     command: ModelCommand,
@@ -5229,7 +5339,21 @@ fn run_model_command(
             if trimmed.is_empty() {
                 bail!("Model name cannot be empty");
             }
-            let canonical = canonical_model_for_set(trimmed);
+            // The short names are DeepSeek's. They expand wherever a DeepSeek
+            // id is servable (DeepSeek itself and the hosts that serve its
+            // models, such as OpenRouter or Together). On a vendor that only
+            // serves its own family (OpenAI, Anthropic, ...) `pro` is that
+            // vendor's own name, so it is stored as typed.
+            let expanded = canonical_model_for_set(trimmed);
+            let canonical = if expanded != trimmed
+                && model_set_route(store, resolved_runtime)?.is_some_and(|(provider, base_url)| {
+                    codewhale_config::known_foreign_model_owner(provider, expanded, &base_url)
+                        .is_some()
+                }) {
+                trimmed
+            } else {
+                expanded
+            };
             codewhale_tui::route_preferences::set(store.path(), "model", canonical)?;
             store.reload()?;
             println!("Default model set to '{canonical}'");
@@ -5275,7 +5399,15 @@ fn run_thread_command(
         let resolved_runtime = resolve_runtime_for_dispatch(store, runtime_overrides);
         return run_tui_in_process(cli, &resolved_runtime, passthrough);
     }
-    let state = StateStore::open(None)?;
+    run_thread_store_command(&StateStore::open(None)?, command)
+}
+
+fn run_thread_store_command(state: &StateStore, command: ThreadCommand) -> Result<()> {
+    let require_thread = |thread_id: &str| -> Result<codewhale_state::ThreadMetadata> {
+        state
+            .get_thread(thread_id)?
+            .with_context(|| format!("thread not found: {thread_id}"))
+    };
     match command {
         ThreadCommand::List { all, limit } => {
             let threads = state.list_threads(ThreadListFilters {
@@ -5297,27 +5429,29 @@ fn run_thread_command(
             Ok(())
         }
         ThreadCommand::Read { thread_id } => {
-            let thread = state.get_thread(&thread_id)?;
+            let thread = require_thread(&thread_id)?;
             println!("{}", serde_json::to_string_pretty(&thread)?);
             Ok(())
         }
         ThreadCommand::Resume { .. } | ThreadCommand::Fork { .. } => {
             unreachable!("thread_delegation routes resume and fork before this match")
         }
+        // The store updates by id and reports nothing for an unknown one, so
+        // check first rather than print success for a no-op.
         ThreadCommand::Archive { thread_id } => {
+            require_thread(&thread_id)?;
             state.mark_archived(&thread_id)?;
             println!("archived {thread_id}");
             Ok(())
         }
         ThreadCommand::Unarchive { thread_id } => {
+            require_thread(&thread_id)?;
             state.mark_unarchived(&thread_id)?;
             println!("unarchived {thread_id}");
             Ok(())
         }
         ThreadCommand::SetName { thread_id, name } => {
-            let mut thread = state
-                .get_thread(&thread_id)?
-                .with_context(|| format!("thread not found: {thread_id}"))?;
+            let mut thread = require_thread(&thread_id)?;
             thread.name = Some(name);
             thread.updated_at = chrono::Utc::now().timestamp();
             state.upsert_thread(&thread)?;
@@ -5325,9 +5459,7 @@ fn run_thread_command(
             Ok(())
         }
         ThreadCommand::ClearName { thread_id } => {
-            let mut thread = state
-                .get_thread(&thread_id)?
-                .with_context(|| format!("thread not found: {thread_id}"))?;
+            let mut thread = require_thread(&thread_id)?;
             thread.name = None;
             thread.updated_at = chrono::Utc::now().timestamp();
             state.upsert_thread(&thread)?;
@@ -5378,11 +5510,11 @@ fn run_app_server_command(
     // Everything below runs the app-server *in this process*, which is why the
     // surface cannot be derived from the executable: `current_exe()` would
     // report every one of these sessions as `cli`.
-    let session = start_cli_telemetry(
-        resolved_runtime,
-        args.config.clone().or_else(|| cli.config.clone()),
-        Surface::AppServer,
-    );
+    //
+    // `codewhale --config X app-server --stdio` must load X too: the global
+    // flag is the fallback for every in-process transport, not only telemetry.
+    let config_path = app_server_config_path(cli, &args);
+    let session = start_cli_telemetry(resolved_runtime, config_path.clone(), Surface::AppServer);
 
     let runtime = match tokio::runtime::Builder::new_multi_thread()
         .enable_all()
@@ -5397,14 +5529,14 @@ fn run_app_server_command(
         }
     };
     if args.stdio {
-        let outcome = runtime.block_on(run_app_server_stdio(args.config));
+        let outcome = runtime.block_on(run_app_server_stdio(config_path));
         finish_cli_telemetry(session, &outcome);
         return outcome;
     }
     if args.socket {
         let outcome = runtime.block_on(run_daemon_socket(DaemonSocketOptions {
             socket_path: args.socket_path,
-            config_path: args.config,
+            config_path,
         }));
         finish_cli_telemetry(session, &outcome);
         return outcome;
@@ -5423,7 +5555,7 @@ fn run_app_server_command(
         .and_then(|listen| {
             runtime.block_on(run_app_server(AppServerOptions {
                 listen,
-                config_path: args.config,
+                config_path,
                 auth_token: args.auth_token.or_else(app_server_token_from_env),
                 insecure_no_auth: args.insecure_no_auth,
                 cors_origins: args.cors_origin,
@@ -5433,6 +5565,12 @@ fn run_app_server_command(
     outcome
 }
 
+/// The config file an in-process app-server loads: the subcommand's own
+/// `--config`, else the global one.
+fn app_server_config_path(cli: &Cli, args: &AppServerArgs) -> Option<PathBuf> {
+    args.config.clone().or_else(|| cli.config.clone())
+}
+
 /// Build the `serve` argv forwarded to the TUI binary for
 /// `codewhale app-server --http`/`--mobile`. Maps app-server flags onto the
 /// matching `serve` flags (note `--insecure-no-auth` → `--insecure`). The
@@ -6995,6 +7133,323 @@ verbosity = "concise"
         );
     }
 
+    #[test]
+    fn model_set_keeps_short_names_literal_off_deepseek_routes() {
+        let _env = env_lock();
+        let home = tempfile::tempdir().expect("isolated home");
+        let _home = ScopedEnvVar::set("CODEWHALE_HOME", &home.path().to_string_lossy());
+        let _config_path = ScopedEnvVar::remove("CODEWHALE_CONFIG_PATH");
+        let _legacy_config_path = ScopedEnvVar::remove("DEEPSEEK_CONFIG_PATH");
+        let path = home.path().join("config.toml");
+        for (provider, expected) in [
+            (ProviderKind::Openai, "pro"),
+            (ProviderKind::Anthropic, "pro"),
+            (ProviderKind::Deepseek, "deepseek-v4-pro"),
+            // Hosts that serve DeepSeek but do not resolve `pro` themselves.
+            (ProviderKind::Openrouter, "deepseek-v4-pro"),
+            (ProviderKind::Together, "deepseek-v4-pro"),
+        ] {
+            std::fs::write(&path, format!("provider = \"{}\"\n", provider.as_str())).unwrap();
+            let mut store = ConfigStore::load(Some(path.clone())).unwrap();
+            let runtime = resolved_runtime_for_test(provider, ProviderSource::Config);
+            run_model_command(
+                &mut store,
+                ModelCommand::Set {
+                    model: "pro".into(),
+                },
+                None,
+                &runtime,
+            )
+            .unwrap();
+            assert_eq!(store.config.provider, provider);
+            assert_eq!(
+                store
+                    .config
+                    .providers
+                    .for_provider(provider)
+                    .model
+                    .as_deref(),
+                Some(expected),
+                "{provider:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn model_set_in_a_workspace_config_follows_the_effective_route() {
+        let _env = env_lock();
+        let home = tempfile::tempdir().expect("isolated home");
+        let _home = ScopedEnvVar::set("CODEWHALE_HOME", &home.path().to_string_lossy());
+        let _config_path = ScopedEnvVar::remove("CODEWHALE_CONFIG_PATH");
+        let _legacy_config_path = ScopedEnvVar::remove("DEEPSEEK_CONFIG_PATH");
+        // A project config that names no provider: its root `model` applies to
+        // the route in effect (OpenAI here, from the global config), not to
+        // the DeepSeek default this file alone would suggest.
+        let repo = home.path().join("repo");
+        std::fs::create_dir_all(repo.join(".git")).unwrap();
+        std::fs::create_dir_all(repo.join(".codewhale")).unwrap();
+        let path = repo.join(".codewhale").join("config.toml");
+        assert!(codewhale_config::config_path_is_workspace_scoped(&path));
+        for (provider, expected) in [
+            (ProviderKind::Openai, "pro"),
+            (ProviderKind::Deepseek, "deepseek-v4-pro"),
+        ] {
+            std::fs::write(&path, "").unwrap();
+            let mut store = ConfigStore::load(Some(path.clone())).unwrap();
+            let mut runtime = resolved_runtime_for_test(provider, ProviderSource::Config);
+            runtime.base_url = codewhale_config::provider::provider_for_kind(provider)
+                .default_base_url()
+                .to_string();
+            run_model_command(
+                &mut store,
+                ModelCommand::Set {
+                    model: "pro".into(),
+                },
+                None,
+                &runtime,
+            )
+            .unwrap();
+            let saved = std::fs::read_to_string(&path).unwrap();
+            assert!(
+                saved.contains(&format!("model = \"{expected}\"")),
+                "{provider:?}: {saved}"
+            );
+        }
+    }
+
+    #[test]
+    fn thread_commands_refuse_unknown_ids_instead_of_reporting_success() {
+        let dir = tempfile::tempdir().expect("state dir");
+        let state = StateStore::open(Some(dir.path().join("state.db"))).expect("open state");
+        for command in [
+            ThreadCommand::Archive {
+                thread_id: "missing".into(),
+            },
+            ThreadCommand::Unarchive {
+                thread_id: "missing".into(),
+            },
+            ThreadCommand::Read {
+                thread_id: "missing".into(),
+            },
+        ] {
+            let label = format!("{command:?}");
+            let error = run_thread_store_command(&state, command)
+                .expect_err("an unknown thread id must fail");
+            assert!(
+                format!("{error:#}").contains("thread not found: missing"),
+                "{label}: {error:#}"
+            );
+        }
+    }
+
+    #[test]
+    fn app_server_in_process_transports_honor_the_global_config() {
+        let cli = parse_ok(&[
+            "codewhale",
+            "--config",
+            "/tmp/global-config.toml",
+            "app-server",
+            "--stdio",
+        ]);
+        let Some(Commands::AppServer(args)) = &cli.command else {
+            panic!("expected app-server");
+        };
+        assert_eq!(
+            app_server_config_path(&cli, args),
+            Some(PathBuf::from("/tmp/global-config.toml"))
+        );
+
+        let cli = parse_ok(&[
+            "codewhale",
+            "--config",
+            "/tmp/global-config.toml",
+            "app-server",
+            "--config",
+            "/tmp/app-server-config.toml",
+            "--socket",
+        ]);
+        let Some(Commands::AppServer(args)) = &cli.command else {
+            panic!("expected app-server");
+        };
+        assert_eq!(
+            app_server_config_path(&cli, args),
+            Some(PathBuf::from("/tmp/app-server-config.toml"))
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn inline_lane_start_exits_nonzero_when_the_lane_fails() {
+        let _env = env_lock();
+        let home = tempfile::tempdir().expect("isolated home");
+        let _home = ScopedEnvVar::set("CODEWHALE_HOME", &home.path().to_string_lossy());
+        let request = |script: &str| LaneStartRequest {
+            workflow: None,
+            fleet: None,
+            issue: None,
+            goal: None,
+            runtime: "inline".to_string(),
+            worktree_repo: None,
+            branch: None,
+            worktree_path: None,
+            worktree_ttl_secs: None,
+            command: vec!["sh".into(), "-c".into(), script.to_string()],
+            environment: Vec::new(),
+            cwd: None,
+        };
+        let error = start_lane(request("exit 3")).expect_err("a failed inline lane must fail");
+        assert!(format!("{error:#}").contains(" failed"), "{error:#}");
+        start_lane(request("exit 0")).expect("a completed inline lane succeeds");
+    }
+
+    #[test]
+    fn interactive_api_key_prompt_reads_through_the_hidden_reader() {
+        let key = read_prompted_api_key(
+            "deepseek",
+            true,
+            |prompt| {
+                assert_eq!(prompt, "Enter API key for deepseek: ");
+                Ok("  sk-hidden-fixture \n".to_string())
+            },
+            || panic!("terminal input must not use the plain reader"),
+        )
+        .unwrap();
+        assert_eq!(key, "sk-hidden-fixture");
+        let key = read_prompted_api_key(
+            "deepseek",
+            false,
+            |_| panic!("piped input has no terminal to hide"),
+            || Ok("sk-piped-fixture".to_string()),
+        )
+        .unwrap();
+        assert_eq!(key, "sk-piped-fixture");
+        assert!(
+            read_prompted_api_key("deepseek", true, |_| Ok("  \n".into()), || unreachable!())
+                .is_err()
+        );
+    }
+
+    #[test]
+    fn hidden_key_prompt_uses_a_terminal_stream_when_stderr_is_redirected() {
+        // `read_secure_line` on a non-terminal stream returns "" without
+        // reading, so `auth set 2>err.log` must prompt on stdout instead.
+        assert_eq!(hidden_prompt_stream(true, true), Some(PromptStream::Stderr));
+        assert_eq!(
+            hidden_prompt_stream(true, false),
+            Some(PromptStream::Stderr)
+        );
+        assert_eq!(
+            hidden_prompt_stream(false, true),
+            Some(PromptStream::Stdout)
+        );
+        assert_eq!(hidden_prompt_stream(false, false), None);
+    }
+
+    #[test]
+    fn auth_clear_fails_when_the_secret_store_keeps_the_key() {
+        use codewhale_secrets::{KeyringStore, SecretsError};
+        use std::sync::Arc;
+
+        struct UndeletableStore(Option<&'static str>);
+
+        impl KeyringStore for UndeletableStore {
+            fn get(&self, _key: &str) -> Result<Option<String>, SecretsError> {
+                Ok(self.0.map(str::to_string))
+            }
+
+            fn set(&self, _key: &str, _value: &str) -> Result<(), SecretsError> {
+                Err(SecretsError::ReadOnly)
+            }
+
+            fn delete(&self, _key: &str) -> Result<(), SecretsError> {
+                Err(SecretsError::Keyring("test delete failure".to_string()))
+            }
+
+            fn backend_name(&self) -> &'static str {
+                "undeletable test store"
+            }
+        }
+
+        let dir = tempfile::TempDir::new().expect("tempdir");
+        let path = dir.path().join("config.toml");
+        let clear = |held: Option<&'static str>| {
+            let mut store = ConfigStore::load(Some(path.clone())).expect("load config");
+            store.config.providers.deepseek.api_key = Some("sk-config-fixture".to_string());
+            store.save().unwrap();
+            let secrets = Secrets::new(Arc::new(UndeletableStore(held)));
+            let outcome = run_auth_command_with_secrets(
+                &mut store,
+                AuthCommand::Clear {
+                    provider: ProviderKind::Deepseek,
+                },
+                &secrets,
+            );
+            assert!(store.config.providers.deepseek.api_key.is_none());
+            outcome
+        };
+
+        let error = clear(Some("sk-keyring-fixture")).expect_err("a kept key is not cleared");
+        let message = format!("{error:#}");
+        assert!(message.contains("refused the delete"), "{message}");
+        assert!(!message.contains("sk-keyring-fixture"), "{message}");
+        clear(None).expect("nothing stored means nothing left to revoke");
+    }
+
+    #[test]
+    fn xai_auth_clear_reports_a_kept_key_after_the_revocation_commits() {
+        use codewhale_secrets::{KeyringStore, SecretsError};
+        use std::sync::Arc;
+
+        struct UndeletableStore;
+
+        impl KeyringStore for UndeletableStore {
+            fn get(&self, _key: &str) -> Result<Option<String>, SecretsError> {
+                Ok(Some("xai-keyring-fixture".to_string()))
+            }
+
+            fn set(&self, _key: &str, _value: &str) -> Result<(), SecretsError> {
+                Err(SecretsError::ReadOnly)
+            }
+
+            fn delete(&self, _key: &str) -> Result<(), SecretsError> {
+                Err(SecretsError::Keyring("test delete failure".to_string()))
+            }
+
+            fn backend_name(&self) -> &'static str {
+                "undeletable test store"
+            }
+        }
+
+        let _env = env_lock();
+        let home = tempfile::tempdir().expect("isolated home");
+        // The owned credentials directory is opened without following links,
+        // so the home must not sit behind one (macOS `/var` -> `/private/var`).
+        let home_path = home.path().canonicalize().expect("canonical home");
+        let _home = ScopedEnvVar::set("CODEWHALE_HOME", &home_path.to_string_lossy());
+        let path = home_path.join("config.toml");
+        let mut store = ConfigStore::load(Some(path.clone())).expect("load config");
+        store.config.providers.xai.api_key = Some("xai-config-fixture".to_string());
+        store.config.providers.xai.auth_mode = Some("api_key".to_string());
+        store.save().unwrap();
+        let secrets = Secrets::new(Arc::new(UndeletableStore));
+        let error = run_auth_command_with_secrets(
+            &mut store,
+            AuthCommand::Clear {
+                provider: ProviderKind::Xai,
+            },
+            &secrets,
+        )
+        .expect_err("a kept xAI key is not cleared");
+        let message = format!("{error:#}");
+        assert!(message.contains("refused the delete"), "{message}");
+        assert!(!message.contains("xai-keyring-fixture"), "{message}");
+        // The error is raised after the transaction commits, so the saved
+        // config leg is not rolled back.
+        let saved = ConfigStore::load(Some(path)).expect("reload config");
+        assert!(saved.config.providers.xai.api_key.is_none());
+        assert!(saved.config.providers.xai.auth_mode.is_none());
+    }
+
     #[test]
     fn parses_thread_command_matrix() {
         let cli = parse_ok(&["deepseek", "thread", "list", "--all", "--limit", "50"]);

crates/cli/src/update.rs

@@ -1108,8 +1108,14 @@ fn push_unique_path(paths: &mut Vec<PathBuf>, path: PathBuf) {
 fn push_update_path(paths: &mut Vec<PathBuf>, path: PathBuf, current_exe: &Path) {
     // Keep a same-target symlink as a symlink. Replacing its target refreshes
     // the alias too. Foreign/broken links stay in the plan and fail validation.
+    // Compare canonical paths on both sides: `current_exe()` is not
+    // canonicalized on macOS, so an install dir reached through a symlinked
+    // directory would otherwise never match its own alias.
     let same_target_link = std::fs::symlink_metadata(&path).is_ok_and(|m| m.is_symlink())
-        && std::fs::canonicalize(&path).is_ok_and(|resolved| resolved == current_exe);
+        && match [path.as_path(), current_exe].map(std::fs::canonicalize) {
+            [Ok(resolved), exe] => resolved == current_exe || exe.is_ok_and(|exe| resolved == exe),
+            _ => false,
+        };
     if !same_target_link {
         push_unique_path(paths, path);
     }
@@ -2687,6 +2693,24 @@ mod tests {
         }
     }
 
+    #[cfg(all(unix, not(target_os = "android")))]
+    #[test]
+    fn same_target_symlink_is_kept_when_the_install_dir_is_reached_through_a_symlink() {
+        use std::os::unix::fs::symlink;
+        let dir = tempfile::TempDir::new().unwrap();
+        let root = dir.path().canonicalize().unwrap();
+        let real_dir = root.join("real-bin");
+        std::fs::create_dir(&real_dir).unwrap();
+        let linked_dir = root.join("linked-bin");
+        symlink(&real_dir, &linked_dir).unwrap();
+        // `current_exe()` on macOS reports the path as reached, not resolved.
+        let primary = linked_dir.join("codewhale");
+        write_installed_binary(&primary, b"running bytes");
+        symlink("codewhale", linked_dir.join("codew")).unwrap();
+        let plan = update_plan_for_exe(&primary);
+        assert_eq!(plan.target_paths.as_slice(), std::slice::from_ref(&primary));
+    }
+
     #[test]
     fn legacy_binary_message_gives_copy_pasteable_migration_steps() {
         let message = legacy_binary_message(Path::new("/usr/local/bin/deepseek-tui"));

crates/config/src/credentials.rs

@@ -168,8 +168,9 @@ fn set_provider_api_key_unlocked(
 pub struct ClearOutcome {
     /// The secret-store slot the clear targeted.
     pub slot: &'static str,
-    /// `None` when the secret store accepted the delete; otherwise the backend
-    /// error, already stringified so it carries no credential material.
+    /// `None` when the secret store accepted the delete or holds no key for the
+    /// slot; otherwise the backend error, already stringified so it carries no
+    /// credential material.
     pub secret_store_error: Option<String>,
 }
 
@@ -216,7 +217,14 @@ pub fn clear_provider_api_key(
         store.config = original_config;
         return Err(error);
     }
-    let secret_store_error = secrets.delete(slot).err().map(|error| error.to_string());
+    // A backend that refuses every delete (a read-only store) but holds no key
+    // for this slot has nothing left to revoke, so that refusal is not a
+    // failure. Both callers get this rule from here.
+    let secret_store_error = secrets
+        .delete(slot)
+        .err()
+        .filter(|_| !matches!(secrets.get(slot), Ok(None)))
+        .map(|error| error.to_string());
     Ok(ClearOutcome {
         slot,
         secret_store_error,
@@ -226,6 +234,50 @@ pub fn clear_provider_api_key(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use codewhale_secrets::{KeyringStore, SecretsError};
+    use std::sync::Arc;
+
+    /// A store that refuses every delete and holds `.0` for every slot.
+    struct UndeletableStore(Option<&'static str>);
+
+    impl KeyringStore for UndeletableStore {
+        fn get(&self, _key: &str) -> Result<Option<String>, SecretsError> {
+            Ok(self.0.map(str::to_string))
+        }
+
+        fn set(&self, _key: &str, _value: &str) -> Result<(), SecretsError> {
+            Err(SecretsError::ReadOnly)
+        }
+
+        fn delete(&self, _key: &str) -> Result<(), SecretsError> {
+            Err(SecretsError::Keyring("test delete failure".to_string()))
+        }
+
+        fn backend_name(&self) -> &'static str {
+            "undeletable test store"
+        }
+    }
+
+    #[test]
+    fn a_refused_delete_fails_the_clear_only_while_the_store_still_holds_a_key() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let path = dir.path().join("config.toml");
+        for (held, fails) in [(Some("sk-keyring-fixture"), true), (None, false)] {
+            let mut store = ConfigStore::load(Some(path.clone())).expect("load config");
+            store.config.providers.deepseek.api_key = Some("sk-config-fixture".to_string());
+            let secrets = Secrets::new(Arc::new(UndeletableStore(held)));
+            let outcome = clear_provider_api_key(&mut store, &secrets, ProviderKind::Deepseek)
+                .expect("the config leg saves");
+            assert!(store.config.providers.deepseek.api_key.is_none());
+            assert_eq!(outcome.is_complete(), !fails, "delete refusal completion");
+            if let Some(error) = outcome.secret_store_error {
+                assert!(
+                    !error.contains("sk-keyring-fixture"),
+                    "credential leaked into error"
+                );
+            }
+        }
+    }
 
     fn store_with(body: &str) -> (tempfile::TempDir, ConfigStore) {
         let dir = tempfile::tempdir().expect("tempdir");

crates/core/src/lib.rs

@@ -804,17 +804,32 @@ impl ThreadManager {
         self.store.delete_thread_goal(&params.thread_id)
     }
 
+    fn knows_thread(&self, thread_id: &str) -> Result<bool> {
+        Ok(self.running_threads.contains_key(thread_id)
+            || self.store.get_thread(thread_id)?.is_some())
+    }
+
     /// Archives a thread so it no longer appears in default listings.
-    pub fn archive_thread(&mut self, thread_id: &str) -> Result<()> {
+    ///
+    /// Returns `false` for an unknown id: the store updates by id and reports
+    /// nothing for one it does not have, so the caller must not claim success.
+    pub fn archive_thread(&mut self, thread_id: &str) -> Result<bool> {
+        if !self.knows_thread(thread_id)? {
+            return Ok(false);
+        }
         self.store.mark_archived(thread_id)?;
         if let Some(thread) = self.running_threads.get_mut(thread_id) {
             thread.status = ThreadStatus::Archived;
         }
-        Ok(())
+        Ok(true)
     }
 
-    /// Restores an archived thread to active status.
-    pub fn unarchive_thread(&mut self, thread_id: &str) -> Result<()> {
+    /// Restores an archived thread to active status. Returns `false` for an
+    /// unknown id.
+    pub fn unarchive_thread(&mut self, thread_id: &str) -> Result<bool> {
+        if !self.knows_thread(thread_id)? {
+            return Ok(false);
+        }
         self.store.mark_unarchived(thread_id)?;
         if let Some(metadata) = self.store.get_thread(thread_id)? {
             let thread = to_protocol_thread(metadata);
@@ -822,7 +837,7 @@ impl ThreadManager {
                 *cached = thread;
             }
         }
-        Ok(())
+        Ok(true)
     }
 
     /// Records a user message in a thread and updates its preview and timestamp.
@@ -1220,10 +1235,10 @@ impl Runtime {
                 }
             }
             ThreadRequest::Archive { thread_id } => {
-                self.thread_manager.archive_thread(&thread_id)?;
+                let found = self.thread_manager.archive_thread(&thread_id)?;
                 Ok(ThreadResponse {
                     thread_id,
-                    status: "archived".to_string(),
+                    status: if found { "archived" } else { "missing" }.to_string(),
                     thread: None,
                     threads: Vec::new(),
                     goal: None,
@@ -1233,14 +1248,18 @@ impl Runtime {
                     approval_policy: None,
                     sandbox: None,
                     events: Vec::new(),
-                    data: json!({}),
+                    data: if found {
+                        json!({})
+                    } else {
+                        json!({"error":"thread not found"})
+                    },
                 })
             }
             ThreadRequest::Unarchive { thread_id } => {
-                self.thread_manager.unarchive_thread(&thread_id)?;
+                let found = self.thread_manager.unarchive_thread(&thread_id)?;
                 Ok(ThreadResponse {
                     thread_id,
-                    status: "unarchived".to_string(),
+                    status: if found { "unarchived" } else { "missing" }.to_string(),
                     thread: None,
                     threads: Vec::new(),
                     goal: None,
@@ -1250,7 +1269,11 @@ impl Runtime {
                     approval_policy: None,
                     sandbox: None,
                     events: Vec::new(),
-                    data: json!({}),
+                    data: if found {
+                        json!({})
+                    } else {
+                        json!({"error":"thread not found"})
+                    },
                 })
             }
             // A thread message is a *turn*, and this type is not the turn
@@ -2448,6 +2471,51 @@ mod tests {
         assert_eq!(persisted.approval_mode.as_deref(), Some("on-request"));
     }
 
+    #[tokio::test]
+    async fn archiving_an_unknown_thread_reports_missing_instead_of_success() {
+        let mut runtime = Runtime::new(
+            ConfigToml::default(),
+            temp_core_state("archive-unknown"),
+            Arc::new(McpManager::default()),
+            HookDispatcher::default(),
+        );
+        for request in [
+            ThreadRequest::Archive {
+                thread_id: "no-such-thread".to_string(),
+            },
+            ThreadRequest::Unarchive {
+                thread_id: "no-such-thread".to_string(),
+            },
+        ] {
+            let response = runtime.handle_thread(request).await.expect("handled");
+            assert_eq!(response.status, "missing");
+            assert_eq!(response.data["error"], "thread not found");
+        }
+
+        let spawned = runtime
+            .thread_manager
+            .spawn_thread_with_history(
+                "deepseek".to_string(),
+                PathBuf::from("/tmp/codewhale"),
+                InitialHistory::New,
+                true,
+            )
+            .expect("spawn thread");
+        let thread_id = spawned.thread.id.clone();
+        let response = runtime
+            .handle_thread(ThreadRequest::Archive {
+                thread_id: thread_id.clone(),
+            })
+            .await
+            .expect("handled");
+        assert_eq!(response.status, "archived");
+        let response = runtime
+            .handle_thread(ThreadRequest::Unarchive { thread_id })
+            .await
+            .expect("handled");
+        assert_eq!(response.status, "unarchived");
+    }
+
     #[tokio::test]
     async fn thread_message_is_refused_rather_than_faked() {
         // This arm used to record the user message, emit canned

npm/codewhale/scripts/install.js

@@ -566,6 +566,7 @@ function httpRequest(rawUrl, opts = {}) {
     let totalTimer = null;
     let stallTimer = null;
     let settled = false;
+    let handedOff = false;
     let req = null;
     let res = null;
     const signal = opts.signal;
@@ -587,7 +588,20 @@ function httpRequest(rawUrl, opts = {}) {
     };
 
     const fail = (err) => {
-      if (settled) return;
+      if (settled) {
+        // The stall, total, and abort budgets outlive the handoff: once the
+        // caller owns the body, end it with the error so the caller's own
+        // `error` handler rejects instead of waiting on a silent socket.
+        if (handedOff) {
+          cleanup();
+          try {
+            if (res && !res.destroyed) res.destroy(err);
+          } catch {
+            // ignore
+          }
+        }
+        return;
+      }
       settled = true;
       cleanup();
       try {
@@ -603,6 +617,15 @@ function httpRequest(rawUrl, opts = {}) {
       reject(err);
     };
 
+    // Hand the live response stream to the caller. The timers stay armed
+    // until the body ends or closes (see `fail`).
+    const handOff = (response) => {
+      settled = true;
+      handedOff = true;
+      response.on("close", () => cleanup());
+      resolve({ redirect: null, response });
+    };
+
     if (signal) {
       if (signal.aborted) {
         reject(abortError());
@@ -627,6 +650,12 @@ function httpRequest(rawUrl, opts = {}) {
       if (stallMs <= 0) return;
       if (stallTimer) clearTimeout(stallTimer);
       stallTimer = setTimeout(() => {
+        // A body the caller has paused (a slow disk pushing back through
+        // `pipe`) is not a stalled network. The total budget still bounds it.
+        if (handedOff && res && res.readableFlowing === false) {
+          armStallTimer();
+          return;
+        }
         fail(new DownloadTimeoutError(
           `download stalled — no bytes received for ${stallMs} ms ` +
           `(set CODEWHALE_DOWNLOAD_STALL_MS to raise it; total budget is ${totalTimeoutMs} ms)`,
@@ -692,9 +721,7 @@ function httpRequest(rawUrl, opts = {}) {
             return;
           }
           if (settled) return;
-          settled = true;
-          // Hand the live response stream to the caller.
-          resolve({ redirect: null, response });
+          handOff(response);
         });
         req.once("error", (err) => fail(err));
         req.once("socket", (s) => {
@@ -757,8 +784,7 @@ function httpRequest(rawUrl, opts = {}) {
                 return;
               }
               if (settled) return;
-              settled = true;
-              resolve({ redirect: null, response });
+              handOff(response);
             },
           );
           req.once("error", (err) => fail(err));
@@ -821,8 +847,7 @@ function httpRequest(rawUrl, opts = {}) {
                   return;
                 }
                 if (settled) return;
-                settled = true;
-                resolve({ redirect: null, response });
+                handOff(response);
               });
               req.once("error", (err) => fail(err));
               req.end();

npm/codewhale/scripts/run.js

@@ -1,4 +1,5 @@
-const { spawnSync } = require("child_process");
+const { spawn: spawnChild } = require("child_process");
+const os = require("os");
 const { getBinaryPath, installFailureHint } = require("./install");
 
 const pkg = require("../package.json");
@@ -41,11 +42,85 @@ function printVersionFallback(binaryName, error) {
   }
 }
 
+// Signals that end the wrapper must end the native binary too. With a
+// blocking spawn the wrapper died on SIGTERM and left the child running (still
+// executing tools, still holding its port); as PID 1 in a container it ignored
+// SIGTERM entirely. So SIGTERM, which a terminal never sends, is forwarded.
+//
+// SIGINT (Ctrl-C) and SIGHUP (terminal hangup) come from the terminal, which
+// delivers them to the whole foreground process group, native child included.
+// Forwarding them would deliver each one twice, and the native binary treats a
+// second signal as "skip the graceful drain". On Windows `child.kill()` is a
+// hard TerminateProcess, which would race the child's own Ctrl-C cleanup. The
+// wrapper only has to outlive them, then mirror how the child ended.
+const FORWARDED_SIGNALS = process.platform === "win32" ? [] : ["SIGTERM"];
+const OUTLIVED_SIGNALS = ["SIGINT", "SIGHUP"];
+
+function ignoreSignal() {}
+
+// Run the native binary and settle with how it ended. While it runs, the
+// forwarded signals go to the child and the outlived ones do not kill the
+// wrapper.
+function runChild(spawn, binaryPath, args, proc) {
+  return new Promise((resolve) => {
+    let child;
+    try {
+      child = spawn(binaryPath, args, { stdio: "inherit" });
+    } catch (error) {
+      resolve({ error });
+      return;
+    }
+    const listeners = [
+      ...FORWARDED_SIGNALS.map((signal) => [
+        signal,
+        () => {
+          try {
+            child.kill(signal);
+          } catch {
+            // The child already exited; its exit event settles the run.
+          }
+        },
+      ]),
+      ...OUTLIVED_SIGNALS.map((signal) => [signal, ignoreSignal]),
+    ];
+    for (const [signal, listener] of listeners) {
+      proc.on(signal, listener);
+    }
+    let settled = false;
+    const settle = (result) => {
+      if (settled) return;
+      settled = true;
+      for (const [signal, listener] of listeners) {
+        proc.removeListener(signal, listener);
+      }
+      resolve(result);
+    };
+    child.once("error", (error) => settle({ error }));
+    child.once("exit", (status, signal) => settle({ status, signal }));
+  });
+}
+
+// Mirror a signal death to the parent shell: re-raise it on the wrapper once
+// our listeners are gone, and fall back to the conventional 128 + n.
+function exitLikeChild(result, exit, proc) {
+  if (result.signal) {
+    try {
+      proc.kill(proc.pid, result.signal);
+    } catch {
+      // fall through to the numeric status
+    }
+    const number = os.constants.signals[result.signal];
+    return exit(number ? 128 + number : 1);
+  }
+  return exit(result.status ?? 1);
+}
+
 async function run(binaryName, options = {}) {
   const args = options.args || process.argv.slice(2);
   const resolveBinaryPath = options.getBinaryPath || getBinaryPath;
-  const spawn = options.spawnSync || spawnSync;
+  const spawn = options.spawn || spawnChild;
   const exit = options.exit || process.exit;
+  const proc = options.process || process;
   const versionFlag = isVersionFlag(args);
 
   let binaryPath;
@@ -59,9 +134,7 @@ async function run(binaryName, options = {}) {
     throw error;
   }
 
-  const result = spawn(binaryPath, args, {
-    stdio: "inherit",
-  });
+  const result = await runChild(spawn, binaryPath, args, proc);
   if (result.error) {
     if (versionFlag) {
       printVersionFallback(binaryName, result.error);
@@ -69,7 +142,7 @@ async function run(binaryName, options = {}) {
     }
     throw result.error;
   }
-  return exit(result.status ?? 1);
+  return exitLikeChild(result, exit, proc);
 }
 
 async function runCodeWhale() {
@@ -89,7 +162,7 @@ module.exports = {
   runCodeWhale,
   runCodeWhaleTui,
   reportStartFailure,
-  _internal: { isVersionFlag, printVersionFallback },
+  _internal: { isVersionFlag, printVersionFallback, FORWARDED_SIGNALS, OUTLIVED_SIGNALS },
 };
 
 if (require.main === module) {

npm/codewhale/test/install.test.js

@@ -346,3 +346,109 @@ test("httpRequest handles invalid URL parsing errors", async () => {
     assert.match(err.message, /Invalid URL: not-a-valid-url/);
   }
 });
+
+// A server that sends headers and part of a body, then either
+// goes silent or keeps trickling bytes, and never ends the response.
+async function withStallingServer(t, { trickleMs = 0 } = {}) {
+  const http = require("node:http");
+  const timers = [];
+  const server = http.createServer((req, res) => {
+    res.writeHead(200, { "Content-Type": "application/octet-stream" });
+    res.write("partial body");
+    if (trickleMs > 0) {
+      timers.push(setInterval(() => res.write("."), trickleMs));
+    }
+  });
+  await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve));
+  t.after(() => {
+    for (const timer of timers) clearInterval(timer);
+    server.closeAllConnections();
+    return new Promise((resolve) => server.close(resolve));
+  });
+  // Keep a developer's proxy variables from routing the loopback request.
+  const noProxy = process.env.NO_PROXY;
+  process.env.NO_PROXY = "*";
+  t.after(() => {
+    if (noProxy === undefined) delete process.env.NO_PROXY;
+    else process.env.NO_PROXY = noProxy;
+  });
+  return `http://127.0.0.1:${server.address().port}/asset`;
+}
+
+// Consume a handed-off body the way `downloadText` and `streamToFile` do:
+// resolve on `end`, reject on `error`.
+function consumeBody(response) {
+  return new Promise((resolve, reject) => {
+    response.on("data", () => {});
+    response.on("end", resolve);
+    response.on("error", reject);
+    response.resume();
+  });
+}
+
+test("a body that stalls after the headers fails with the stall timeout", { timeout: 5000 }, async (t) => {
+  const url = await withStallingServer(t);
+  const { response } = await _internal.httpRequest(url, {
+    stallMs: 150,
+    totalTimeoutMs: 30000,
+  });
+  await assert.rejects(consumeBody(response), (error) => {
+    assert.equal(error.code, "EDOWNLOADTIMEOUT");
+    assert.match(error.message, /stalled/);
+    return true;
+  });
+});
+
+test("a body that trickles past the total budget fails with the total timeout", { timeout: 5000 }, async (t) => {
+  const url = await withStallingServer(t, { trickleMs: 40 });
+  const { response } = await _internal.httpRequest(url, {
+    stallMs: 1000,
+    totalTimeoutMs: 400,
+  });
+  await assert.rejects(consumeBody(response), (error) => {
+    assert.equal(error.code, "EDOWNLOADTIMEOUT");
+    assert.match(error.message, /total timeout/);
+    return true;
+  });
+});
+
+test("a body paused by a slow consumer is not reported as a stall", { timeout: 5000 }, async (t) => {
+  const http = require("node:http");
+  const server = http.createServer((req, res) => {
+    res.writeHead(200, { "Content-Type": "application/octet-stream" });
+    res.write("first chunk ");
+    setTimeout(() => res.end("rest of the body"), 50);
+  });
+  await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve));
+  t.after(() => {
+    server.closeAllConnections();
+    return new Promise((resolve) => server.close(resolve));
+  });
+  const noProxy = process.env.NO_PROXY;
+  process.env.NO_PROXY = "*";
+  t.after(() => {
+    if (noProxy === undefined) delete process.env.NO_PROXY;
+    else process.env.NO_PROXY = noProxy;
+  });
+  const { response } = await _internal.httpRequest(
+    `http://127.0.0.1:${server.address().port}/asset`,
+    { stallMs: 150, totalTimeoutMs: 30000 },
+  );
+  // Hold the stream paused for several stall budgets, as `pipe` does while a
+  // slow disk drains, then let it flow again.
+  let body = "";
+  const done = new Promise((resolve, reject) => {
+    response.on("data", (chunk) => {
+      body += chunk;
+    });
+    response.on("end", resolve);
+    response.on("error", reject);
+  });
+  response.resume();
+  response.once("data", () => {
+    response.pause();
+    setTimeout(() => response.resume(), 600);
+  });
+  await done;
+  assert.equal(body, "first chunk rest of the body");
+});

npm/codewhale/test/run.test.js

@@ -1,8 +1,21 @@
 const assert = require("node:assert/strict");
+const { spawn } = require("node:child_process");
+const { EventEmitter } = require("node:events");
+const fs = require("node:fs");
+const os = require("node:os");
+const path = require("node:path");
 const test = require("node:test");
 
 const { run, reportStartFailure, _internal } = require("../scripts/run");
 
+// A stand-in for a spawned child that exits on the next tick.
+function exitingChild(status, signal = null) {
+  const child = new EventEmitter();
+  child.kill = () => true;
+  process.nextTick(() => child.emit("exit", status, signal));
+  return child;
+}
+
 test("version fallback handles only version flags", () => {
   assert.equal(_internal.isVersionFlag(["--version"]), true);
   assert.equal(_internal.isVersionFlag(["-V"]), true);
@@ -17,12 +30,12 @@ test("version flags prefer the installed binary over package metadata", async ()
   await run("codewhale", {
     args: ["--version"],
     getBinaryPath: async () => "/tmp/codewhale-test-binary",
-    spawnSync: (binary, args, options) => {
+    spawn: (binary, args, options) => {
       spawned = true;
       assert.equal(binary, "/tmp/codewhale-test-binary");
       assert.deepEqual(args, ["--version"]);
       assert.deepEqual(options, { stdio: "inherit" });
-      return { status: 0 };
+      return exitingChild(0);
     },
     exit: (status) => {
       exits.push(status);
@@ -43,9 +56,9 @@ test("codew wrapper dispatches the native shortcut binary", async () => {
       resolvedNames.push(name);
       return "/tmp/codew-test-binary";
     },
-    spawnSync: (binary, args) => {
+    spawn: (binary, args) => {
       spawned.push({ binary, args });
-      return { status: 0 };
+      return exitingChild(0);
     },
     exit: () => {},
   });
@@ -72,7 +85,7 @@ test("version flags fall back to package metadata when the binary is unavailable
           code: "ENOTFOUND",
         });
       },
-      spawnSync: () => {
+      spawn: () => {
         throw new Error("spawn should not run without a binary");
       },
       exit: (status) => {
@@ -115,3 +128,167 @@ test("start failures print the install hint for download errors", () => {
   reportStartFailure("codewhale", new Error("permission denied"), log);
   assert.deepEqual(logged, ["Failed to start codewhale: permission denied"]);
 });
+
+test("termination signals reach the native child and its status is kept", async () => {
+  const proc = new EventEmitter();
+  const child = new EventEmitter();
+  const killed = [];
+  child.kill = (signal) => {
+    killed.push(signal);
+    process.nextTick(() => child.emit("exit", 143, null));
+    return true;
+  };
+  const exits = [];
+  const running = run("codewhale", {
+    args: ["exec", "--auto", "task"],
+    getBinaryPath: async () => "/tmp/codewhale-test-binary",
+    spawn: () => child,
+    exit: (status) => exits.push(status),
+    process: proc,
+  });
+  await new Promise((resolve) => setImmediate(resolve));
+  // Terminal signals are outlived, never forwarded: the terminal already
+  // delivered them to the child through the process group.
+  for (const signal of ["SIGINT", "SIGHUP"]) {
+    assert.equal(proc.listenerCount(signal), 1, signal);
+    proc.emit(signal);
+  }
+  assert.deepEqual(killed, []);
+  const forwarded = _internal.FORWARDED_SIGNALS;
+  assert.deepEqual(forwarded, process.platform === "win32" ? [] : ["SIGTERM"]);
+  if (forwarded.length === 0) {
+    child.emit("exit", 0, null);
+    await running;
+    assert.deepEqual(exits, [0]);
+    return;
+  }
+  assert.equal(proc.listenerCount("SIGTERM"), 1);
+  proc.emit("SIGTERM");
+  await running;
+
+  assert.deepEqual(killed, ["SIGTERM"]);
+  assert.deepEqual(exits, [143]);
+  for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"]) {
+    assert.equal(proc.listenerCount(signal), 0, signal);
+  }
+});
+
+test("a child killed by a signal is reported as that signal", async () => {
+  const proc = new EventEmitter();
+  const raised = [];
+  proc.pid = 4242;
+  proc.kill = (pid, signal) => raised.push({ pid, signal });
+  const exits = [];
+  await run("codewhale", {
+    args: [],
+    getBinaryPath: async () => "/tmp/codewhale-test-binary",
+    spawn: () => exitingChild(null, "SIGTERM"),
+    exit: (status) => exits.push(status),
+    process: proc,
+  });
+  assert.deepEqual(raised, [{ pid: 4242, signal: "SIGTERM" }]);
+  assert.deepEqual(exits, [128 + os.constants.signals.SIGTERM]);
+});
+
+test(
+  "SIGTERM to the wrapper process does not leave the native child running",
+  { skip: process.platform === "win32" && "POSIX signals only", timeout: 20000 },
+  async () => {
+    const dir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "codewhale-run-signal-"));
+    const pidFile = path.join(dir, "child.pid");
+    const childScript =
+      `require("fs").writeFileSync(${JSON.stringify(pidFile)}, String(process.pid));` +
+      "setInterval(() => {}, 1000);";
+    const wrapperScript =
+      `require(${JSON.stringify(path.join(__dirname, "..", "scripts", "run.js"))})` +
+      `.run("codewhale", { getBinaryPath: async () => process.execPath, ` +
+      `args: ["-e", ${JSON.stringify(childScript)}] });`;
+    const wrapper = spawn(process.execPath, ["-e", wrapperScript], { stdio: "ignore" });
+    let childPid = null;
+    const alive = (pid) => {
+      try {
+        process.kill(pid, 0);
+        return true;
+      } catch {
+        return false;
+      }
+    };
+    try {
+      const deadline = Date.now() + 10000;
+      while (childPid === null && Date.now() < deadline) {
+        try {
+          childPid = Number.parseInt(fs.readFileSync(pidFile, "utf8"), 10) || null;
+        } catch {
+          await new Promise((resolve) => setTimeout(resolve, 25));
+        }
+      }
+      assert.ok(childPid, "native child should have started");
+      const wrapperExit = new Promise((resolve) => wrapper.once("exit", resolve));
+      wrapper.kill("SIGTERM");
+      await wrapperExit;
+      const settle = Date.now() + 5000;
+      while (alive(childPid) && Date.now() < settle) {
+        await new Promise((resolve) => setTimeout(resolve, 25));
+      }
+      assert.equal(alive(childPid), false, "native child outlived the wrapper");
+    } finally {
+      if (childPid && alive(childPid)) process.kill(childPid, "SIGKILL");
+      if (wrapper.exitCode === null && wrapper.signalCode === null) wrapper.kill("SIGKILL");
+      await fs.promises.rm(dir, { recursive: true, force: true });
+    }
+  },
+);
+
+test(
+  "Ctrl-C to the terminal's process group reaches the native child once",
+  { skip: process.platform === "win32" && "POSIX process groups only", timeout: 20000 },
+  async () => {
+    const dir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "codewhale-run-sigint-"));
+    const pidFile = path.join(dir, "child.pid");
+    const logFile = path.join(dir, "signals.log");
+    // Count every SIGINT, then exit the way a Ctrl-C'd CLI does.
+    const childScript =
+      `const fs = require("fs");` +
+      `process.on("SIGINT", () => { fs.appendFileSync(${JSON.stringify(logFile)}, "INT\\n");` +
+      ` setTimeout(() => process.exit(130), 300); });` +
+      `fs.writeFileSync(${JSON.stringify(pidFile)}, String(process.pid));` +
+      "setInterval(() => {}, 1000);";
+    const wrapperScript =
+      `require(${JSON.stringify(path.join(__dirname, "..", "scripts", "run.js"))})` +
+      `.run("codewhale", { getBinaryPath: async () => process.execPath, ` +
+      `args: ["-e", ${JSON.stringify(childScript)}] });`;
+    // `detached` makes the wrapper a process-group leader, standing in for the
+    // terminal's foreground job; the native child joins that group.
+    const wrapper = spawn(process.execPath, ["-e", wrapperScript], {
+      stdio: "ignore",
+      detached: true,
+    });
+    let childPid = null;
+    try {
+      const deadline = Date.now() + 10000;
+      while (childPid === null && Date.now() < deadline) {
+        try {
+          childPid = Number.parseInt(fs.readFileSync(pidFile, "utf8"), 10) || null;
+        } catch {
+          await new Promise((resolve) => setTimeout(resolve, 25));
+        }
+      }
+      assert.ok(childPid, "native child should have started");
+      const wrapperExit = new Promise((resolve) =>
+        wrapper.once("exit", (code, signal) => resolve({ code, signal })),
+      );
+      process.kill(-wrapper.pid, "SIGINT");
+      const ended = await wrapperExit;
+      const received = fs.readFileSync(logFile, "utf8").trim().split("\n");
+      assert.deepEqual(received, ["INT"], "the child must see one SIGINT, not a forwarded copy");
+      assert.deepEqual(ended, { code: 130, signal: null });
+    } finally {
+      try {
+        process.kill(-wrapper.pid, "SIGKILL");
+      } catch {
+        // already gone
+      }
+      await fs.promises.rm(dir, { recursive: true, force: true });
+    }
+  },
+);