Skip to content

fix(deploy): use server group's last_revision for incremental deploys#19

Merged
facundofarias merged 2 commits into
mainfrom
fix/deploy-group-incremental
Jun 1, 2026
Merged

fix(deploy): use server group's last_revision for incremental deploys#19
facundofarias merged 2 commits into
mainfrom
fix/deploy-group-incremental

Conversation

@facundofarias
Copy link
Copy Markdown
Contributor

@facundofarias facundofarias commented Jun 1, 2026

Summary

  • dhq deploy -s "Group Name" always did a fresh full deploy from the first commit, ignoring the group's prior deployment history.
  • Root cause: when -s resolved to a server group, resolvedServer was set to nil and the group was never captured, so resolveStartRevision returned "" — which the API interprets as "deploy entire branch from the first commit".
  • Fix: capture resolvedGroup *sdk.ServerGroup alongside resolvedServer for both name- and UUID-based group resolution, and use ServerGroup.LastRevision / ServerGroup.PreferredBranch when no server is resolved. Matches the Rails-side ServerGroup#last_revision semantics (end_revision of the most recent group-targeted deployment).
  • Newly-created groups still fall through to a full deploy because their LastRevision is empty — preserving the expected first-deploy behaviour.

Reported by

Customer follow-up to #11 / DHQ-586: name resolution now works, but the resulting deploy ignores the previous commit and starts fresh.

Test plan

  • go vet ./... clean
  • golangci-lint run ./... — 0 issues
  • go test ./... — full suite passes
  • 6 new unit tests cover the group paths:
    • TestResolveStartRevision_GroupLastRevisionUsedWhenNoServer
    • TestResolveStartRevision_GroupIgnoredWhenServerHasLastRevision
    • TestResolveStartRevision_FreshGroupReturnsEmpty
    • TestResolveStartRevision_FullClearsGroupLastRevision
    • TestResolveBranchAndRevision_UsesGroupPreferredBranch
    • TestResolveBranchAndRevision_FetchesGroupWhenServerFails
    • TestResolveBranchAndRevision_ServerPreferredBranchBeatsGroup
  • Manual smoke against staging: dhq deploy -p <project> -s "<group name>" — verify the new deployment's start_revision matches the group's previous end_revision.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Deployment now properly recognizes and targets server groups as well as individual servers, improving branch selection during deploys.
  • Bug Fixes
    • Incremental deployment baseline logic corrected for group deployments so start revisions are chosen correctly, avoiding unnecessary full deployments.

Deploying to a server group via `dhq deploy -s "Group Name"` always
performed a full deploy from the first commit, ignoring the group's
prior deployment history.

When the `-s` value resolved to a group, the code set
`resolvedServer = nil` and never captured the group, so
`resolveStartRevision` (which only looked at `Server.LastRevision`)
returned `""`. The API treats an empty `start_revision` as "deploy
entire branch from the first commit" — exactly the fresh-deploy
behaviour the customer reported.

Capture `resolvedGroup *sdk.ServerGroup` alongside `resolvedServer` so
both name- and UUID-based group resolution flow into the same
incremental path used for individual servers. `ServerGroup.LastRevision`
is the `end_revision` of the most recent group-targeted deployment
(Rails: `ServerGroup#last_revision`), which is the correct incremental
baseline. Newly-created groups have an empty value so the first group
deploy still falls through to a full deploy.

Also use `ServerGroup.PreferredBranch` when no server is resolved, so
group deploys honour the group's configured branch instead of silently
deploying the repository default branch's tip.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 96ad839e-86ed-460a-a247-1421b0967abf

📥 Commits

Reviewing files that changed from the base of the PR and between 04a4555 and 2a509c6.

📒 Files selected for processing (1)
  • internal/commands/deploy_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/commands/deploy_test.go

Walkthrough

Deploy command extended to support both Server and ServerGroup targets. Branch and start-revision resolution functions now accept pre-resolved groups. Command flow tracks and resolves group identifiers, fetching missing data eagerly. Test infrastructure and coverage updated to validate group-aware branch selection, incremental baselines, and resolution precedence.

Changes

Server group deployment support

