Skip to content

perf: use date comparison to see if version should allowed if created before approval rule#753

Merged
adityachoudhari26 merged 1 commit intomainfrom
approval-evaluator-perf
Jan 20, 2026
Merged

perf: use date comparison to see if version should allowed if created before approval rule#753
adityachoudhari26 merged 1 commit intomainfrom
approval-evaluator-perf

Conversation

@adityachoudhari26
Copy link
Copy Markdown
Member

@adityachoudhari26 adityachoudhari26 commented Jan 20, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Fixed approval policy evaluation to automatically approve versions created prior to when the approval policy was established, eliminating unnecessary approval requirements for legacy versions.
  • Refactor

    • Enhanced policy initialization to properly record and persist policy creation timestamps, enabling the approval evaluation system to accurately determine version eligibility based on policy timing.
  • Tests

    • Updated test coverage to reflect the new automatic approval behavior for pre-policy versions.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 20, 2026

📝 Walkthrough

Walkthrough

The changes implement timestamp-based approval evaluation by introducing policy creation timestamps that are propagated to rules, and modifying the approval evaluator to auto-approve versions created before the policy was established instead of checking deployment history.

Changes

Cohort / File(s) Summary
Approval Evaluator
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/approval/approval.go
Added ruleCreatedAt field to struct, replaced release history lookup with timestamp comparison logic. Versions created before policy now auto-approved; added error handling for timestamp parsing with pending result on failure.
Approval Tests
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/approval/approval_test.go
Renamed test from TestAnyApprovalEvaluator_AlreadyDeployed to TestAnyApprovalEvaluator_OlderVersionAllowed; updated assertion message to reflect pre-policy version semantics.
Policy Storage
apps/workspace-engine/pkg/workspace/store/policy.go
Added time package import. Initialize policy CreatedAt to current RFC3339 timestamp when empty; propagate timestamp to rules and populate rule.PolicyId when missing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • chore: policy bypass #724: Directly modifies the same AnyApprovalEvaluator struct and NewEvaluator constructor, introducing overlapping changes to approval evaluation logic.
  • feat: ws engine gradual rollout #658: Uses VersionAnyApproval rules whose evaluation behavior is altered by the timestamp-based pre-policy version handling introduced here.

Poem

🐰 A policy born with timestamp in hand,
Now rules remember when they took their stand,
Old versions smile—they predate the law,
Approved before approval's need was drawn. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing release store queries with date comparison logic to determine if versions created before an approval rule should be automatically allowed.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

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

@adityachoudhari26 adityachoudhari26 merged commit 39c55a3 into main Jan 20, 2026
9 checks passed
@adityachoudhari26 adityachoudhari26 deleted the approval-evaluator-perf branch January 20, 2026 22:46
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