feat(report): save every deploy report, compare it with the last few, and export it by mhenrixon · Pull Request #158 · zoolutions/dash

mhenrixon · 2026-09-10T18:23:44Z

PR 4 of #154. Slices 1–3 measured the deploy and said what to change about the Dockerfile; this one gives all of it somewhere to live and something to be compared against.

Summary

Saved reports — lib/dash/report/writer.rb writes one JSON document per run under .dash/reports/<UTC timestamp>-<destination>-<command>.json: phases with their SSH accounting, the build steps, the advice, and how the run ended. Newest report: history: per destination (20 by default) survive a prune; history: 0 writes none. The directory gets its own .gitignore (* / !.gitignore) on first use. A deploy that failed is saved too, with "status": "failed" and the error — usually the run you most want to read afterwards.
Trends — lib/dash/report/trends.rb compares the run with the last five saved deploys of the same command and destination, once there are three to compare with: trend-build, trend-boot, trend-total (over 1.5× the median), and trend-overhead (that, or ten seconds of startup + secrets + locks regardless, naming whichever row dominated). They obey report: ignore: like every other rule and print in the same Advice block.
dash report — lib/dash/cli/report.rb re-renders the last saved report exactly as the deploy printed it, build rows back under the phase they hung under. dash report --last N prints one row per deploy; dash report path prints the directory. Read-only and entirely local: no lock, no SSH, safe to run mid-deploy.
Exports — the same numbers ship through the existing OTel logger as dash.phase, dash.build, dash.build.step and dash.advice events (all carrying the run's deployment.id), and reach the post-deploy hook as DASH_BUILD_RUNTIME, DASH_BOOT_RUNTIME, DASH_ADVICE_COUNT, DASH_ADVICE_WARNINGS and DASH_REPORT_PATH, each with its KAMAL_* twin. A phase that did not run contributes no variable rather than a zero.
Docs — the Deploy report page gains Trends, Saved reports (with the schema), dash report, OpenTelemetry and Hooks sections; Hooks and Commands gain the new surface; the post-deploy sample hook lists the new variables.

Still free: the cost-guard test passes with an unchanged command sequence. Nothing added here can end a deploy — every entry point is guarded and costs one yellow Deploy report unavailable line at worst.

Refs #154

What it looks like

  Finished all in 196.2 seconds
  Startup (load, config)                  0.9s
  Build and push app image              140.0s
    [build 5/9] RUN bundle install                                 84.1s
    cached steps                                                  9 of 14
  Boot                                   55.2s  12 ssh    41.0s
  Advice
    warn  Dockerfile:14   COPY . . runs before `bundle install` (line 21) (measured 84.1s uncached)
                          → copy the dependency manifests first, install, then copy the rest
    info  deploy history  build 140.0s vs median 61.0s over the last 5 deploys
  Report written to .dash/reports/2026-09-10T12-00-00Z-production-deploy.json

Test plan

bundle exec ruby -Itest -e 'Dir["test/**/*_test.rb"].grep_v(/integration/).each { |f| require File.expand_path(f) }' — 1846 runs, 0 failures
bundle exec rubocop --parallel — 273 files, no offenses
cd docs && bundle exec rspec — 108 examples, 0 failures
The cost-guard test deploy issues no commands beyond the pinned sequence passes with the sequence untouched
Manual: deploy something three times, then dash report, dash report --last 5, dash report path

Deviations & judgment calls

Trend rules do not live in the Dockerfile analyzer. The issue says "trend rules, run by the analyzer". They cannot be: analyze! runs right after the build phase so the deploy still gets advice when the boot fails, and at that moment Boot and the total runtime do not exist yet. They also have nothing to do with the Dockerfile. They landed as Dash::Report::Trends, run from Dash::Cli::Base#finish_report once the last phase has closed. Rule ids, thresholds, severities and report: ignore: behaviour are exactly as specified.
They still produce Dash::Dockerfile::Finding. That struct is already the advice type for non-Dockerfile locations (builder.cache); renaming it to something neutral would have touched all fifteen rules from PR 3 for no behaviour change. Left as is, deliberately — worth revisiting if a fourth non-Dockerfile advice source appears.
Writer takes the run metadata, not the config. The trend rules and the saved file have to agree on what this run was, so Cli::Base#report_run builds that hash once and both read it. The writer derives its filename from the started_at already in the document rather than being handed a second clock.
Phase names are literals in Trends, not shared constants. Sharing them would have meant timed(Dash::Report::PHASES[:build]) through Cli::Main, which reads worse than the string it replaces. test/cli/main_test.rb already pins every one of those names in the table it asserts, so a rename breaks a test rather than silently disabling a rule.
Step#to_h gained steps_in_stage. label is derived from it, so a JSON round trip could not rebuild the row without it. Nothing consumed that hash before this PR.
guarded_report gained a fallback argument so the hook-details caller can get {} back rather than nil. Third caller, so the helper grew the shape all three want.

Discoveries

CliTestCase now stubs reports_directory to a tmpdir. reports_directory is relative to the working directory, so the first green run wrote .dash/reports/ into this repository and the trend rules started comparing test runs with each other. Any future deploy-time file write needs the same treatment.
The generated .dash/reports/.gitignore shows up in git status. Its content is one of the issue's "do not reopen" decisions and it is the right one for a project that commits .dash/ — but on such a project the first deploy after upgrading leaves one untracked file, which Dash::Git.uncommitted_changes reports as "Building with uncommitted changes" on the next build until it is committed. Projects that gitignore .dash/ wholesale never see it. Left as specified; flagging it because it is the one user-visible side effect nobody asked for.
bin/test was not run. The issue's verification gates list the full suite for PRs 2, 3 and 5 — PR 4 changes no argv, and the cost-guard test still passes with an unchanged command sequence. Say the word and I will run it.

https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm

Summary by cubic

Deploy reports now outlive the terminal: every run saves a JSON document under .dash/reports/, compares itself against the last few deploys of the same command and destination, and can be re-read with dash report. Failed runs are saved too, with the error that ended them. Part 4 of #154.

Saved reports and trends

Reports keep the newest report: history: per destination (20 by default); history: 0 writes none, and negative values are rejected at config parse.
The reports directory gets its own .gitignore on first use, which shows up as an untracked file on projects that commit .dash/.
Four trend rules (trend-build, trend-boot, trend-total, trend-overhead) fire once three comparable deploys exist, comparing against the median of the last five, and obey report: ignore:.
dash report re-renders the latest report exactly as the deploy printed it; --last N prints one row per deploy (positive counts only); path prints the directory. All local and read-only — no lock, no SSH.
Reports are written atomically — name and content appear together so an interrupted deploy leaves nothing behind, never overwriting a file, and cleanup cannot remove a report that was already published; same-second or same-name runs take a numeric suffix — and unreadable or malformed files are skipped in silence.

Exports

The OTel logger ships dash.phase, dash.build, dash.build.step, and dash.advice events, all tagged with the run's deployment.id.
The post-deploy hook receives DASH_BUILD_RUNTIME, DASH_BOOT_RUNTIME, DASH_ADVICE_COUNT, DASH_ADVICE_WARNINGS, and DASH_REPORT_PATH with KAMAL_* twins; a phase that didn't run contributes no variable. Under dash setup the hook fires before the outer report is finalised, so DASH_REPORT_PATH is absent there, and a hook failure there marks the deferred report failed.

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

… and export it ## Summary The report a deploy prints has, until now, existed only in the terminal it scrolled past. This gives it somewhere to live and something to be compared against. - Every `deploy`, `redeploy`, `setup` and `rollback` writes a JSON report under `.dash/reports/<UTC timestamp>-<destination>-<command>.json` — phases with their SSH accounting, build steps, advice, and how the run ended. Kept newest-first per destination up to `report: history:` (20), `0` writes none. The directory gets its own `.gitignore` on first use. A failed deploy is saved too, with the error that ended it. - Four trend rules compare the run with the last five saved deploys of the same command and destination, once there are three to compare with: `trend-build`, `trend-boot`, `trend-total`, and `trend-overhead` (which also fires on ten seconds of overhead regardless, and names the row responsible). They obey `report: ignore:` like every other rule. - `dash report` re-renders the last saved report exactly as the deploy printed it, `dash report --last N` prints one row per deploy, `dash report path` prints the directory. Entirely local: no lock, no SSH. - The same numbers ship through the existing OTel logger as `dash.phase`, `dash.build`, `dash.build.step` and `dash.advice` events, and reach the `post-deploy` hook as `DASH_BUILD_RUNTIME`, `DASH_BOOT_RUNTIME`, `DASH_ADVICE_COUNT`, `DASH_ADVICE_WARNINGS` and `DASH_REPORT_PATH` (with their `KAMAL_*` twins). Still free: the cost-guard test in `test/cli/main_test.rb` passes with an unchanged command sequence. Nothing here can end a deploy — every entry point is guarded and costs one yellow line at worst. Refs #154 ## Test Coverage - test/report/writer_test.rb: naming, the .gitignore, pruning per destination, `history: 0`, failed runs, and that what is written reads back as the same table - test/report/history_test.rb: ordering, destination scoping, unreadable/foreign-schema files skipped, pruning - test/report/trends_test.rb: each rule's threshold, the five-deploy window, the three-deploy floor, command and status scoping, `ignore` - test/cli/report_test.rb: bare `dash report`, `--last`, `path`, destinations, an empty directory, and that reading issues no commands at all - test/cli/main_test.rb: the saved document, failure status, trend advice in the block and in the file, the post-deploy hook variables, and the unchanged command sequence - test/output/otel_logger_test.rb, test/otel_shipper_test.rb: the four event shapes and booleans staying boolean - test/timings_test.rb, test/report_test.rb, test/build/report_test.rb: the `to_h`/`from_h` round trip the saved reports depend on ## Verification - [x] bundle exec rubocop --parallel passes - [x] unit tests pass (1846 runs, 0 failures) - [x] cd docs && bundle exec rspec passes (108 examples, 0 failures) Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm

cubic-dev-ai

All reported issues were addressed across 29 files

_{Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic}

…erwrite one Review pass on #158. Four real defects, all in the same family: values that reach the report machinery without ever being checked. - `report: history: -1` fell through the `keep.zero?` guard and raised out of `entries.drop(-1)`, so every deploy printed "Deploy report unavailable" and nothing was ever pruned. Rejected at config parse instead, the way an invalid `hadolint:` already is — a negative count is a typo, and reading it as "keep none" would silently stop saving the reports the operator was configuring. - `dash report --last -1` raised `ArgumentError: negative array size` out of `Array#first`. A typo in a flag now gets a sentence. - A file that parsed as JSON and claimed schema 1 but held the wrong shapes (`"phases": "nope"`) crashed `dash report` in the renderer, a long way from where the mistake was made. `History` now checks the shape and skips it with the unreadable ones. - Two runs could resolve to the same filename — `started_at` is recorded to the second, and `eu/west` and `eu-west` sanitize alike — and the second silently overwrote the first. The writer now takes the next free name, and writes through a temporary file so an interrupted deploy cannot leave a truncated report that nothing ever prunes. Also: the otel test now captures every event and asserts what was NOT shipped, so the `dockerfile_steps` filter its name promises is actually locked in (a per-vertex regression previously passed), and the docs schema example, its `dash_version`, the trend precondition and the `--last` wording are corrected. The two report-lifecycle caveats a reviewer raised — `dash setup` firing `post-deploy` before the outer report is finalised, and `status` describing the deploy rather than the hook after it — are documented rather than restructured. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm

cubic-dev-ai

All reported issues were addressed across 11 files (changes from recent commits).

_{Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic}

…suffixes by run Second review pass on #158, all on the collision handling the first pass added. - Checking for a free name and then renaming into it is two steps, and another deploy can land between them. The report is now written under a private temporary name and hard-linked into place: the link is the check and the claim in one atomic, no-replace step, retried with the next suffix on EEXIST. A filesystem without hard links falls back to rename, which is still atomic. - `X.json` sorts after `X-2.json` byte for byte, so with `history: 1` the prune kept the older run and deleted the one just written. History now orders a trailing `-N` numerically after the name it collided with. - Shape checks were not enough: `"seconds": {}` inside a phase still crashed `dash report` at render. The well-formedness test is now the one that matters — render it — inside the same rescue that skips unreadable files. - The otel build test asserts every documented attribute again; the docs say what a post-deploy hook failure does to `status` under `dash setup`, where it lands inside the frame that writes the report. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm

cubic-dev-ai

All reported issues were addressed across 6 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic}

