Detect stale containers: read the version list and the running version in one round trip per host

[mhenrixon]

Detect stale containers: read the version list and the running version in one round trip per host

Follow-up to #154 (PR 5, #159, which gave Dash::Cli::App::Boot a single boot_state capture but left stale_containers with its two). Sibling of #160 (proxy boot) and the pull and boot issues filed from the same deploy report.

Problem / Goal

On a real 4-host deploy on dash 4.1.0 (3 web, 1 job):

  Detect stale containers                 1.0s   8 ssh     3.5s

Dash::Cli::App#stale_containers runs per (host, role) and issues two captures back to back against the same docker daemon: list_versions (every container of the role, any status) and current_running_version (the one that is running). Four hosts, one role each, eight round trips to produce four small lists. The deploy calls this with --stop before every boot, so it is paid on every deploy.

Done looks like: one capture per (host, role) returns both answers, the row reads 4 ssh on the same topology, stopping a stale container still costs one round trip per container, and the standalone dash app stale_containers output is byte-identical.

Context (read these first)

• lib/dash/cli/app.rb — stale_containers (with_lock_if_stopping → on_roles(DASH.roles, hosts: DASH.app_hosts) → two capture_with_info calls, then execute *app.stop(version:) per stale version when --stop). lib/dash/cli/main.rb invokes it as timed("Detect stale containers") { invoke "dash:cli:app:stale_containers", [], invoke_options.merge(stop: true) } in deploy and redeploy.
• lib/dash/commands/app.rb — boot_state(version) and BOOT_STATE_SEPARATOR (--%--, chosen because a container id is hex and a version is a name suffix, so neither side can forge the line): the exact shape to mirror. list_versions(*docker_args, statuses:) (docker ps … --format "{{.Names}}" | extract_version_from_name), current_running_version, stop(version:) (container_id_for_version | xargs docker stop <stop_args>), extract_version_from_name.
• lib/dash/cli/app/boot.rb — capture_boot_state: how the combined output is split on the separator with partition and each half strip.presenced. Copy this, do not invent a second parsing style.
• lib/dash/sshkit_with_ext.rb — SSHKitDslRoles#on_roles: per-(host, role) threads; nothing to change, but it is why the count is per role, not per host.
• Tests: test/cli/app_test.rb (existing stale_containers assertions and the perf(deploy): cut a quarter of a deploy's SSH round trips #159 "single round trip" test near line 66 as the model, recorded_commands from test/cli/cli_test_case.rb), test/commands/app_test.rb (boot_state assertion as the model for the new builder), test/cli/main_test.rb (cost-guard; app:stale_containers is stubbed there, so pin the reduction in app_test).
• Rules: .claude/rules/performance.md, .claude/rules/coding-style.md (shell composed in Dash::Commands::App), .claude/rules/testing.md.

Decision

Add Dash::Commands::App#stale_state (name open) that chains list_versions, an echo of BOOT_STATE_SEPARATOR, and current_running_version, exactly like boot_state; stale_containers captures it once and splits.

• raise_on_non_zero_exit: false stays, as both captures have it today; an empty half means "none", as today.
• Stopping stays one execute per stale version. On a healthy fleet that list is empty (the previous deploy's prune already removed old containers), so the common case is exactly one round trip per (host, role).
• Output lines ("Detected stale container …", "Stopping stale container …") and the --quiet behaviour are untouched.

Alternatives considered

• Fold stale detection into Boot's boot_state capture. Different subcommand, different lock (with_lock_if_stopping takes the deploy lock only with --stop; boot runs under the same lock already held), and stale_containers is also a standalone command. Folding would move a lock-protected stop into the boot's own flow. Rejected; keep the subcommand, halve its cost.
• One docker ps that prints name and status and derive both answers locally. Same round-trip count as the chain, but it re-implements current_running_container's filter in Ruby and drifts from the builder the rest of the gem uses. Rejected.
• Stop all stale versions in one docker stop a b c. Only pays off when there are several stale containers on one host, which the prune keeps rare; and it changes the per-version "Stopping …" line ordering. Not in this issue; note it as optional in the PR if the harness shows it matters.

Settled in interview: none needed; the request is a fold with no operator-facing surface.

Design decisions the executor must not reopen

• Reuse BOOT_STATE_SEPARATOR and the partition split from capture_boot_state; do not introduce a second separator or parser.
• No change to which containers count as stale, to the stop arguments, or to the lock behaviour.

Implementation steps

One small PR (perf/stale-containers-single-capture off fresh main). Baseline first: the Detect stale containers row from a real multi-host deploy.

1. lib/dash/commands/app.rb — stale_state chaining the two existing builders around the separator echo. Unit test the exact string in test/commands/app_test.rb next to boot_state.
2. lib/dash/cli/app.rb — stale_containers captures stale_state once, splits with the same partition idiom as capture_boot_state (extract a tiny shared helper if both call sites end up identical, otherwise three lines is fine).
3. Tests RED first in test/cli/app_test.rb: one capture per (host, role) on deploy_with_roles.yml; stale versions detected and stopped exactly as before; empty output on both halves means nothing stale; --quiet still silent.
4. Real deploy via the integration harness; paste before/after rows.

Verification gates

• bundle exec ruby -Itest -e 'Dir["test/**/*_test.rb"].grep_v(/integration/).each { |f| require File.expand_path(f) }' — green
• bundle exec rubocop --parallel — no offenses
• bin/test — full suite (Docker + published proxy image)
• PR description shows the Detect stale containers row before and after (expected 8 → 4 on four single-role hosts) and states it is a fold.
• Cost-guard sequence in test/cli/main_test.rb unchanged.

Out of scope

• Changing what is considered stale, the stop arguments, or the prune that follows the boot.
• Folding this into the boot phase.
• No direct pushes to main, no manual lib/dash/version.rb bumps, no MINIMUM_VERSION change, nothing in ../kamal-proxy, no frozen-artifact renames.

Execution

Hand to a fresh implementation session on the sonnet tier. Baseline row first, then steps 1–4.