Conversation
dash is read far more often than it is written, and every reader has been re-deriving the same things from 164 files: which layer may talk to SSHKit, which direction the Dockerfile parser is allowed to be wrong in, why the report writer takes two steps to publish one file. That reasoning now lives in lode/ next to the code, so the next change starts from it instead of rebuilding it. lode/ holds a summary, terminology, practices, a workflow profile and eight subsystem summaries — the CLI and its two locks, the commander and command builders, deploy.yml validation, the Dockerfile advice (with a grammar table for every form the parser handles or deliberately approximates), the deploy report, the timing and SSHKit layer, testing and CI, and the docs site. Every line range and count is computed from the source, not read off the screen. lode/review/ holds 94 accepted review findings rewritten as rules about the system, across eight files: dockerfile-parsing, dockerfile-rules, reports, build-measurement, cli-and-proxy, config-and-secrets, testing, and docs-and-release. Sources: all 50 cubic learnings, and the 93 maintainer replies on merged PRs #155-#169. Duplicates between the two sources were merged (most of PR #157's Dockerfile thread is also a cubic learning), and five rejected suggestions became "Not a bug" entries with the reasoning that rejected them. Four invariants I had written as review rules turned out to come from the code rather than from any review thread — the config digest, Cobra boolean flags, empty --hosts filters, DASH_* precedence — so they stay in the subsystem summaries and are not claimed as findings. Six doc-versus-code disagreements are in the PR body rather than in any lode file, the largest being that CLAUDE.md still says stage 3c has not shipped while lib/dash/configuration/proxy.rb has named the container, network and volume "dash-*" since PR #130. Retires the eight local commands the lode plugin supersedes — lfg, plan, tdd, github-review-pr, github-review-failures, github-review-comments, finish-prs, debug-flaky — and points CLAUDE.md's table at the /lode: equivalents. The four repo-specific ones (architect, security, perf, review-pr) stay. Claude-Session: https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF
Minitest counts seven more than a line-based grep because three files generate tests inside each loops. Claude-Session: https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
17 issues found across 33 files
Confidence score: 2/5
.claude/settings.jsoncombines an invalid boolean plugin value with an unpinned GitHub source, which can prevent the plugin from resolving correctly and allow later default-branch changes to execute developer-permission hooks; use the expected version-string mapping and pin the repository revision.lode/reports/summary.mddocuments failure isolation thatfinish_reportdoes not provide: an error fromputs DASH.report.linescan escape finalization, and the--skip-pushbehavior is also misstated; wrap rendering withguarded_reportand correct the behavior description.- Several documentation references are materially inaccurate:
lode/plans/README.mdlinks to nonexistent relative paths, whilelode/review/docs-and-release.mdandlode/review/dockerfile-parsing.mdmisdescribe trend eligibility, version drift, heredoc recovery, and interpolation behavior; correct these against the implementation before relying on the guidance.
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=".claude/settings.json">
<violation number="1" location=".claude/settings.json:6">
P2: When a developer installs this project’s plugin, the enabled `lode` plugin resolves an unpinned GitHub repository, so later default-branch changes can execute unreviewed gate hooks with developer permissions. Pin the marketplace source to a reviewed commit and update that ref deliberately.</violation>
<violation number="2" location=".claude/settings.json:11">
P2: The `enabledPlugins` value for `lode@zoolutions` is the boolean `true`, but Claude Code's `enabledPlugins` setting maps `"plugin@marketplace"` to a version string to install (e.g. `"0.1.0"`). Setting a boolean is outside the documented schema, so the plugin version is not pinned and the stage-one pre-PR gate may not be enabled as intended. The PR description itself identifies the plugin build as 0.1.0 ("the cached 0.1.0 push hook").</violation>
</file>
<file name="lode/review/build-measurement.md">
<violation number="1" location="lode/review/build-measurement.md:39">
P3: The rule cites `#on_data`, `#finish` as `(lines 51-60)`, but only `#finish` lives there (lines 51-60). `#on_data` is on lines 37-50 in `lib/dash/build/progress_parser.rb`. Since these rules are deliberately precise citations (the PR description bills itself on computed, verified line ranges), the grouped range is wrong for `on_data`. Split the citation, e.g. `#on_data` (lines 37-50), `#finish` (lines 51-60).</violation>
</file>
<file name="lode/terminology.md">
<violation number="1" location="lode/terminology.md:7">
P2: The destination entry reverses proxy scoping: dash runs one proxy container per host shared by all destinations, not one proxy run per destination. State that the proxy is once per host and shared across destinations.</violation>
</file>
<file name="lode/review/docs-and-release.md">
<violation number="1" location="lode/review/docs-and-release.md:15">
P2: Three failed recent deploys do not eliminate trends when three older successful same-command reports remain retained. Say trends require three successful same-command reports in retained history; failed reports are skipped.</violation>
<violation number="2" location="lode/review/docs-and-release.md:35">
P2: This rule is already false: the saved-report example uses `4.0.8`, but `lib/dash/version.rb` is `4.1.1`, and the page has no claimed drift note. Update the example and add the note, or correct this rule to describe the actual state.</violation>
</file>
<file name="lode/review/dockerfile-parsing.md">
<violation number="1" location="lode/review/dockerfile-parsing.md:12">
P2: An unterminated heredoc can still produce findings from lines that look like valid instructions. `append_heredocs` reprocesses those lines, so document the recovery as independent parsing rather than finding-free parsing.</violation>
<violation number="2" location="lode/review/dockerfile-parsing.md:35">
P3: This rule misstates the code it documents. `Stage#interpolated_tag?` is `(tag || image).to_s.match?(INTERPOLATION)`, not scoped to `tag`: when there is no tag it falls back to interpolation in the image name. Since `Rules::LatestBase#finding_for` treats `interpolated_tag?` as a `return if` guard, `FROM $REGISTRY/app` (interpolation in the image, no tag) does suppress the warning — so interpolation in the image name *can* pin. The rule title and "`interpolated_tag?` is scoped to `tag`" contradict the paragraph's own closing clause ("an interpolated image with no tag still passes"). Reword the title and the "scoped to tag" claim so the title reads strict (tag-only) and the explanation names the `tag || image` fallback as the reason the no-tag case passes.</violation>
</file>
<file name="lode/docs-site/summary.md">
<violation number="1" location="lode/docs-site/summary.md:5">
P3: The claim that docs/Dockerfile "set BUNDLE_FROZEN: "false"" is inaccurate: the Dockerfile never sets a BUNDLE_FROZEN env var. It unfreezes bundler through `ENV BUNDLE_DEPLOYMENT="0"` and `RUN bundle config set frozen false`. Only docs-ci.yml sets the literal BUNDLE_FROZEN value. Since this summary advertises file-and-method precision, restate the Dockerfile half to name the actual mechanism.</violation>
</file>
<file name="lode/configuration/summary.md">
<violation number="1" location="lode/configuration/summary.md:13">
P3: The `#initialize` citation says line 57, but `def initialize(raw_config, ...)` is at line 58 in lib/dash/configuration.rb. The file pledges that every line reference was computed, and its other ranges (161-181, 243-251, 41-43) are exact, so this off-by-one should be corrected to 58.</violation>
<violation number="2" location="lode/configuration/summary.md:36">
P3: A multi-host primary role does not always auto-enable the loadbalancer; `auto_load_balanced_primary_role?` also requires `primary_role.running_proxy?`. Document that condition so non-proxied roles and their fixtures are not mischaracterized.</violation>
</file>
<file name="lode/review/cli-and-proxy.md">
<violation number="1" location="lode/review/cli-and-proxy.md:9">
P3: Rule 1's Where mis-cites loadbalancer.rb as a user of confirmed_empty?. The method is defined in base.rb:51 and called only in proxy.rb:396-397; loadbalancer.rb's legacy_rename (:62) gates on a marker file ([ :test, -f, legacy_rename_marker ]) and never calls confirmed_empty?. Correct the citation so the lode rule points future reviewers at the right file.</violation>
</file>
<file name="lode/review/config-and-secrets.md">
<violation number="1" location="lode/review/config-and-secrets.md:16">
P3: The rationale 'history: -1 ... pruned nothing, so the directory grew without bound' describes silent growth, but History#prune calls entries.drop(count) and Ruby's Array#drop(-1) raises ArgumentError, so a deploy with history: -1 would abort at prune after publishing the report, not silently accumulate. The rule (reject negatives) is still right, but the stated symptom is inaccurate for the current code.</violation>
</file>
<file name="lode/plans/README.md">
<violation number="1" location="lode/plans/README.md:6">
P2: These relative paths send planners to nonexistent `lode/ROADMAP.md` and `lode/tmp/` locations. Use `../../ROADMAP.md` and `../../tmp/` so plans point to the actual roadmap and ignored scratch directory.</violation>
</file>
<file name="lode/reports/summary.md">
<violation number="1" location="lode/reports/summary.md:21">
P2: `finish_report` does not guard `puts DASH.report.lines`; a rendering error can still escape report finalization, so this line incorrectly documents failure isolation. Wrap the print in `guarded_report` and update the summary.</violation>
<violation number="2" location="lode/reports/summary.md:37">
P2: A `--skip-push` deploy still analyzes an available Dockerfile and emits static findings; only build-measured rules stay silent. Replace this parenthetical with the actual behavior.</violation>
</file>
<file name="lode/summary.md">
<violation number="1" location="lode/summary.md:3">
P3: When a project has only `.kamal`, this summary sends readers to the wrong report path. Document `.dash/reports` as the default and `.kamal/reports` as the legacy fallback.</violation>
</file>
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Re-trigger cubic
| } | ||
| }, | ||
| "enabledPlugins": { | ||
| "lode@zoolutions": true |
There was a problem hiding this comment.
P2: The enabledPlugins value for lode@zoolutions is the boolean true, but Claude Code's enabledPlugins setting maps "plugin@marketplace" to a version string to install (e.g. "0.1.0"). Setting a boolean is outside the documented schema, so the plugin version is not pinned and the stage-one pre-PR gate may not be enabled as intended. The PR description itself identifies the plugin build as 0.1.0 ("the cached 0.1.0 push hook").
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/settings.json, line 11:
<comment>The `enabledPlugins` value for `lode@zoolutions` is the boolean `true`, but Claude Code's `enabledPlugins` setting maps `"plugin@marketplace"` to a version string to install (e.g. `"0.1.0"`). Setting a boolean is outside the documented schema, so the plugin version is not pinned and the stage-one pre-PR gate may not be enabled as intended. The PR description itself identifies the plugin build as 0.1.0 ("the cached 0.1.0 push hook").</comment>
<file context>
@@ -0,0 +1,13 @@
+ }
+ },
+ "enabledPlugins": {
+ "lode@zoolutions": true
+ }
+}
</file context>
|
|
||
| ## The deploy | ||
|
|
||
| - **destination** — the `-d`/`--destination` suffix that picks up `config/deploy.<destination>.yml` on top of `config/deploy.yml` (`Dash::Configuration.load_raw_config`). It scopes the deploy lock, the report history and the report filenames; it does **not** scope the proxy, which one host runs once for every destination deployed onto it. |
There was a problem hiding this comment.
P2: The destination entry reverses proxy scoping: dash runs one proxy container per host shared by all destinations, not one proxy run per destination. State that the proxy is once per host and shared across destinations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/terminology.md, line 7:
<comment>The destination entry reverses proxy scoping: dash runs one proxy container per host shared by all destinations, not one proxy run per destination. State that the proxy is once per host and shared across destinations.</comment>
<file context>
@@ -0,0 +1,45 @@
+
+## The deploy
+
+- **destination** — the `-d`/`--destination` suffix that picks up `config/deploy.<destination>.yml` on top of `config/deploy.yml` (`Dash::Configuration.load_raw_config`). It scopes the deploy lock, the report history and the report filenames; it does **not** scope the proxy, which one host runs once for every destination deployed onto it.
+- **deploy lock** — a `mkdir` lock on the primary host, scoped to service + destination so two destinations can deploy at once (`Dash::Commands::Lock::SCOPES`, line 10). Taken outside the server lock, always in that order, so no cycle forms (`Dash::Cli::Base#modify`, lines 262-268).
+- **server lock** — a second lock (`Dash::Commander#server_lock`, `scope: :server`) taken on the proxy hosts — or on every host when the config declares none (`Dash::Cli::Base#server_lock_hosts`, lines 413-415) — because the proxy container is shared across destinations while the deploy lock is not (`Dash::Cli::Base`, lines 281-284). Hosts are taken one at a time and rolled back on contention, so a retry does not collide with the locks it already won.
</file context>
Suggested change
| - **destination** — the `-d`/`--destination` suffix that picks up `config/deploy.<destination>.yml` on top of `config/deploy.yml` (`Dash::Configuration.load_raw_config`). It scopes the deploy lock, the report history and the report filenames; it does **not** scope the proxy, which one host runs once for every destination deployed onto it. | |
| - **destination** — the `-d`/`--destination` suffix that picks up `config/deploy.<destination>.yml` on top of `config/deploy.yml` (`Dash::Configuration.load_raw_config`). It scopes the deploy lock, the report history and the report filenames; it does **not** scope the proxy, which runs once per host and is shared by every destination deployed there. |
| - **Origin:** cubic learning 5af29be4; PR #158 | ||
|
|
||
| ### The trend window is described as "three *succeeded* reports of the same command", not "three deploys" | ||
| - **Holds because:** `Trends#comparable` filters on `status == "succeeded"` before counting, and counts retained reports — so a destination whose last three deploys failed has no trend at all, which is the opposite of what "after three deploys you get trends" promises. |
There was a problem hiding this comment.
P2: Three failed recent deploys do not eliminate trends when three older successful same-command reports remain retained. Say trends require three successful same-command reports in retained history; failed reports are skipped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/review/docs-and-release.md, line 15:
<comment>Three failed recent deploys do not eliminate trends when three older successful same-command reports remain retained. Say trends require three successful same-command reports in retained history; failed reports are skipped.</comment>
<file context>
@@ -0,0 +1,52 @@
+- **Origin:** cubic learning 5af29be4; PR #158
+
+### The trend window is described as "three *succeeded* reports of the same command", not "three deploys"
+- **Holds because:** `Trends#comparable` filters on `status == "succeeded"` before counting, and counts retained reports — so a destination whose last three deploys failed has no trend at all, which is the opposite of what "after three deploys you get trends" promises.
+- **Where:** `docs/app/views/docs/pages/deploy_report.rb`; `lib/dash/report/trends.rb#comparable` (lines 43-48)
+- **Origin:** PR #158
</file context>
Suggested change
| - **Holds because:** `Trends#comparable` filters on `status == "succeeded"` before counting, and counts retained reports — so a destination whose last three deploys failed has no trend at all, which is the opposite of what "after three deploys you get trends" promises. | |
| - **Holds because:** `Trends#comparable` filters on `status == "succeeded"` before counting the retained history, so a destination with fewer than three successful same-command reports retained has no trend; failed reports themselves are skipped. |
| - **Origin:** cubic learning b1721a6f; PR #157 | ||
|
|
||
| ### A sample that embeds a version goes stale on the next release, and says so | ||
| - **Holds because:** the saved-report example carries `dash_version`, which is a real field of a real file. It was corrected to match `lib/dash/version.rb`, and the correction is not automatable — a note in the page records that it is a sample of a saved file and will drift. |
There was a problem hiding this comment.
P2: This rule is already false: the saved-report example uses 4.0.8, but lib/dash/version.rb is 4.1.1, and the page has no claimed drift note. Update the example and add the note, or correct this rule to describe the actual state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/review/docs-and-release.md, line 35:
<comment>This rule is already false: the saved-report example uses `4.0.8`, but `lib/dash/version.rb` is `4.1.1`, and the page has no claimed drift note. Update the example and add the note, or correct this rule to describe the actual state.</comment>
<file context>
@@ -0,0 +1,52 @@
+- **Origin:** cubic learning b1721a6f; PR #157
+
+### A sample that embeds a version goes stale on the next release, and says so
+- **Holds because:** the saved-report example carries `dash_version`, which is a real field of a real file. It was corrected to match `lib/dash/version.rb`, and the correction is not automatable — a note in the page records that it is a sample of a saved file and will drift.
+- **Where:** `docs/app/views/docs/pages/deploy_report.rb`
+- **Origin:** PR #158
</file context>
| ### A heredoc whose delimiter never arrives consumes nothing at all | ||
| - **Holds because:** `RUN printf '%s' '<<EOF'` is a shell word, not a heredoc, and so is a typo'd delimiter. Consuming to the end of the file would fold every later instruction into that one and lose every finding after it. `append_heredocs` scans ahead for the delimiter and returns `[ text, index ]` — the index it was *entered* with — when it cannot find one. | ||
| - **Where:** `lib/dash/dockerfile/parser.rb#append_heredocs` (lines 107-125) | ||
| - **Safe direction:** the whole heredoc body parses as nonsense instructions of its own, which produce no findings; the alternative silently blinds the rest of the file. |
There was a problem hiding this comment.
P2: An unterminated heredoc can still produce findings from lines that look like valid instructions. append_heredocs reprocesses those lines, so document the recovery as independent parsing rather than finding-free parsing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/review/dockerfile-parsing.md, line 12:
<comment>An unterminated heredoc can still produce findings from lines that look like valid instructions. `append_heredocs` reprocesses those lines, so document the recovery as independent parsing rather than finding-free parsing.</comment>
<file context>
@@ -0,0 +1,99 @@
+### A heredoc whose delimiter never arrives consumes nothing at all
+- **Holds because:** `RUN printf '%s' '<<EOF'` is a shell word, not a heredoc, and so is a typo'd delimiter. Consuming to the end of the file would fold every later instruction into that one and lose every finding after it. `append_heredocs` scans ahead for the delimiter and returns `[ text, index ]` — the index it was *entered* with — when it cannot find one.
+- **Where:** `lib/dash/dockerfile/parser.rb#append_heredocs` (lines 107-125)
+- **Safe direction:** the whole heredoc body parses as nonsense instructions of its own, which produce no findings; the alternative silently blinds the rest of the file.
+- **Proven by:** `test/dockerfile/parser_test.rb:157` ("an unterminated heredoc does not swallow the rest of the file", with the quoted-token input)
+- **Origin:** cubic learnings 01552777, 9a9f188e; PR #157
</file context>
Suggested change
| - **Safe direction:** the whole heredoc body parses as nonsense instructions of its own, which produce no findings; the alternative silently blinds the rest of the file. | |
| - **Safe direction:** lines after an unterminated heredoc are reprocessed as independent instructions, so valid-looking body text may produce findings; this preserves visibility into the rest of the file. |
|
|
||
| ## Validation | ||
|
|
||
| `Dash::Configuration::Validation` is a concern each section includes. `validation_doc` reads `lib/dash/configuration/docs/<section>.yml` — **15** commented-YAML files, the same ones `dash docs` prints — and `Dash::Configuration::Validator` walks the operator's config against that example, checking types key by key and rejecting unknown keys. Nine sections have a `Validator` subclass under `lib/dash/configuration/validator/` for semantics the example cannot express: `accessory`, `alias`, `builder`, `configuration`, `env`, `proxy`, `registry`, `role`, `servers`. |
There was a problem hiding this comment.
P3: The #initialize citation says line 57, but def initialize(raw_config, ...) is at line 58 in lib/dash/configuration.rb. The file pledges that every line reference was computed, and its other ranges (161-181, 243-251, 41-43) are exact, so this off-by-one should be corrected to 58.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/configuration/summary.md, line 13:
<comment>The `#initialize` citation says line 57, but `def initialize(raw_config, ...)` is at line 58 in lib/dash/configuration.rb. The file pledges that every line reference was computed, and its other ranges (161-181, 243-251, 41-43) are exact, so this off-by-one should be corrected to 58.</comment>
<file context>
@@ -0,0 +1,60 @@
+
+## Validation
+
+`Dash::Configuration::Validation` is a concern each section includes. `validation_doc` reads `lib/dash/configuration/docs/<section>.yml` — **15** commented-YAML files, the same ones `dash docs` prints — and `Dash::Configuration::Validator` walks the operator's config against that example, checking types key by key and rejecting unknown keys. Nine sections have a `Validator` subclass under `lib/dash/configuration/validator/` for semantics the example cannot express: `accessory`, `alias`, `builder`, `configuration`, `env`, `proxy`, `registry`, `role`, `servers`.
+
+So the docs are load-bearing twice over: they are what `dash docs` prints, what the validator checks against, and (through `docs/app/models/config_doc.rb`) what the documentation site's Configuration pages are generated from.
</file context>
Suggested change
| `Dash::Configuration::Validation` is a concern each section includes. `validation_doc` reads `lib/dash/configuration/docs/<section>.yml` — **15** commented-YAML files, the same ones `dash docs` prints — and `Dash::Configuration::Validator` walks the operator's config against that example, checking types key by key and rejecting unknown keys. Nine sections have a `Validator` subclass under `lib/dash/configuration/validator/` for semantics the example cannot express: `accessory`, `alias`, `builder`, `configuration`, `env`, `proxy`, `registry`, `role`, `servers`. | |
| `#initialize` (58-…) |
|
|
||
| ### Absence of a container is only ever concluded from a listing that succeeded | ||
| - **Holds because:** `! docker container inspect name >/dev/null 2>&1` exits non-zero both for "no such container" and for "the daemon could not be asked" — busy, permission denied, socket gone — so it reads a transient failure as confirmed absence. In the legacy-rename bridge that meant writing `.legacy-renamed` with the legacy container still running, and every later deploy skipping the migration permanently. A **listing** exits 0 whether or not anything matched and non-zero only on a genuine failure, and `result=$(list)` carries the listing's own status (POSIX, verified against sh and bash), so the chain stops before the test runs. | ||
| - **Where:** `lib/dash/commands/base.rb#confirmed_empty?` (lines 51-53), used from `lib/dash/commands/proxy.rb` and `loadbalancer.rb` |
There was a problem hiding this comment.
P3: Rule 1's Where mis-cites loadbalancer.rb as a user of confirmed_empty?. The method is defined in base.rb:51 and called only in proxy.rb:396-397; loadbalancer.rb's legacy_rename (:62) gates on a marker file ([ :test, -f, legacy_rename_marker ]) and never calls confirmed_empty?. Correct the citation so the lode rule points future reviewers at the right file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/review/cli-and-proxy.md, line 9:
<comment>Rule 1's Where mis-cites loadbalancer.rb as a user of confirmed_empty?. The method is defined in base.rb:51 and called only in proxy.rb:396-397; loadbalancer.rb's legacy_rename (:62) gates on a marker file ([ :test, -f, legacy_rename_marker ]) and never calls confirmed_empty?. Correct the citation so the lode rule points future reviewers at the right file.</comment>
<file context>
@@ -0,0 +1,64 @@
+
+### Absence of a container is only ever concluded from a listing that succeeded
+- **Holds because:** `! docker container inspect name >/dev/null 2>&1` exits non-zero both for "no such container" and for "the daemon could not be asked" — busy, permission denied, socket gone — so it reads a transient failure as confirmed absence. In the legacy-rename bridge that meant writing `.legacy-renamed` with the legacy container still running, and every later deploy skipping the migration permanently. A **listing** exits 0 whether or not anything matched and non-zero only on a genuine failure, and `result=$(list)` carries the listing's own status (POSIX, verified against sh and bash), so the chain stops before the test runs.
+- **Where:** `lib/dash/commands/base.rb#confirmed_empty?` (lines 51-53), used from `lib/dash/commands/proxy.rb` and `loadbalancer.rb`
+- **Safe direction:** an unreadable docker state means "not yet migrated" — the bridge runs again, which is a no-op if it already happened. Its argument must be a listing, never an inspect.
+- **Origin:** PR #167
</file context>
Suggested change
| - **Where:** `lib/dash/commands/base.rb#confirmed_empty?` (lines 51-53), used from `lib/dash/commands/proxy.rb` and `loadbalancer.rb` | |
| - **Where:** `lib/dash/commands/base.rb#confirmed_empty?` (lines 51-53), used from `lib/dash/commands/proxy.rb` |
| - **Origin:** cubic learning 44aaf149; PR #157 | ||
|
|
||
| ### A negative `report: history:` raises; `history: 0` does not | ||
| - **Holds because:** `history: -1` made every deploy print "Deploy report unavailable" **and** pruned nothing, so the directory grew without bound. A typo-like count must not read as a disablement — but `0` is an explicit, supported disablement and is not a typo, so it turns saving off silently and on purpose. |
There was a problem hiding this comment.
P3: The rationale 'history: -1 ... pruned nothing, so the directory grew without bound' describes silent growth, but History#prune calls entries.drop(count) and Ruby's Array#drop(-1) raises ArgumentError, so a deploy with history: -1 would abort at prune after publishing the report, not silently accumulate. The rule (reject negatives) is still right, but the stated symptom is inaccurate for the current code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/review/config-and-secrets.md, line 16:
<comment>The rationale 'history: -1 ... pruned nothing, so the directory grew without bound' describes silent growth, but History#prune calls entries.drop(count) and Ruby's Array#drop(-1) raises ArgumentError, so a deploy with history: -1 would abort at prune after publishing the report, not silently accumulate. The rule (reject negatives) is still right, but the stated symptom is inaccurate for the current code.</comment>
<file context>
@@ -0,0 +1,27 @@
+- **Origin:** cubic learning 44aaf149; PR #157
+
+### A negative `report: history:` raises; `history: 0` does not
+- **Holds because:** `history: -1` made every deploy print "Deploy report unavailable" **and** pruned nothing, so the directory grew without bound. A typo-like count must not read as a disablement — but `0` is an explicit, supported disablement and is not a typo, so it turns saving off silently and on purpose.
+- **Where:** `lib/dash/configuration/report.rb#ensure_valid_history` (lines 60-64); `lib/dash/report/writer.rb#write` returns nil when `keep.zero?`
+- **Proven by:** `test/configuration/report_test.rb:41` ("a negative history is rejected rather than read as off"), `test/report/writer_test.rb:68` ("history: 0 writes nothing at all")
</file context>
Suggested change
| - **Holds because:** `history: -1` made every deploy print "Deploy report unavailable" **and** pruned nothing, so the directory grew without bound. A typo-like count must not read as a disablement — but `0` is an explicit, supported disablement and is not a typo, so it turns saving off silently and on purpose. | |
| - **Holds because:** `history: -1` is a typo-like count that must not silently read as "keep none", while `0` is an explicit, supported disablement that turns saving off silently and on purpose. |
|
|
||
| `Dash::Configuration::Proxy` (671 lines) holds the identity constants (`CONTAINER_NAME`, `NETWORK`, `CONFIG_VOLUME`, `IMAGE_TITLE`, their `LEGACY_*` twins, `LEGACY_RENAME_MARKER`) and the loadbalancer decision. | ||
|
|
||
| `effective_loadbalancer` (243-251) resolves in order: nothing unless the config is load-balanced at all; `false` when `loadbalancer: false`; the primary role's first host when `loadbalancer: true`; the named host when one is given; the primary role's first host when `auto_load_balanced_primary_role?`. **The last branch is the auto-activation**: a primary role with more than one web host turns the loadbalancer on without anyone writing it down, which is why multi-host test fixtures that are not testing it must set `loadbalancer: false` (`.claude/rules/testing.md`) and why `Dash::Cli::Main#print_config_banner` prints the reason when the key is absent. |
There was a problem hiding this comment.
P3: A multi-host primary role does not always auto-enable the loadbalancer; auto_load_balanced_primary_role? also requires primary_role.running_proxy?. Document that condition so non-proxied roles and their fixtures are not mischaracterized.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/configuration/summary.md, line 36:
<comment>A multi-host primary role does not always auto-enable the loadbalancer; `auto_load_balanced_primary_role?` also requires `primary_role.running_proxy?`. Document that condition so non-proxied roles and their fixtures are not mischaracterized.</comment>
<file context>
@@ -0,0 +1,60 @@
+
+`Dash::Configuration::Proxy` (671 lines) holds the identity constants (`CONTAINER_NAME`, `NETWORK`, `CONFIG_VOLUME`, `IMAGE_TITLE`, their `LEGACY_*` twins, `LEGACY_RENAME_MARKER`) and the loadbalancer decision.
+
+`effective_loadbalancer` (243-251) resolves in order: nothing unless the config is load-balanced at all; `false` when `loadbalancer: false`; the primary role's first host when `loadbalancer: true`; the named host when one is given; the primary role's first host when `auto_load_balanced_primary_role?`. **The last branch is the auto-activation**: a primary role with more than one web host turns the loadbalancer on without anyone writing it down, which is why multi-host test fixtures that are not testing it must set `loadbalancer: false` (`.claude/rules/testing.md`) and why `Dash::Cli::Main#print_config_banner` prints the reason when the key is absent.
+
+`Dash::Configuration::Proxy::Run` (341 lines) materialises the `dash-proxy run` invocation. `MINIMUM_VERSION` (`v1.1.0.1`) is the image tag the gem requires; `Rakefile`'s release task refuses to release while it is not pullable from `ghcr.io/zoolutions/dash-proxy`, and `bin/sync-proxy-flags` regenerates `test/fixtures/kamal_proxy_flags.yml` from the image so `test/proxy_flag_coverage_test.rb` catches drift between the gem's `deploy.yml` surface and the proxy's actual flags.
</file context>
Suggested change
| `effective_loadbalancer` (243-251) resolves in order: nothing unless the config is load-balanced at all; `false` when `loadbalancer: false`; the primary role's first host when `loadbalancer: true`; the named host when one is given; the primary role's first host when `auto_load_balanced_primary_role?`. **The last branch is the auto-activation**: a primary role with more than one web host turns the loadbalancer on without anyone writing it down, which is why multi-host test fixtures that are not testing it must set `loadbalancer: false` (`.claude/rules/testing.md`) and why `Dash::Cli::Main#print_config_banner` prints the reason when the key is absent. | |
| `effective_loadbalancer` (243-251) resolves in order: nothing unless the config is load-balanced at all; `false` when `loadbalancer: false`; the primary role's first host when `loadbalancer: true`; the named host when one is given; the primary role's first host when `auto_load_balanced_primary_role?`. **The last branch is the auto-activation**: a primary role that runs a proxy and has more than one host turns the loadbalancer on without anyone writing it down, which is why multi-host proxied test fixtures that are not testing it must set `loadbalancer: false` (`.claude/rules/testing.md`) and why `Dash::Cli::Main#print_config_banner` prints the reason when the key is absent. |
| @@ -0,0 +1,5 @@ | |||
| # dash | |||
|
|
|||
| A Ruby CLI that deploys containerised web apps to servers over SSH, published as the `dash` gem with `dash` as its executable and `Dash::` as its namespace. It began as a fork of basecamp/kamal and made a clean break in 2026-08 (issue #115): there is no `upstream` remote, no sync, and nothing is contributed back — a basecamp fix arrives only by deliberate cherry-pick (`.claude/rules/upstream-sync.md`). What dash adds over its origin is the measurement layer: every deploy times its own phases and the SSH round trips inside them (`Dash::Timings`), reads the buildx progress stream it was already printing to cost every build step (`Dash::Build::ProgressParser`), runs a rule set over the Dockerfile it just built (`Dash::Dockerfile::Analyzer`), compares the run with the last few saved ones (`Dash::Report::Trends`), and saves the whole thing as JSON under `.dash/reports`. | |||
There was a problem hiding this comment.
P3: When a project has only .kamal, this summary sends readers to the wrong report path. Document .dash/reports as the default and .kamal/reports as the legacy fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/summary.md, line 3:
<comment>When a project has only `.kamal`, this summary sends readers to the wrong report path. Document `.dash/reports` as the default and `.kamal/reports` as the legacy fallback.</comment>
<file context>
@@ -0,0 +1,5 @@
+# dash
+
+A Ruby CLI that deploys containerised web apps to servers over SSH, published as the `dash` gem with `dash` as its executable and `Dash::` as its namespace. It began as a fork of basecamp/kamal and made a clean break in 2026-08 (issue #115): there is no `upstream` remote, no sync, and nothing is contributed back — a basecamp fix arrives only by deliberate cherry-pick (`.claude/rules/upstream-sync.md`). What dash adds over its origin is the measurement layer: every deploy times its own phases and the SSH round trips inside them (`Dash::Timings`), reads the buildx progress stream it was already printing to cost every build step (`Dash::Build::ProgressParser`), runs a rule set over the Dockerfile it just built (`Dash::Dockerfile::Analyzer`), compares the run with the last few saved ones (`Dash::Report::Trends`), and saves the whole thing as JSON under `.dash/reports`.
+
+Three invariants govern changes. **The layer cake stays separated** — `Dash::Cli::*` parses options and orchestrates `on()`/`modify()` blocks, `Dash::Commands::*` builds argv arrays with no SSH in them, `Dash::Configuration::*` turns `deploy.yml` into objects, SSHKit executes (`CLAUDE.md`, `.claude/rules/coding-style.md`). **The proxy image comes before the gem** — `Dash::Configuration::Proxy::Run::MINIMUM_VERSION` must name a published, public `ghcr.io/zoolutions/dash-proxy` tag before a release, and `rake release` aborts when it is not pullable (`Rakefile:38-44`). **The report is a courtesy** — measuring, analysing, saving and shipping it all run inside rescues, so a broken report costs one yellow line and never a deploy; the one place report configuration does raise is `Dash::Configuration::Report`, at config-validation time, where an unrecognised `hadolint:` or a negative `history:` is a typo that must not read as "off" (`Dash::Configuration::Report#ensure_valid_hadolint_setting`, lines 52-56, and `#ensure_valid_history`, lines 60-64).
</file context>
Suggested change
| A Ruby CLI that deploys containerised web apps to servers over SSH, published as the `dash` gem with `dash` as its executable and `Dash::` as its namespace. It began as a fork of basecamp/kamal and made a clean break in 2026-08 (issue #115): there is no `upstream` remote, no sync, and nothing is contributed back — a basecamp fix arrives only by deliberate cherry-pick (`.claude/rules/upstream-sync.md`). What dash adds over its origin is the measurement layer: every deploy times its own phases and the SSH round trips inside them (`Dash::Timings`), reads the buildx progress stream it was already printing to cost every build step (`Dash::Build::ProgressParser`), runs a rule set over the Dockerfile it just built (`Dash::Dockerfile::Analyzer`), compares the run with the last few saved ones (`Dash::Report::Trends`), and saves the whole thing as JSON under `.dash/reports`. | |
| A Ruby CLI that deploys containerised web apps to servers over SSH, published as the `dash` gem with `dash` as its executable and `Dash::` as its namespace. It began as a fork of basecamp/kamal and made a clean break in 2026-08 (issue #115): there is no `upstream` remote, no sync, and nothing is contributed back — a basecamp fix arrives only by deliberate cherry-pick (`.claude/rules/upstream-sync.md`). What dash adds over its origin is the measurement layer: every deploy times its own phases and the SSH round trips inside them (`Dash::Timings`), reads the buildx progress stream it was already printing to cost every build step (`Dash::Build::ProgressParser`), runs a rule set over the Dockerfile it just built (`Dash::Dockerfile::Analyzer`), compares the run with the last few saved ones (`Dash::Report::Trends`), and saves the whole thing as JSON under the project report directory (`.dash/reports`, falling back to `.kamal/reports` for legacy projects). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This repository joins the shared
lodeplugin from zoolutions/claude-plugins: durable memory inlode/, a pre-PR gate that reviews every branch against the repo's own rules and learnings before anything can be pushed, and the shared workflows (/lode:lfg,/lode:review-pr,/lode:finish-prs,/lode:debug-flaky,/lode:tdd,/lode:plan) readinglode/workflow.md.lode/— summary, terminology, practices, map, and eight subsystem summaries (CLI, commands, configuration, Dockerfile advice, reports, observability, testing and CI, docs site). The Dockerfile parser gets a 24-row grammar table, one row per form handled or deliberately approximated, each with its safe failure direction and itsparser_test.rbcitation. Every claim cites a file and method; every count was computed (10 subcommands, 15 analyzer rules, 12 dependency installs, 97 unit-test files with 1,926 tests, 7 CI cells).lode/review/— 94 rules in eight files, from all 50 cubic learnings for this repo and the review threads of merged PRs test: stop the unit suite depending on a running Docker daemon #133 through fix(proxy): route every container-creating path through the stage-3c bridge #169. Duplicates across the two sources were merged; five reasoned rejections are recorded as Not a bug; 55 rules cite the test that proves them and the rest say "no test".lode/workflow.md(128 lines) — the profile.lfg,plan,tdd,github-review-pr,github-review-failures,github-review-comments,finish-prs,debug-flaky. Kept:architect,security,perf,review-pr..claude/settings.jsonenableslode@zoolutions;CLAUDE.mdgains a Memory section and the new command rows;.gitignoregainslode/tmp/.No Ruby changes. Two things surfaced that need their own PRs: #170, the
RUBYOPTfrozen-string-literal expression inci.ymlhas never matched (startsWith(matrix.ruby-version, '3.4.')against"3.4"); andCLAUDE.md's "stage 3c has not shipped" story is stale:lib/dash/configuration/proxy.rbhas carried thedash-proxy/dash/dash-proxy-confignames withLEGACY_*twins and a migration bridge since #130. Four smaller doc-versus-code gaps are recorded in the lode where they apply.Test plan
bundle exec ruby -Itest test/dockerfile/parser_test.rb22 runs green; the unit-only one-liner run in full (1,926 runs, no Docker);bin/release --dry-run;bundle exec bin/dash help/lode:lfgon the fix for ci.yml's frozen-string-literal RUBYOPT never applies: startsWith(matrix.ruby-version, '3.4.') against "3.4" #170 reads the profile and reaches the gateDeviations & judgment calls
/lode:seed, resumed once after a rate limit (every draft re-verified; twelve wrong ranges and one fabricated citation in them fixed), then gated here.LODE_SKIP_GATE=1after the gate passed: the orchestrating session runs the cached 0.1.0 push hook keyed on a different checkout. A session opened here will not need it.Gate
Rounds: 1. Findings: 0 P1, 0 P2, 2 P3 (one fixed, one corrected here). Rejected 0, deferred 0.
https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF
Summary by cubic
Adds the shared
lodeplugin's durable memory (lode/) and pre-PR gate, so future changes start from documented knowledge of the code instead of re-deriving it. The eight local commands (lfg,plan,tdd,github-review-*,finish-prs,debug-flaky) are retired in favor of the shared/lode:*workflows; no Ruby changes.The lode
lode/holds a summary, terminology, practices, a workflow profile, and eight subsystem summaries; every count and line range is computed from source, and each claim cites a file and method.lode/review/holds 94 accepted review findings as rules across eight files, sourced from cubic learnings and merged-PR threads; five rejections are recorded as Not a bug, and 55 rules cite the test that proves them.lode/workflow.mdis the profile the shared workflows read;CLAUDE.mdgains a Memory section,.claude/settings.jsonenableslode@zoolutions, and.gitignoreadds/lode/tmp/.Side effects
RUBYOPTcheck inci.ymlnever matched (ci.yml's frozen-string-literal RUBYOPT never applies: startsWith(matrix.ruby-version, '3.4.') against "3.4" #170), andCLAUDE.md's "stage 3c has not shipped" story is stale since feat(proxy): rename container identity to dash and migrate hosts across #130.LODE_SKIP_GATE=1because the push hook runs against a different checkout.Written for commit fba6a44. Summary will update on new commits.