perf(boot): wait for readiness on the host and take the container id from docker run

[mhenrixon]

Summary

Two round trips come out of every boot, and one of them is the row that scaled with how long a container takes to come up.

The readiness wait moves onto the host. Dash::Commands::App#wait_for_ready builds a shell loop that evaluates the same status Dash::Cli::Healthcheck::Poller used to read one round trip at a time — docker inspect's health status, or the healthcheck: exec: probe's exit code — returns the moment that status is one the poller accepts, and otherwise keeps looking until deploy_timeout. Dash::Cli::App::Boot runs it as a single capture with an interaction handler attached, so the Container not ready yet, retrying in 1s (Xs elapsed, Ys left) beacons still print once a second while it waits. Progress goes to stderr and the final status to stdout, which is what keeps the captured value the status and nothing else.

The proxy target comes out of docker run. docker run --detach prints the id of the container it just started; boot captures it and takes the first twelve characters — the same short id docker container ls --quiet printed — instead of asking docker for it again on the next round trip.

Every readiness decision still lives in the poller, word for word: drift, announce_missing_gate, the readiness-delay confirm, the timeout error. The host loop only decides when to return, never what it means — Dash::Commands::Base::READY_STATUSES is the one thing both sides read, so they cannot drift apart.

Closes #163

Round trips per host, per role shape

Counted from the boot path and pinned by tests (test/cli/app_test.rb), not from a live deploy — see Deviations.

Round trip	web (proxy)	job (healthcheck, no proxy)	unchecked (no healthcheck)
boot_state	1	1	1
audit + ensure_env_directory (upload! is SFTP)	1	1	1
docker run --detach	1	1	1
container_id_for_version	1 0	–	–
dash-proxy deploy	1	–	–
readiness wait	–	N 1	N 2
stop old version	1	1	1
clean_up_assets	1	–	–
total	7 → 6	4 + N → 5	4 + N → 6

On the 4-host deploy in the issue (N ≈ 5 on the job host) that is web 7 → 6 and job 9 → 5.

• The container_id_for_version saving is a skip — the answer was already on the host's stdout.
• The readiness saving is a fold: N attempts become one blocking command. It is the row that matters, because N grows with boot time — a container that takes 60s cost ten round trips, and now costs one.
• The unchecked role keeps a second round trip: its readiness delay is spent on the laptop, so the confirming read afterwards has to be its own command.

unhealthy still waits — the issue's one reversed decision

The issue proposed returning from the wait the moment docker reports unhealthy, and invited the executor to keep the retry if there was a reason. There is one, and it is decisive: dash's default healthcheck emits --health-interval 1s with no --health-start-period, and docker's default --health-retries is 3. A container whose app needs longer than ~3s to serve /up is therefore reported unhealthy at t≈3s and healthy when it finishes booting. Today's poller waits through that and accepts the boot; returning early on unhealthy would fail essentially every normal Rails boot at three seconds.

So the host loop returns early for exactly the statuses Poller#acceptable? accepts and waits out the deadline for everything else — which is precisely what the client-side poll did, attempt by attempt. The only behaviour that changes is the beacon cadence: a fixed second instead of 1s, 2s, 3s … , which is strictly more responsive.

Test plan

• bundle exec ruby -Itest -e 'Dir["test/**/*_test.rb"].grep_v(/integration/).each { |f| require File.expand_path(f) }' — 1899 runs, 0 failures
• bundle exec rubocop --parallel — no offenses
• bin/test — full suite, 1918 runs, Docker + ghcr.io/zoolutions/dash-proxy:v1.1.0.1 (MINIMUM_VERSION unchanged). The integration deploys run the wait loop on real Docker-in-Docker hosts, including app_with_roles' unchecked workers role
• docs/ — bundle exec rspec spec/config_docs_spec.rb green after the doc-wording updates
• The generated loop run under a real POSIX sh against a fake docker: starting → healthy exits 0 with the status on stdout and beacons on stderr; never-ready exits 1 at the deadline with the last status; no-healthcheck:running returns immediately; timeout: 0 makes exactly one observation without spinning; the exec branch loops on the probe's exit code
• New unit coverage: exact command strings for both branches (test/commands/app_test.rb), the streaming handler against chunks split mid-line (test/cli/healthcheck/progress_reporter_test.rb), the poller's new call pattern (test/cli/healthcheck/poller_test.rb), and at the boot level — the proxy target read from the run, one wait round trip for a healthchecked role, wait-plus-confirm for an unchecked one, the handler and raise_on_non_zero_exit: false wiring, and a deadline that fails once rather than waiting twice
• A real multi-host deploy against a staging target (before/after per-host rows) — not run in this session, no hosts available; the integration harness covers the shell loop end to end but reports no timings
• Ctrl-C during the wait leaves no orphaned loop on the host — not verified

