refactor: only send single plan comment to release pr#1009
refactor: only send single plan comment to release pr#1009adityachoudhari26 merged 1 commit intomainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 27 minutes and 17 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors GitHub PR commenting for deployment plans to post a single comment that links to the Ctrlplane plan detail page, instead of posting/updating per-target plan result comments.
Changes:
- Remove per-target PR commenting from
deploymentplanresultprocessing (and delete the old comment implementation). - Add a new
deploymentplanPR comment writer that upserts one “plan link” comment per plan. - Add supporting plumbing:
BASE_URLconfig, workspace lookup (GetWorkspaceByID) to obtain the workspace slug, and a new sqlc query.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/workspace-engine/svc/controllers/deploymentplanresult/github_comment.go | Removes the old per-target/agent PR comment implementation. |
| apps/workspace-engine/svc/controllers/deploymentplanresult/controller.go | Stops emitting PR comments from plan-result processing paths. |
| apps/workspace-engine/svc/controllers/deploymentplan/github_comment.go | Adds PR comment upsert logic that posts a single plan link comment. |
| apps/workspace-engine/svc/controllers/deploymentplan/controller.go | Calls new plan-link commenting helper after processing targets. |
| apps/workspace-engine/svc/controllers/deploymentplan/getters.go | Extends Getter interface with GetWorkspaceByID. |
| apps/workspace-engine/svc/controllers/deploymentplan/getters_postgres.go | Implements GetWorkspaceByID for Postgres-backed getter. |
| apps/workspace-engine/svc/controllers/deploymentplan/controller_test.go | Updates mocks to satisfy the extended Getter interface. |
| apps/workspace-engine/pkg/db/workspaces.sql.go | Adds generated sqlc method GetWorkspaceByID. |
| apps/workspace-engine/pkg/db/queries/workspaces.sql | Adds GetWorkspaceByID query definition. |
| apps/workspace-engine/pkg/config/env.go | Introduces BaseURL (env BASE_URL) for building PR comment links. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func buildPlanCommentBody( | ||
| marker, baseURL, workspaceSlug string, | ||
| plan db.DeploymentPlan, | ||
| ) string { | ||
| planURL := fmt.Sprintf( | ||
| "%s/%s/deployments/%s/plans/%s", | ||
| strings.TrimRight(baseURL, "/"), | ||
| workspaceSlug, | ||
| plan.DeploymentID, | ||
| plan.ID, | ||
| ) | ||
|
|
||
| var sb strings.Builder | ||
| sb.WriteString(marker) | ||
| sb.WriteString("\n") | ||
| sb.WriteString("### Ctrlplane Deployment Plan\n\n") | ||
| fmt.Fprintf(&sb, "**Version:** `%s`\n\n", plan.VersionTag) | ||
| fmt.Fprintf(&sb, "[View plan →](%s)\n", planURL) | ||
| return sb.String() |
There was a problem hiding this comment.
The PR-comment link generation is new behavior, but there’s no unit test coverage validating the constructed URL/comment body (e.g., baseURL trimming, workspace slug + deploymentId + planId path). Adding a focused test around buildPlanCommentBody (and/or marker format) would help prevent regressions and accidental route mismatches.
| workspace, err := c.getter.GetWorkspaceByID(ctx, plan.WorkspaceID) | ||
| if err != nil { | ||
| return fmt.Errorf("get workspace: %w", err) | ||
| } | ||
| return MaybeCommentPlanLink(ctx, plan, workspace.Slug) |
There was a problem hiding this comment.
commentPlanLink always queries GetWorkspaceByID even when PR commenting will be a no-op (e.g., missing github/owner/github/repo/git/sha in plan.VersionMetadata or when the GitHub bot isn’t configured). This adds an extra DB round-trip per plan unnecessarily. Consider short-circuiting before the workspace lookup when the required metadata/bot config is absent, and only fetching the workspace slug when you’re actually going to attempt the GitHub upsert.
No description provided.