Layer / File(s) Summary
Branch and start-revision resolution logic
internal/commands/deploy.go
resolveBranchAndRevision accepts a pre-resolved ServerGroup and falls back to group PreferredBranch or Branch when available. resolveStartRevision accepts a group parameter and returns group.LastRevision when available for incremental deployments. Updated documentation clarifies the fallback chain for branch selection.
Deploy command flow refactoring
internal/commands/deploy.go
Deploy command tracks both resolvedServer and resolvedGroup variables. During target name resolution, group-matching path sets resolvedGroup, clears resolvedServer, and updates the server identifier. Eager-fetch populates either resolvedServer or resolvedGroup based on what resolves. Both resolved values flow into branch/start-revision resolution and StartRevision computation.
Test infrastructure and signature updates
internal/commands/deploy_test.go
Test mux extended with server-group endpoint mappings and GET /server_groups/:id route handler. All existing resolveBranchAndRevision test calls updated to pass the new knownGroup parameter (set to nil for server-only paths).
Test coverage for server-group resolution and incremental behavior
internal/commands/deploy_test.go
New and updated tests for resolveStartRevision validate nil-target behavior, group LastRevision baseline usage, --full and --start-revision override precedence, and server LastRevision fallback. Branch resolution tests extended to cover group preferred_branch honoring, group fetch after server 404, and precedence when both server and group preferred branches are present.

Possibly related PRs

  • deployhq/deployhq-cli#11: Both PRs modify deploy target resolution to correctly handle server-group inputs; this PR extends resolution to track both server and group separately and uses both for branch/start-revision selection.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: enabling server group's last_revision for incremental deploys, which directly addresses the root cause described in the PR objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/deploy-group-incremental

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
internal/commands/deploy_test.go (1)

429-454: ⚡ Quick win

Consider adding branchesCalled assertion for consistency.

The equivalent server test (TestResolveBranchAndRevision_UsesServerPreferredBranch at line 118) explicitly verifies branchesCalled is true. Adding the same assertion here would improve test explicitness and pattern consistency.

Proposed assertion to add

After line 453, add:

 	assert.Equal(t, "sha-of-release", revision, "must be release's tip, not main's")
+	assert.True(t, mux.branchesCalled, "should query /branches to map branch→sha")
 	assert.False(t, mux.latestRevCalled, "should NOT fall back to repo default")
🤖 Prompt for 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.

In `@internal/commands/deploy_test.go` around lines 429 - 454, Add an assertion to
verify the branches endpoint was called by asserting the branchEndpointMux
instance's branchesCalled flag is true; in
TestResolveBranchAndRevision_UsesGroupPreferredBranch (where mux is the
branchEndpointMux) add something like assert.True(t, mux.branchesCalled, "should
call branches endpoint") just before or after the existing latestRevCalled
assertion to match the pattern used in
TestResolveBranchAndRevision_UsesServerPreferredBranch.
🤖 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.

Nitpick comments:
In `@internal/commands/deploy_test.go`:
- Around line 429-454: Add an assertion to verify the branches endpoint was
called by asserting the branchEndpointMux instance's branchesCalled flag is
true; in TestResolveBranchAndRevision_UsesGroupPreferredBranch (where mux is the
branchEndpointMux) add something like assert.True(t, mux.branchesCalled, "should
call branches endpoint") just before or after the existing latestRevCalled
assertion to match the pattern used in
TestResolveBranchAndRevision_UsesServerPreferredBranch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1ecc05ff-df65-4711-9958-37df4d53d6b5

📥 Commits

Reviewing files that changed from the base of the PR and between c5cb8e5 and 04a4555.

📒 Files selected for processing (2)
  • internal/commands/deploy.go
  • internal/commands/deploy_test.go

Mirrors the equivalent assertion in TestResolveBranchAndRevision_UsesServerPreferredBranch
so the test explicitly verifies the branch→sha lookup path was taken, not only
that the repo-default fallback was skipped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@facundofarias facundofarias merged commit 82c1e4e into main Jun 1, 2026
3 checks passed
@facundofarias facundofarias deleted the fix/deploy-group-incremental branch June 1, 2026 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant