Skip to content

approvals/v1: add auto-approve policy fields#53

Closed
haasonsaas wants to merge 1 commit intomainfrom
codex/proto-approvals-auto-approve-habits
Closed

approvals/v1: add auto-approve policy fields#53
haasonsaas wants to merge 1 commit intomainfrom
codex/proto-approvals-auto-approve-habits

Conversation

@haasonsaas
Copy link
Copy Markdown
Contributor

Summary

  • add workspace-level auto-approve policy settings to approvals.v1.ApprovalPolicy
  • expose approved_count on ApprovalHabit for UI confidence displays
  • extend RequestApprovalResponse to surface the auto-approved decision path

Testing

  • make generate
  • go test ./gen/go/approvals/v1/...

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 14, 2026

PR Summary

Medium Risk
This is a schema/API change to approvals.v1 that alters generated client/server types; while fields are additive (proto3-compatible), downstream services and consumers may need updates to handle the new auto-approve decision path.

Overview
Adds workspace-level auto-approval configuration to ApprovalPolicy (auto_approve_enabled, auto_approve_threshold, min_observations, excluded_risk_levels).

Extends ApprovalHabit with approved_count, and updates RequestApprovalResponse to optionally return the resulting ApprovalDecision plus an auto_approved flag so callers can distinguish automatic vs manual approval outcomes.

Regenerates Go/Python/TypeScript protobuf outputs to reflect the updated schema.

Reviewed by Cursor Bugbot for commit 9fc6bc2. Bugbot is set up for automated code reviews on this repo. Configure here.

@haasonsaas
Copy link
Copy Markdown
Contributor Author

Review: overlap with #56

This PR and #56 both address #55 (auto-approve fields). They touch identical files and will conflict. Key design differences:

Auto-approve config modeling

PR #53 PR #56
Config shape Flat fields on ApprovalPolicy (auto_approve_enabled, auto_approve_threshold, min_observations, excluded_risk_levels — fields 3-6) Dedicated AutoApproveConfig message nested in ApprovalPolicy.auto_approve_config (field 3)
Response shape RequestApprovalResponse gets decision (field 2) + auto_approved bool (field 3) RequestApprovalResponse gets AutoApproveEvidence message (field 2)
Decision enum No new enum value Adds DECISION_TYPE_AUTO_APPROVED = 5

Recommendation: prefer #56's approach

  1. Nested message > flat fields on policy. AutoApproveConfig is a cohesive concept. Flat fields on ApprovalPolicy (3-6) pollute the policy message and can't be passed independently. A nested message can be nil-checked cleanly and extended without touching the parent.

  2. DECISION_TYPE_AUTO_APPROVED enum value is important. Consumers need to distinguish auto-approved from human-approved in switch statements, audit logs, and metrics. A bool doesn't carry through the decision pipeline — it only exists on the response.

  3. Evidence > bool. AutoApproveEvidence (pattern, confidence, observation_count, threshold_applied) provides the audit trail. A bare auto_approved: true tells you what happened but not why — critical for compliance and debugging.

Suggest closing this PR in favor of #56, which also adds approved_count to ApprovalHabit (same as this PR).

@haasonsaas
Copy link
Copy Markdown
Contributor Author

Closing in favor of #56, which covers the same scope with a stronger design: nested AutoApproveConfig message (cleaner nil-check, independently passable), DECISION_TYPE_AUTO_APPROVED enum value (critical for downstream switch exhaustiveness), and AutoApproveEvidence message (proper audit trail). Both PRs add approved_count to ApprovalHabit. Field numbers conflict, so only one can merge.

@haasonsaas haasonsaas closed this Apr 14, 2026
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