…k every top-level field Third review pass on #158. - The hard-link claim had a rename fallback for filesystems without links, and rename replaces — so on exactly those mounts, two same-second runs overwrote each other again. An exclusive create (O_EXCL) is universal and never replaces, so it is now the only claim mechanism: create the name, then rename the private scratch file onto a placeholder only this run holds. A placeholder that cannot be filled is deleted rather than left as an empty report nothing would ever prune. - The render probe alone let `phases: nil` through as an empty table, ignored `build: false`, and never looked at `error`, which `dash report` then tried to dig into. The top-level fields the writer always sets are checked by shape before the probe runs. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm

cubic-dev-ai

All reported issues were addressed across 4 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic}

Fourth review pass on #158, on a regression the third pass introduced. Claiming the name with an exclusive create and then renaming the content onto it left a window: a process killed between the two steps leaves an empty `.json`. Nothing ever clears that up — every reader skips a file it cannot parse, and the prune only counts the files it could read — so it stays in the directory forever. A Ctrl-C was enough, since Interrupt is not a SystemCallError and never reached the cleanup. The name and the content now always arrive together. A hard link publishes a file that is already complete, atomically, and only if the name is free. On a filesystem with no hard links, an exclusive create claims the name and the content goes in through that same descriptor — still no replacing, still no window where the file exists empty, and a failed write takes the name back down with it. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic}

