fix(fleet): SSH destination checks, live wall-clock limits, policy prompt delivery, worker env, fleet save guard

[Hmbown]

No-Issue: verified bug-hunt findings

Summary

Fleet host/manager/store fixes, each re-verified on origin/main (606c36f) before fixing, plus a second commit (1b1744b) that addresses the review of the first. Files: crates/tui/src/fleet/{host,manager,store,worker_runtime,executor}.rs, crates/tui/src/child_env.rs, and the existing CLI test module in crates/tui/src/lib.rs.

1. SSH Fleet destination could be read as an ssh option (host.rs)

• Problem: build_ssh_command passed the configured host / user@host straight to ssh as the destination, with no -- and no validation. A host or user value beginning with - was parsed by ssh as an option.
• Resulting behavior: -- is placed immediately before the destination. SshFleetHostConfig::validate (used by from_host_spec and the adapter constructor) refuses a host or user that starts with -. A host is limited to host-name, IP-literal and alias characters (ASCII alphanumerics and . - _ : %, so no @). A user must be printable ASCII, and the characters OpenSSH itself refuses in a command-line user or host (' ` " $ \ ; & < > | ( ) { }) are rejected. Ordinary values are unchanged: fleet@builder.example.test, alice@corp.example@10.0.0.7 and fe80::1%en0 are all still accepted.
• Tests: fleet_host_ssh_refuses_option_like_or_malformed_destination covers option-like input, whitespace, NUL, shell characters, @ in the host, a Unicode format character and non-ASCII users, and checks that ordinary destinations are accepted. fleet_host_ssh_command_uses_sendenv_without_argv_secret_values asserts that -- directly precedes the destination.

2. Per-task wall-clock limit was only checked after the worker exited (manager.rs, executor.rs)

• Problem: the timeout_seconds / budget.max_seconds check sat inside if let Some(terminal) = poll_terminal_with_status(..). A hung worker never produced a terminal, so it was never stopped and the run never finished. A worker that exited cleanly but was observed just after the limit was recorded as Timeout.
• Resulting behavior: each tick with no observed exit compares the live worker's running time to the limit. On breach the worker is stopped, forgotten and finalized as Timeout (task Failed). An observed exit keeps its real outcome. worker_running_for now returns None once the executor has handed out that worker's exit. A later tick past the deadline therefore cannot stop that worker and write Timeout over the outcome, even if an earlier ledger write for the exit failed.
• Tests:
  • wall_clock_limit_stops_a_hung_worker_and_records_timeout: real sleep 30 worker, timeout_seconds = 1. It finishes within 10s as Failed/Timeout. The unused marker/trap scaffolding was removed.
  • worker_exit_observed_after_the_deadline_keeps_its_real_outcome: the worker exits at 0.2s and the next tick runs at 1.5s. The receipt is Pass.
  • consumed_worker_exit_is_not_recorded_as_a_timeout: the exit is consumed first, then a tick runs past the deadline. No terminal is recorded and no Timeout receipt is written.

3. [fleet.exec] append_system_prompt failed every Fleet task at launch (worker_runtime.rs, executor.rs)

• Problem: apply_exec_hardening appended [Policy] text to spec.objective but not to launch_manifest.prompt. With the coordination manager attached (always the case for codewhale fleet run), validate_registered_launch_spec rejected the task as "inconsistent persisted prompt". Separately, the policy was passed as a separate --append-system-prompt <value> argument. A policy that reads exactly like an exec option, such as --hooks, made clap reject the worker command.
• Resulting behavior: hardening no longer touches the objective. The policy is delivered once, as system prompt text, in a single --append-system-prompt=<value> argument, so clap always takes it as the value.
• Tests: configured_policy_prompt_keeps_registered_launch_spec_consistent and exec_hardening_leaves_policy_prompt_out_of_the_objective cover the objective change. worker_command_policy_prompt_that_looks_like_a_flag_parses parses the built worker argv through the real Cli, once with a Markdown bullet policy and once with --hooks, and asserts the value is kept intact and hooks stays false.
• Note on the review lead: I checked it in clap, and a leading - bullet list already parsed with the split form. Only flag-identical text failed, and the test covers that case.

4. Fleet workers lost proxy/CA/temp/toolchain environment (host.rs, child_env.rs)

• Problem: the spawn path calls env_clear() and rebuilt the env with only HOME, PATH and the Windows system root/COMSPEC. The first commit added a second hand-written list, which disagreed with child_env.rs: it lacked PATHEXT, WINDIR, ProgramFiles, CURL_CA_BUNDLE, REQUESTS_CA_BUNDLE, NODE_EXTRA_CA_CERTS, CARGO_HOME, USER, TERM and more.
• Resulting behavior: the worker base env comes from the existing child_env allowlist via the new child_env::sanitized_runtime_env_from, so there is one allowlist. Provider keys, *_TOKEN, CARGO_REGISTRY_* and DATABASE_URL are still dropped, and telemetry is still forced off.
• Proxy credentials (explicit decision): the worker process keeps user:password@ in proxy URLs because it is Codewhale itself and must reach its provider through the same proxy as the parent. Every tool the worker starts still builds its env through sanitized_child_env, which strips the userinfo at the model-facing boundary.
• Tests: worker_base_env_uses_the_child_env_allowlist_and_keeps_proxy_route feeds a parent snapshot to base_env_from. It asserts that 15 non-secret keys arrive unchanged, that 5 secret-shaped keys are absent, and that telemetry is forced off. It checks values, not only key names.

5. save_fleet could replace a same-slug file that is not a readable v2 Fleet of that name (store.rs)

• Problem: the NameTaken guard ran only when the existing file parsed as a v2 Fleet. Any other file at the slug was overwritten: a legacy roster, an exact fleet, a Fleet from a newer revision, schema = "Fleet", or a Fleet mid-edit. atomic_write also used a fixed shared <slug>.tmp.
• Resulting behavior: a non-empty existing file is replaced only when FleetFile::parse accepts it and its name matches. A readable v2 Fleet with another name returns NameTaken. Anything else is left unchanged, with one of two messages. A file that declares the v2 schema gets "is a Fleet file this build cannot read ()". Anything else gets "holds another fleet file (a legacy roster or exact fleet)". Both messages say to fix or move the file, or save under another name, and no longer point to migrate. atomic_write now uses the repository's utils::write_atomic_workspace, which gives each write its own temp file and keeps ordinary permissions.
• Tests: save_refuses_to_overwrite_a_legacy_fleet_file_of_the_same_slug and save_leaves_an_unreadable_fleet_file_of_the_same_slug_unchanged cover revision 3, Fleet, mid-edit TOML, the "cannot read" message and replacing an empty file.
• Not addressed: two first-time saves of different names that map to the same slug at the same moment still end last-writer-wins. Closing that needs a store lock or a no-clobber publish. I left it out of this slice.

Testing

• The CLI parser regression lives in the existing lib.rs CLI test module, keeping Fleet independent of the top-level CLI. Its real worker-command assertions are unchanged. After this test-only move, the focused Rust test passed 1/1, all 23 module-boundary Python tests passed, and the checked-in module graph baseline passed without modification.
• Independent integration gate at 1b1744b: npm test 670 passed (68 wrapper, 16 SDK, 50 extension-host, 536 web); npm run check:web passed. The source and checkout stayed unchanged during these checks.
• cargo test -p codewhale-tui --lib -- fleet::host:: fleet::manager:: fleet::store:: fleet::worker_runtime:: fleet::executor:: child_env:: gives 226 passed, 0 failed, 0 ignored (macOS, local).
• save_fleet consumers (fleet:: config::scope_tests tools::subagent::tests::roster_routes tools::workflow::shortlist_tests tui::views::fleet_) give 588 passed, 0 failed.
• With only the production hunks reverted (tests kept, including the first commit's manager hunk), the 8 new or updated tests give 0 passed, 8 failed, each for the expected reason: CURL_CA_BUNDLE missing, builder;true accepted, the --hooks policy rejected by clap, the combined flag missing, the rev-3 Fleet overwritten, Timeout recorded over a consumed exit, Timeout instead of Pass for a late-observed exit, and the hung worker not stopped within 10s.
• The first commit's own 7 regression tests failed 7/7 with its production hunks reverted.
• cargo fmt --all -- --check is clean. check-dead-code-budget, check-blocking-calls-budget, check-runtime-contract-budget, check-lexicon and check-persistence-backlog-budget all pass.
• Not run locally: clippy and the full workspace suite (CI owns these).

🤖 Generated with Claude Code

https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.