feat: rename approval policies to approval rules and fix approvals decoding - #477
Open
octopus-hideaki wants to merge 3 commits into
Open
feat: rename approval policies to approval rules and fix approvals decoding#477octopus-hideaki wants to merge 3 commits into
octopus-hideaki wants to merge 3 commits into
Conversation
The server renamed this feature: the ApprovalPolicy resource is now
ApprovalRule and the endpoint moved from ~/api/{spaceId}/approvalpolicies
to ~/api/{spaceId}/approvalrules. Approval rule ids are now ApprovalRules-N
and scope ids ApprovalRuleTagScopes-N / ApprovalRuleIdScopes-N.
Adds pkg/approvalrules and removes pkg/approvalpolicies. The old package is
removed rather than deprecated because its endpoint no longer exists on any
current server, so it cannot function. Callers rename the package and its
types; every field keeps its name, type and default.
Also adds the new TenantApprovalStrategy field, which determines how
tenanted deployments are gated: PerRelease (the default) uses one change
request for all tenants of a release, PerTenant gives each tenant its own.
Verified end-to-end against a live instance: list, get, create with
PerTenant, update and delete.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GetByID expected a ServerTaskApproval envelope, but the endpoint returns the resource directly, so it silently returned (nil, nil). Decode the bare resource instead. Adds the ApproverUsername, ApproverDisplayName and ApproverEmailAddress fields the server now returns on an approval. Documents the change request grouping: deployments are gated per change request, several server tasks can share one, and approval votes are tallied across every server task approval sharing a ChangeRequest.Id. As a result ListApprovals and GetByTaskID return votes for the whole change request, so entries may carry a sibling ServerTaskApprovalId. Also documents the resulting write failures, including one vote per user per change request. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MinimumApproversRequired was always serialized, so a rule built without it sent zero and the server rejected the request as out of range. The field is optional on the API, where omitting it applies the default of 2, and zero is never valid, so omit it when unset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bec-callow-oct
approved these changes
Sep 4, 2026
bec-callow-oct
left a comment
Contributor
There was a problem hiding this comment.
Tested locally and found no issues
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
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.
Brings the client in line with the server's approvals rework. Follow-up to #426.
Fixes: HPY-1572
Important
This removes
pkg/approvalpoliciesand replaces it withpkg/approvalrules. That is a breaking change to the API published in v2.113.0, but the feature was in closed alpha and the old endpoint no longer exists on any current server, so the removed package could not function. It is deliberately not marked as a semver breaking change.The rename
The server renamed the feature:
ApprovalPolicy→ApprovalRule, and the endpoint moved from~/api/{spaceId}/approvalpoliciesto~/api/{spaceId}/approvalrules. Ids are nowApprovalRules-N, and scope idsApprovalRuleTagScopes-N/ApprovalRuleIdScopes-N.pkg/approvalrulesmirrors the previous package one-for-one — every field keeps its JSON name, type, optionality and default — so migrating is a rename of the package and its types.Tenant approval strategy
New
TenantApprovalStrategyfield on the rule, withTenantApprovalStrategyPerRelease(the default) andTenantApprovalStrategyPerTenant. It determines how tenanted deployments are gated: one change request covering all tenants of a release, or one per tenant. When several rules match a deployment the most restrictive wins.Change requests
Deployments are now gated per change request, and several server tasks can share one. The public API surface is unchanged — votes are still cast against a server task approval — but the semantics around it are not, so the runtime package changes are documentation plus two corrections:
ApproverUsername,ApproverDisplayName, andApproverEmailAddressfields that the server now returns on an approval.ListApprovalsandGetByTaskIDreturn votes for the whole change request, deduplicated to one per user with rejection winning — so entries may carry a siblingServerTaskApprovalId. Also documents the resulting write failures, including one vote per user per change request.Testing
Unit tests for the new package (including tenant strategy round-tripping), the approver fields, the bare get-by-id decode, and cross-change-request vote lists.
Verified end-to-end against a live instance on
2026.3.15282: approval rules list/get/create-with-PerTenant/update/delete, plus approver details decoding from real vote data.🤖 Generated with Claude Code