Fifth review pass on #158. - The no-hard-links fallback wrote the content through the descriptor that claimed the name, which exposed a half-written report to readers and, on an Interrupt, left it there for good. It now renames the completed scratch file onto the claimed placeholder — atomic, and the only thing rename can replace is a placeholder this run created — and takes the placeholder down in an `ensure`, so a full disk and the operator's Ctrl-C both clean up after themselves. - The test guarding the scratch-write failure was stubbing File.write before prepare_directory had written the .gitignore, so it failed there and never reached the code it was about. The directory is made first now, and the test genuinely covers the path. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic}

Sixth review pass on #158. - `filled` set `moved = true` after the rename returned, and its `ensure` deleted the candidate whenever that flag was false. An interrupt landing between the successful rename and the assignment would therefore delete a report that had already been published. The rename consumes the scratch file, so the filesystem already knows the answer: a scratch that is still there is a rename that did not happen. Keying the cleanup on that removes the flag and the window it opened. - Re-added the success-path assertion that no scratch file survives a write, dropped when the publish path was rewritten, for both the hard-link and the no-hard-link route. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm

cubic-dev-ai

0 issues found across 2 files (changes from recent commits).

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Re-trigger cubic}

* perf(deploy): cut a quarter of a deploy's SSH round trips The deploy report's per-phase command counts (#155-#158) showed where dash was paying for connections it did not need. Every reduction here is one the table measured, and the table is how each was verified afterwards. A three-host integration deploy, before -> after (distinct round trips; the "Acquire server lock" rows nest inside the phase above them): Build and push app image 19 -> 15 Acquire deploy lock 4 -> 4 Ensure dash-proxy 32 -> 29 (server lock 5 -> 2) Detect stale containers 4 -> 4 Boot 30 -> 24 Prune 21 -> 10 (server lock 5 -> 2) total 110 -> 86 (-22%) A redeploy goes 67 -> 57. What changed: - Dash::Commands::App#boot_state asks the clash check and the running-version read in one command. The running version is therefore read before a clashing container is renamed, so when the clash IS the running container the version to stop afterwards is the name it was renamed to - otherwise the boot would stop the container it just started. - Dash::Commands::Auditor#record_then puts an audit line and the action it describes in one shell string, still in that order. Used by the boot, the latest-image tag, the image pull and both prune sweeps. - Dash::Commands::Builder::Base#clean_then_pull drops the stale image and pulls in one command; the removal stays best-effort. validate_image keeps its own round trip, or a failed pull would report a missing service label. - Dash::Cli::Base#ensure_run_directory sweeps each host once per process rather than once per lock acquire. Tracked per host, not as a flag: `dash upgrade` narrows the host set between acquires. Folded audit lines now print at INFO as part of the command they lead; the audit log itself and `dash audit` are unchanged. Also fixes test/cli/build_test.rb's build-report assertions, which only held on a dirty checkout: `stdouted` strips, eating the indent of a ` Build` header printed first, and CI hid it by running `rm Gemfile.lock` before the suite. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm * fix(build): keep a failed audit from pulling the image anyway Review findings on #159. `clean_then_pull` returned `clean || true && pull`. `&&` and `||` bind equally and associate left, so once record_then prepended the audit the chain read `(((run_dir && audit) && clean) || true) && pull` - a FAILED audit write fell into the same `|| true` and the pull ran anyway, exit status 0, where two separate executes would have raised. Parenthesise the best-effort clean so it confines the `|| true` to itself. Also from the review, all test-only: - The "single round trip" boot assertion counted Printer#execute_command, but both reads are captures and a stubbed capture_with_info never reaches that layer - it passed on the unfolded code too. Count capture_with_info instead; it now fails when the two reads are split. - Assert the pull's exact total command count rather than an integer-divided per-host average, which rounded away one extra command on a single host. - Move recorded_commands to CliTestCase; main_test keeps the lock-details redaction as recorded_deploy_commands on top of it. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm * test: stop recorded_commands swallowing commands past its block Review finding on #159. The Printer stub returns nil instead of printing, and mocha leaves it standing until the end of the test - so every command issued after the block was silently invisible, while the helper's comment promised recording only "during the block". Probed on a prune run: after the block, 0 of 4 `Running ...` lines printed; with the ensure, all 4 do. No caller ran commands after the block today, so this was a trap for the next one rather than a live bug. Refs #154 Claude-Session: https://claude.ai/code/session_01MKyeeYAuG4fR11sBmJa8dm