Deviations & judgment calls

Deviation — unhealthy does not return early from the host loop. Reasoned above; it is the one decision in the issue's Decision section I reversed, and the tests pin the behaviour either way.

Judgment — the block protocol is |mode, seconds_left|. The poller needs two different reads from the CLI: the blocking wait, and the plain status confirm after a readiness delay. Splitting wait_for_healthy into two callables would have churned every poller test, so the block is called with :wait / :confirm instead; procs ignore extra args, so the existing tests' blocks were untouched. seconds_left is passed so a retried wait cannot overshoot deploy_timeout.

Judgment — the run capture is unconditional, not proxy-only. One code path reads better than branching execute/capture on running_proxy?, and the id is what the failure message is about for every role.

Judgment — stub_capture echoes the captured command into SSHKit's output. A stubbed capture_with_info is intercepted above the Printer and never printed, so moving docker run from execute to capture made it invisible to roughly a dozen assertions across three test files. Rather than rewrite them all to read a recorded array, the shared stub helper echoes what it answered into the same stream stdouted reads. Side effects in a mocha matcher are not lovely; recorded_commands and stub_boot_state already use the idiom.

Discovery — the docs described the old cost model. lib/dash/configuration/docs/role.yml and docs/app/views/docs/pages/worker_roles.rb both claimed an exec probe costs "an SSH round trip plus a process spawn per poll" and that dash "polls docker's verdict with backoff". Neither survives this change; both updated.

Discovery — a latent bug next door, left alone. Dash::Cli::Main#container_available? rescues SSHKit::Runner::MultipleExecuteError, which sshkit 1.25 does not define, so a rollback to a version whose container is missing raises NameError instead of the intended message. Surfaced when a test stub moved; out of this issue's path and not touched here. Worth its own issue.

---

Summary by cubic

Moves the readiness wait onto the host and reads the proxy target out of the docker run output, cutting two round trips from every boot.

The container readiness wait used to poll from the laptop, one SSH round trip per attempt — a container that takes 60 seconds to boot cost ten trips. It's now a single blocking shell loop on the host; it returns the moment the status is one the poller accepts and otherwise runs until deploy_timeout. Progress beacons still print once a second, streamed back through an SSHKit interaction handler. The proxy target no longer costs a docker container ls read; boot takes the first twelve characters of the ID that docker run --detach already prints.

Round trips per host

• Proxy role: 7 → 6.
• Healthchecked role without proxy: 4 + N → 5.
• Unchecked role: 4 + N → 6.

Every readiness decision — drift detection, missing-gate warning, readiness-delay confirm, timeout error — still lives in the poller; the host loop only decides when to return. The one deliberate deviation from the issue proposal is that unhealthy still waits out the deadline: dash's default healthcheck probes every second with no start period and docker defaults to 3 retries, so an app slower than ~3 seconds to serve /up reports unhealthy before it's actually ready.

^{Written for commit e3f6ea6. Summary will update on new commits.}

[cubic-dev-ai]

3 issues found across 14 files

Confidence score: 3/5

• lib/dash/commands/app.rb converts Docker or inspected-container failures into an empty readiness status, which can silently wait until deploy_timeout instead of reporting the underlying command failure — preserve probe and inspect failures separately.
• lib/dash/cli/healthcheck/progress_reporter.rb can concatenate stdout status data with a stderr progress line across chunks, dropping the progress beacon — ignore non-:stderr callbacks before appending.
• test/cli/main_test.rb only partially asserts the new readiness machinery, leaving the readiness polling behavior insufficiently covered and allowing regressions to pass unnoticed — restore an explicit assertion for the readiness read.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="test/cli/main_test.rb">

<violation number="1" location="test/cli/main_test.rb:463">
P3: The previous test asserted the poller's health-status read with `expects(...).at_least_once`, but the new readiness machinery is only half-asserted: `stub_readiness_wait` is asserted (`expect: true`) while `stub_readiness_confirm` is registered as a plain `stubs`. Because `stub_capture` with `expect: false` does not require the capture to happen, this test now passes even if the confirm read (the `docker inspect --format` status read for the unchecked container) is removed or broken. Assert both captures, or assert the confirm with `expect: true`, so a regression in the confirm path fails the test.</violation>
</file>

