fix(dashmate): re-sync stale 3.x Drive and rs-dapi images#3898
fix(dashmate): re-sync stale 3.x Drive and rs-dapi images#3898thepastaclaw wants to merge 4 commits into
Conversation
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Changes4.0.0-rc.3 Docker image migration and tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/dashmate/test/unit/config/configFile/migrate3xTo4xRcImages.spec.js`:
- Around line 26-31: The test is missing coverage for the `:3-rc` image variant
even though the migration predicate matches it alongside `dev` and `hotfix`. Add
constants for the rc variants (STALE_DRIVE_RC and STALE_RS_DAPI_RC) following
the same pattern as the existing STALE_DRIVE_DEV and STALE_RS_DAPI_DEV
constants. Then extend the test matrix around line 142-145 to include test cases
for these new rc variant constants, ensuring the migration correctly handles the
rc versions just as it does for dev and hotfix variants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ea52087-7c6e-4e68-9174-d84d8fca3052
📒 Files selected for processing (2)
packages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dashmate/test/unit/config/configFile/migrate3xTo4xRcImages.spec.js
|
@coderabbitai review |
✅ Action performedReview finished.
|
thepastaclaw
force-pushed
the
fix-dashmate-rc-image-migration
branch
from
2ce01c6 to
e11795d
Compare
|
✅ Review complete (commit ced758c) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Targeted dashmate migration that re-keys the 3.x→4.0.0-rc image re-sync to 4.0.0-rc.3 (since the previous 4.0.0-rc.2 keying was a no-op for users already on rc.2) and extends coverage to the :3-hotfix tag series. Both agents independently found no in-scope correctness, security, or coverage issues, and the regression test file gives thorough coverage of the prerelease label series plus custom/missing-config edge cases.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
thepastaclaw
force-pushed
the
fix-dashmate-rc-image-migration
branch
from
e11795d to
086187c
Compare
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Prior review at e11795d had zero findings, so there is nothing to carry forward. Both agents independently re-reviewed head 086187c and reported no in-scope issues. Verified directly: the 4.0.0-rc.3 migration uses anchored regexes ^dashpay/drive:3(?:-(?:dev|rc|hotfix))?$ and ^dashpay/rs-dapi:3(?:-(?:dev|rc|hotfix))?$ to limit rewrites to the four dashmate-shipped stale 3.x tag series, gates writes on defaultConfig.has(...), uses optional chaining to tolerate missing dapi/rsDapi sections, and is re-keyed at 4.0.0-rc.3 to escape the same-version short-circuit from the original #3889 attempt. Scope matches the stated goal; no findings.
…c upgrade (dashpay#3889) A node already on 3.0.1 skipped the 3.0.0 migration that used to re-sync the Drive ABCI and rs-dapi docker images, and the intervening 3.0.1 / 3.0.2 / 3.1.0 migrations only touched Core / Gateway / Tenderdash. After `dashmate update` to 4.0.0-rc.x the config kept the old protocol-11 images (`dashpay/drive:3`, `dashpay/rs-dapi:3`), so the node stayed on protocol 11 and crash-looped after protocol 12 activation. Add a `4.0.0-rc.2` migration that re-syncs both image fields from the current default config (matching the 3.0.0 migration convention), and a focused regression spec that constructs the migration without the full DI container so it doesn't depend on `@dashevo/wasm-dpp` being built. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply review polish on the regression added in 686e995: - Match the sibling 3.0.0 migration's single-line `defaultConfig.get(...)` style in the migration body. - Hoist expected default images into `beforeEach`; remove an unused `customImages` helper parameter. - Parameterise the two `fromVersion` cases (3.0.1 and 3.1.0) so both real-world entry points are exercised by one assertion shape. - Pin against the regression with `expect(...).to.not.equal('dashpay/drive:3')` (and rs-dapi:3) so the test keeps failing even if a future default happens to match the stale value again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…custom images The 4.0.0-rc.2 entry added in dashpay#3889 was both unreachable for already- stamped rc.2 configs (migrateConfigFile short-circuits when fromVersion === toVersion) and unconditional: it overwrote every Drive ABCI / rs-dapi image with the current default, clobbering private forks and vendor-patched builds. Re-key the migration at 4.0.0-rc.3 (the next release; same flow as the 3.0.2 Envoy CVE - dashpay#3794 added the migration, dashpay#3796 cut the release) and only rewrite the stale dashmate-shipped tags (dashpay/drive:3, dashpay/rs-dapi:3, including their -dev/-rc series), mirroring the 3.0.2 Envoy CVE migration's predicate style. Custom images are preserved. The migration fires for both 3.0.x -> 4.0.0-rc.x upgrades (the loop runs every migration with key > fromVersion) and 4.0.0-rc.2 -> 4.0.0-rc.3 once a chore(release) bumps the package version. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous predicate alternation only matched the `-dev` and `-rc`
prerelease label series for `dashpay/drive:3` / `dashpay/rs-dapi:3`,
missing the `-hotfix` tag shipped by 3.0.1-hotfix.{1..4} (dashpay#3020/dashpay#3044/
dashpay#3055/dashpay#3060) and 3.1.0-hotfix.1. A node sitting on `:3-hotfix` would
still crash-loop after protocol 12 activation — the exact failure mode
the migration was meant to prevent. Extend the alternation and add a
regression case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
thepastaclaw
force-pushed
the
fix-dashmate-rc-image-migration
branch
from
086187c to
ced758c
Compare
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-forward prior findings: none; the prior verified output was clean and there is nothing to reconcile. New findings in the latest delta: the Tenderdash custom-image clobber is real in the checked-out code, but it was introduced by upstream base commit 1157ca7 (#3940), not by PR #3898's diff against the current base, so there are no in-scope findings for this PR.
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Issue being fixed or feature implemented
Fixes #3889.
Dashmate configs upgraded from the 3.x line to 4.0.0-rc.2 could keep stale protocol-11 Drive ABCI and rs-dapi Docker image tags (
dashpay/drive:3*,dashpay/rs-dapi:3*). Those images crash-loop after protocol 12 activation.What was done?
4.0.0-rc.3so configs already stamped4.0.0-rc.2can be repaired on the next release bump instead of being skipped by the same-version short-circuit.:3,:3-dev,:3-rc, and:3-hotfix.How Has This Been Tested?
Environment:
/Users/claw/Projects/platform/worktrees/fix-dashmate-rc-image-migrationnpx -p node@22because local system Node 25 hits an EBADF loader issue before the spec runs.Validation:
npx -y -p node@22 node .yarn/releases/yarn-4.12.0.cjs workspace dashmate exec mocha test/unit/config/configFile/migrate3xTo4xRcImages.spec.jsResult:
11 passing (87ms).Pre-PR review gate:
Result:
Recommendation: ship.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit