Skip to content

feat: rename approval policies to approval rules and fix approvals decoding - #477

Open
octopus-hideaki wants to merge 3 commits into
mainfrom
hideakimurakami/approval-rules-rename
Open

feat: rename approval policies to approval rules and fix approvals decoding#477
octopus-hideaki wants to merge 3 commits into
mainfrom
hideakimurakami/approval-rules-rename

Conversation

@octopus-hideaki

@octopus-hideaki octopus-hideaki commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Brings the client in line with the server's approvals rework. Follow-up to #426.

Fixes: HPY-1572

Important

This removes pkg/approvalpolicies and replaces it with pkg/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: ApprovalPolicyApprovalRule, and the endpoint moved from ~/api/{spaceId}/approvalpolicies to ~/api/{spaceId}/approvalrules. Ids are now ApprovalRules-N, and scope ids ApprovalRuleTagScopes-N / ApprovalRuleIdScopes-N.

pkg/approvalrules mirrors 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 TenantApprovalStrategy field on the rule, with TenantApprovalStrategyPerRelease (the default) and TenantApprovalStrategyPerTenant. 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:

  • Adds the ApproverUsername, ApproverDisplayName, and ApproverEmailAddress fields that the server now returns on an approval.
  • Documents that ListApprovals and GetByTaskID return votes for the whole change request, deduplicated to one per user with rejection winning — so entries may carry a sibling ServerTaskApprovalId. 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

octopus-hideaki and others added 2 commits September 2, 2026 15:20
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>
@octopus-hideaki
octopus-hideaki requested a review from a team September 2, 2026 03:29
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 bec-callow-oct left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested locally and found no issues

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.

2 participants