<file name="lib/dash/cli/healthcheck/progress_reporter.rb">

<violation number="1" location="lib/dash/cli/healthcheck/progress_reporter.rb:20">
P2: When stdout’s final status arrives between chunks of a stderr progress line, this shared buffer concatenates the separate streams and drops that progress beacon. Ignore non-`:stderr` callbacks before appending, or maintain one buffer per stream.</violation>
</file>

<file name="lib/dash/commands/app.rb">

<violation number="1" location="lib/dash/commands/app.rb:151">
P2: When Docker or the inspected container is unavailable, this redirection turns a command failure into an empty readiness status and silently waits until `deploy_timeout`. Preserve probe/inspect failures separately from normal `starting` or `unhealthy` statuses so the boot can fail and report the actual Docker error instead of masking it as a readiness timeout.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When stdout’s final status arrives between chunks of a stderr progress line, this shared buffer concatenates the separate streams and drops that progress beacon. Ignore non-:stderr callbacks before appending, or maintain one buffer per stream.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At lib/dash/cli/healthcheck/progress_reporter.rb, line 20:

<comment>When stdout’s final status arrives between chunks of a stderr progress line, this shared buffer concatenates the separate streams and drops that progress beacon. Ignore non-`:stderr` callbacks before appending, or maintain one buffer per stream.</comment>

<file context>
@@ -0,0 +1,35 @@
+  end
+
+  # SSHKit's interaction-handler contract.
+  def on_data(_command, _stream_name, data, _channel = nil)
+    @mutex.synchronize do
+      @buffer << data.to_s
</file context>

Suggested change

	def on_data(_command, _stream_name, data, _channel = nil)
	@mutex.synchronize do
	def on_data(_command, stream_name, data, _channel = nil)
	return unless stream_name == :stderr

[cubic-dev-ai]

P2: When Docker or the inspected container is unavailable, this redirection turns a command failure into an empty readiness status and silently waits until deploy_timeout. Preserve probe/inspect failures separately from normal starting or unhealthy statuses so the boot can fail and report the actual Docker error instead of masking it as a readiness timeout.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At lib/dash/commands/app.rb, line 151:

<comment>When Docker or the inspected container is unavailable, this redirection turns a command failure into an empty readiness status and silently waits until `deploy_timeout`. Preserve probe/inspect failures separately from normal `starting` or `unhealthy` statuses so the boot can fail and report the actual Docker error instead of masking it as a readiness timeout.</comment>

<file context>
@@ -120,6 +141,17 @@ def ensure_env_directory
+      if role.healthcheck&.exec?
+        [ "if", *health_probe(version: version), ">/dev/null 2>&1;", "then status=healthy;", "else status=\"#{EXEC_PROBE_FAILED}\";", "fi;" ]
+      else
+        [ "status=$({", *status(version: version), ";} 2>/dev/null);" ]
+      end
+    end
</file context>

[cubic-dev-ai]

P3: The previous test asserted the poller's health-status read with expects(...).at_least_once, but the new readiness machinery is only half-asserted: stub_readiness_wait is asserted (expect: true) while stub_readiness_confirm is registered as a plain stubs. Because stub_capture with expect: false does not require the capture to happen, this test now passes even if the confirm read (the docker inspect --format status read for the unchecked container) is removed or broken. Assert both captures, or assert the confirm with expect: true, so a regression in the confirm path fails the test.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At test/cli/main_test.rb, line 463:

<comment>The previous test asserted the poller's health-status read with `expects(...).at_least_once`, but the new readiness machinery is only half-asserted: `stub_readiness_wait` is asserted (`expect: true`) while `stub_readiness_confirm` is registered as a plain `stubs`. Because `stub_capture` with `expect: false` does not require the capture to happen, this test now passes even if the confirm read (the `docker inspect --format` status read for the unchecked container) is removed or broken. Assert both captures, or assert the confirm with `expect: true`, so a regression in the confirm path fails the test.</comment>

<file context>
@@ -451,14 +451,16 @@ class CliMainTest < CliTestCase
-      .returns("no-healthcheck:running").at_least_once # health check
+    stub_run_capture # the proxy target, printed by the run itself
+    stub_readiness_wait "no-healthcheck:running", expect: true # workers
+    stub_readiness_confirm "no-healthcheck:running"
 
     Dash::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true)
</file context>