From fbea67e451dd7b55eddda8ca8d2395f98e850bca Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Fri, 17 Apr 2026 14:12:04 -0700 Subject: [PATCH] refactor: only send single plan comment to release pr --- apps/workspace-engine/pkg/config/env.go | 3 + .../pkg/db/queries/workspaces.sql | 5 +- .../workspace-engine/pkg/db/workspaces.sql.go | 16 + .../controllers/deploymentplan/controller.go | 15 + .../deploymentplan/controller_test.go | 7 + .../svc/controllers/deploymentplan/getters.go | 1 + .../deploymentplan/getters_postgres.go | 7 + .../deploymentplan/github_comment.go | 200 +++++++++++ .../deploymentplanresult/controller.go | 42 --- .../deploymentplanresult/github_comment.go | 313 ------------------ 10 files changed, 253 insertions(+), 356 deletions(-) create mode 100644 apps/workspace-engine/svc/controllers/deploymentplan/github_comment.go delete mode 100644 apps/workspace-engine/svc/controllers/deploymentplanresult/github_comment.go diff --git a/apps/workspace-engine/pkg/config/env.go b/apps/workspace-engine/pkg/config/env.go index d4869a3510..28dbe792c3 100644 --- a/apps/workspace-engine/pkg/config/env.go +++ b/apps/workspace-engine/pkg/config/env.go @@ -35,6 +35,9 @@ type Config struct { GithubBotAppID string `default:"" envconfig:"GITHUB_BOT_APP_ID"` GithubBotPrivateKey string `default:"" envconfig:"GITHUB_BOT_PRIVATE_KEY"` + // Public URL of the ctrlplane web app, used in PR comment links. + BaseURL string `default:"https://app.ctrlplane.dev" envconfig:"BASE_URL"` + PostgresURL string `default:"postgresql://ctrlplane:ctrlplane@localhost:5432/ctrlplane" envconfig:"POSTGRES_URL"` PostgresMaxPoolSize int `default:"50" envconfig:"POSTGRES_MAX_POOL_SIZE"` PostgresApplicationName string `default:"workspace-engine" envconfig:"POSTGRES_APPLICATION_NAME"` diff --git a/apps/workspace-engine/pkg/db/queries/workspaces.sql b/apps/workspace-engine/pkg/db/queries/workspaces.sql index e85afc9332..c2e9683e58 100644 --- a/apps/workspace-engine/pkg/db/queries/workspaces.sql +++ b/apps/workspace-engine/pkg/db/queries/workspaces.sql @@ -2,4 +2,7 @@ SELECT EXISTS(SELECT 1 FROM workspace WHERE id = $1) AS exists; -- name: ListWorkspaceIDs :many -SELECT id FROM workspace; \ No newline at end of file +SELECT id FROM workspace; + +-- name: GetWorkspaceByID :one +SELECT id, name, slug, created_at FROM workspace WHERE id = $1; \ No newline at end of file diff --git a/apps/workspace-engine/pkg/db/workspaces.sql.go b/apps/workspace-engine/pkg/db/workspaces.sql.go index 491963e749..424fc682e7 100644 --- a/apps/workspace-engine/pkg/db/workspaces.sql.go +++ b/apps/workspace-engine/pkg/db/workspaces.sql.go @@ -11,6 +11,22 @@ import ( "github.com/google/uuid" ) +const getWorkspaceByID = `-- name: GetWorkspaceByID :one +SELECT id, name, slug, created_at FROM workspace WHERE id = $1 +` + +func (q *Queries) GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Workspace, error) { + row := q.db.QueryRow(ctx, getWorkspaceByID, id) + var i Workspace + err := row.Scan( + &i.ID, + &i.Name, + &i.Slug, + &i.CreatedAt, + ) + return i, err +} + const listWorkspaceIDs = `-- name: ListWorkspaceIDs :many SELECT id FROM workspace ` diff --git a/apps/workspace-engine/svc/controllers/deploymentplan/controller.go b/apps/workspace-engine/svc/controllers/deploymentplan/controller.go index 15e5bbacaf..427b8bb561 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplan/controller.go +++ b/apps/workspace-engine/svc/controllers/deploymentplan/controller.go @@ -127,9 +127,24 @@ func (c *Controller) Process(ctx context.Context, item reconcile.Item) (reconcil } } + if commentErr := c.commentPlanLink(ctx, plan); commentErr != nil { + span.RecordError(commentErr) + } + return reconcile.Result{}, nil } +func (c *Controller) commentPlanLink( + ctx context.Context, + plan db.DeploymentPlan, +) error { + workspace, err := c.getter.GetWorkspaceByID(ctx, plan.WorkspaceID) + if err != nil { + return fmt.Errorf("get workspace: %w", err) + } + return MaybeCommentPlanLink(ctx, plan, workspace.Slug) +} + func (c *Controller) processTarget( ctx context.Context, plan db.DeploymentPlan, diff --git a/apps/workspace-engine/svc/controllers/deploymentplan/controller_test.go b/apps/workspace-engine/svc/controllers/deploymentplan/controller_test.go index e4cc9f3ee9..f57d7825d4 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplan/controller_test.go +++ b/apps/workspace-engine/svc/controllers/deploymentplan/controller_test.go @@ -87,6 +87,13 @@ func (m *mockGetter) ListJobAgentsByWorkspaceID( return m.workspaceAgents, nil } +func (m *mockGetter) GetWorkspaceByID( + _ context.Context, + id uuid.UUID, +) (db.Workspace, error) { + return db.Workspace{ID: id, Slug: "test-workspace"}, nil +} + type insertTargetCall struct { PlanID, EnvID, ResourceID uuid.UUID } diff --git a/apps/workspace-engine/svc/controllers/deploymentplan/getters.go b/apps/workspace-engine/svc/controllers/deploymentplan/getters.go index aa710241ee..7608c386c1 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplan/getters.go +++ b/apps/workspace-engine/svc/controllers/deploymentplan/getters.go @@ -24,6 +24,7 @@ type Getter interface { GetResource(ctx context.Context, id uuid.UUID) (*oapi.Resource, error) GetJobAgent(ctx context.Context, id uuid.UUID) (*oapi.JobAgent, error) ListJobAgentsByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) ([]oapi.JobAgent, error) + GetWorkspaceByID(ctx context.Context, id uuid.UUID) (db.Workspace, error) } // VarResolver resolves deployment variables for a release target. diff --git a/apps/workspace-engine/svc/controllers/deploymentplan/getters_postgres.go b/apps/workspace-engine/svc/controllers/deploymentplan/getters_postgres.go index cf937bf577..8af0b32740 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplan/getters_postgres.go +++ b/apps/workspace-engine/svc/controllers/deploymentplan/getters_postgres.go @@ -86,6 +86,13 @@ func (g *PostgresGetter) GetJobAgent(ctx context.Context, id uuid.UUID) (*oapi.J return db.ToOapiJobAgent(row), nil } +func (g *PostgresGetter) GetWorkspaceByID( + ctx context.Context, + id uuid.UUID, +) (db.Workspace, error) { + return db.GetQueries(ctx).GetWorkspaceByID(ctx, id) +} + type PostgresVarResolver struct { getter variableresolver.Getter } diff --git a/apps/workspace-engine/svc/controllers/deploymentplan/github_comment.go b/apps/workspace-engine/svc/controllers/deploymentplan/github_comment.go new file mode 100644 index 0000000000..ffbbd93f6e --- /dev/null +++ b/apps/workspace-engine/svc/controllers/deploymentplan/github_comment.go @@ -0,0 +1,200 @@ +package deploymentplan + +import ( + "context" + "fmt" + "strings" + + "github.com/google/go-github/v66/github" + "github.com/google/uuid" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "workspace-engine/pkg/config" + "workspace-engine/pkg/db" + gh "workspace-engine/pkg/github" +) + +const ( + metaGitHubOwner = "github/owner" + metaGitHubRepo = "github/repo" + metaGitSHA = "git/sha" +) + +func planCommentMarker(planID uuid.UUID) string { + return fmt.Sprintf("", planID) +} + +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() +} + +// MaybeCommentPlanLink posts or updates a PR comment linking to the plan +// detail page. It requires the following keys in plan.VersionMetadata: +// +// - "github/owner" — GitHub repository owner +// - "github/repo" — GitHub repository name +// - "git/sha" — commit SHA used to find the associated PR +// +// Returns nil (no-op) if any key is missing, the GitHub bot is not +// configured, or no PR is found for the SHA. +func MaybeCommentPlanLink( + ctx context.Context, + plan db.DeploymentPlan, + workspaceSlug string, +) error { + ctx, span := tracer.Start(ctx, "MaybeCommentPlanLink") + defer span.End() + + span.SetAttributes(attribute.String("plan_id", plan.ID.String())) + + owner := plan.VersionMetadata[metaGitHubOwner] + repo := plan.VersionMetadata[metaGitHubRepo] + sha := plan.VersionMetadata[metaGitSHA] + + span.SetAttributes( + attribute.String("github.owner", owner), + attribute.String("github.repo", repo), + attribute.String("git.sha", sha), + ) + + if owner == "" || repo == "" || sha == "" { + span.AddEvent("skipped: missing github metadata") + return nil + } + + client, err := gh.CreateClientForRepo(ctx, owner, repo) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "create github client") + return fmt.Errorf("create github client: %w", err) + } + if client == nil { + span.AddEvent("skipped: github bot not configured") + return nil + } + + prNumber, err := findPRForSHA(ctx, client, owner, repo, sha) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "find PR for SHA") + return fmt.Errorf("find PR for SHA %s: %w", sha, err) + } + if prNumber == 0 { + span.AddEvent("skipped: no PR found for SHA") + return nil + } + span.SetAttributes(attribute.Int("github.pr_number", prNumber)) + + marker := planCommentMarker(plan.ID) + body := buildPlanCommentBody(marker, config.Global.BaseURL, workspaceSlug, plan) + + if err := upsertPlanComment(ctx, client, owner, repo, prNumber, marker, body); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "upsert comment") + return fmt.Errorf("upsert comment on PR #%d: %w", prNumber, err) + } + + span.AddEvent("comment upserted") + return nil +} + +func findPRForSHA( + ctx context.Context, + client *github.Client, + owner, repo, sha string, +) (int, error) { + prs, _, err := client.PullRequests.ListPullRequestsWithCommit( + ctx, owner, repo, sha, &github.ListOptions{PerPage: 1}, + ) + if err != nil { + return 0, fmt.Errorf("list PRs for commit %s: %w", sha, err) + } + for _, pr := range prs { + if pr.GetState() == "open" { + return pr.GetNumber(), nil + } + } + if len(prs) > 0 { + return prs[0].GetNumber(), nil + } + return 0, nil +} + +func findMarkerComment( + ctx context.Context, + client *github.Client, + owner, repo string, + prNumber int, + marker string, +) (*github.IssueComment, error) { + opts := &github.IssueListCommentsOptions{ + ListOptions: github.ListOptions{PerPage: 100}, + } + for { + comments, resp, err := client.Issues.ListComments( + ctx, owner, repo, prNumber, opts, + ) + if err != nil { + return nil, fmt.Errorf("list comments: %w", err) + } + for _, c := range comments { + if strings.Contains(c.GetBody(), marker) { + return c, nil + } + } + if resp.NextPage == 0 { + return nil, nil + } + opts.Page = resp.NextPage + } +} + +func upsertPlanComment( + ctx context.Context, + client *github.Client, + owner, repo string, + prNumber int, + marker, body string, +) error { + existing, err := findMarkerComment(ctx, client, owner, repo, prNumber, marker) + if err != nil { + return err + } + + if existing != nil { + _, _, err := client.Issues.EditComment( + ctx, owner, repo, existing.GetID(), + &github.IssueComment{Body: &body}, + ) + if err != nil { + return fmt.Errorf("edit comment: %w", err) + } + return nil + } + + _, _, err = client.Issues.CreateComment( + ctx, owner, repo, prNumber, + &github.IssueComment{Body: &body}, + ) + if err != nil { + return fmt.Errorf("create comment: %w", err) + } + return nil +} diff --git a/apps/workspace-engine/svc/controllers/deploymentplanresult/controller.go b/apps/workspace-engine/svc/controllers/deploymentplanresult/controller.go index 921bd67341..868680444d 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplanresult/controller.go +++ b/apps/workspace-engine/svc/controllers/deploymentplanresult/controller.go @@ -95,20 +95,6 @@ func (c *Controller) Process(ctx context.Context, item reconcile.Item) (reconcil return reconcile.Result{}, fmt.Errorf("mark result unsupported: %w", updateErr) } - if commentErr := MaybeCommentOnPR( - ctx, - &dispatchCtx, - result.TargetID.String(), - prCommentResult{ - AgentID: dispatchCtx.JobAgent.Id, - AgentName: dispatchCtx.JobAgent.Name, - AgentType: agentType, - Status: "unsupported", - }, - ); commentErr != nil { - span.RecordError(commentErr) - } - return reconcile.Result{}, nil } @@ -133,21 +119,6 @@ func (c *Controller) Process(ctx context.Context, item reconcile.Item) (reconcil ) } - if commentErr := MaybeCommentOnPR( - ctx, - &dispatchCtx, - result.TargetID.String(), - prCommentResult{ - AgentID: dispatchCtx.JobAgent.Id, - AgentName: dispatchCtx.JobAgent.Name, - AgentType: agentType, - Status: "errored", - Message: err.Error(), - }, - ); commentErr != nil { - span.RecordError(commentErr) - } - return reconcile.Result{}, nil } @@ -198,19 +169,6 @@ func (c *Controller) Process(ctx context.Context, item reconcile.Item) (reconcil return reconcile.Result{}, fmt.Errorf("save completed result: %w", err) } - if commentErr := MaybeCommentOnPR(ctx, &dispatchCtx, result.TargetID.String(), prCommentResult{ - AgentID: dispatchCtx.JobAgent.Id, - AgentName: dispatchCtx.JobAgent.Name, - AgentType: agentType, - Status: "completed", - HasChanges: planResult.HasChanges, - Current: planResult.Current, - Proposed: planResult.Proposed, - Message: planResult.Message, - }); commentErr != nil { - span.RecordError(commentErr) - } - return reconcile.Result{}, nil } diff --git a/apps/workspace-engine/svc/controllers/deploymentplanresult/github_comment.go b/apps/workspace-engine/svc/controllers/deploymentplanresult/github_comment.go deleted file mode 100644 index 395d9f36c6..0000000000 --- a/apps/workspace-engine/svc/controllers/deploymentplanresult/github_comment.go +++ /dev/null @@ -1,313 +0,0 @@ -package deploymentplanresult - -import ( - "context" - "fmt" - "strings" - - "github.com/google/go-github/v66/github" - "github.com/pmezard/go-difflib/difflib" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/codes" - "go.opentelemetry.io/otel/trace" - gh "workspace-engine/pkg/github" - "workspace-engine/pkg/oapi" -) - -const ( - metaGitHubOwner = "github/owner" - metaGitHubRepo = "github/repo" - metaGitSHA = "git/sha" -) - -type prCommentResult struct { - AgentID string - AgentName string - AgentType string - Status string - HasChanges bool - Current string - Proposed string - Message string -} - -func commentMarker(targetID string) string { - return fmt.Sprintf("", targetID) -} - -func agentSectionStart(agentID string) string { - return fmt.Sprintf("", agentID) -} - -func agentSectionEnd(agentID string) string { - return fmt.Sprintf("", agentID) -} - -func wrapAgentSection(agentID, content string) string { - return agentSectionStart(agentID) + "\n" + content + agentSectionEnd(agentID) + "\n" -} - -func replaceOrAppendAgentSection(body, agentID, section string) string { - start := agentSectionStart(agentID) - end := agentSectionEnd(agentID) - - startIdx := strings.Index(body, start) - endIdx := strings.Index(body, end) - if startIdx >= 0 && endIdx >= 0 && endIdx > startIdx { - return body[:startIdx] + wrapAgentSection(agentID, section) + body[endIdx+len(end):] - } - - return body + "\n" + wrapAgentSection(agentID, section) -} - -func formatResultSection(r prCommentResult) string { - var sb strings.Builder - fmt.Fprintf(&sb, "**%s** · `%s`\n", r.AgentName, r.AgentType) - - switch r.Status { - case "errored": - fmt.Fprintf(&sb, "> ❌ Error: %s\n", r.Message) - return sb.String() - case "unsupported": - fmt.Fprintf(&sb, "> ⚠️ Agent does not support plan operations\n") - return sb.String() - } - - if !r.HasChanges { - sb.WriteString("> No changes\n") - return sb.String() - } - - diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ - A: difflib.SplitLines(r.Current), - B: difflib.SplitLines(r.Proposed), - FromFile: "current", - ToFile: "proposed", - Context: 3, - }) - if err != nil { - diff = "Failed to compute diff" - } - - sb.WriteString("
\nChanges detected\n\n") - sb.WriteString("```diff\n") - sb.WriteString(diff) - sb.WriteString("```\n") - sb.WriteString("
\n") - - return sb.String() -} - -func buildComment(marker string, dispatchCtx *oapi.DispatchContext, sections []string) string { - var sb strings.Builder - sb.WriteString(marker) - sb.WriteString("\n") - - resourceName := "unknown" - envName := "unknown" - if dispatchCtx.Resource != nil { - resourceName = dispatchCtx.Resource.Name - } - if dispatchCtx.Environment != nil { - envName = dispatchCtx.Environment.Name - } - - fmt.Fprintf(&sb, "#### %s · %s\n\n", resourceName, envName) - sb.WriteString(strings.Join(sections, "\n")) - - return sb.String() -} - -// MaybeCommentOnPR posts or updates a PR comment with plan results for a -// resource. It requires the following keys in DeploymentVersion.Metadata: -// -// - "github/owner" — GitHub repository owner (e.g. "wandb") -// - "github/repo" — GitHub repository name (e.g. "deployments") -// - "git/sha" — full commit SHA used to find the associated PR -// -// Returns nil (no-op) if any key is missing, the GitHub bot is not -// configured, or no PR is found for the SHA. -func MaybeCommentOnPR( - ctx context.Context, - dispatchCtx *oapi.DispatchContext, - targetID string, - result prCommentResult, -) error { - ctx, span := tracer.Start(ctx, "MaybeCommentOnPR") - defer span.End() - - span.SetAttributes(attribute.String("target_id", targetID)) - - if dispatchCtx.Version == nil { - span.AddEvent("skipped: no version in dispatch context") - return nil - } - meta := dispatchCtx.Version.Metadata - owner := meta[metaGitHubOwner] - repo := meta[metaGitHubRepo] - sha := meta[metaGitSHA] - - span.SetAttributes( - attribute.String("github.owner", owner), - attribute.String("github.repo", repo), - attribute.String("git.sha", sha), - ) - - if owner == "" || repo == "" || sha == "" { - span.AddEvent("skipped: missing github metadata", - trace.WithAttributes( - attribute.Bool("has_owner", owner != ""), - attribute.Bool("has_repo", repo != ""), - attribute.Bool("has_sha", sha != ""), - ), - ) - return nil - } - - client, err := gh.CreateClientForRepo(ctx, owner, repo) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, "create github client") - return fmt.Errorf("create github client: %w", err) - } - if client == nil { - span.AddEvent("skipped: github bot not configured") - return nil - } - span.AddEvent("github client created") - - prNumber, err := findPRForSHA(ctx, client, owner, repo, sha) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, "find PR for SHA") - return fmt.Errorf("find PR for SHA %s: %w", sha, err) - } - if prNumber == 0 { - span.AddEvent("skipped: no PR found for SHA") - return nil - } - span.SetAttributes(attribute.Int("github.pr_number", prNumber)) - span.AddEvent("found PR") - - marker := commentMarker(targetID) - section := formatResultSection(result) - - if err := upsertComment( - ctx, - client, - owner, - repo, - prNumber, - marker, - dispatchCtx, - result.AgentID, - section, - ); err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, "upsert comment") - return fmt.Errorf("upsert comment on PR #%d: %w", prNumber, err) - } - - span.AddEvent("comment upserted") - return nil -} - -func findPRForSHA( - ctx context.Context, - client *github.Client, - owner, repo, sha string, -) (int, error) { - prs, _, err := client.PullRequests.ListPullRequestsWithCommit( - ctx, owner, repo, sha, &github.ListOptions{PerPage: 1}, - ) - if err != nil { - return 0, fmt.Errorf("list PRs for commit %s: %w", sha, err) - } - for _, pr := range prs { - if pr.GetState() == "open" { - return pr.GetNumber(), nil - } - } - if len(prs) > 0 { - return prs[0].GetNumber(), nil - } - return 0, nil -} - -func findMarkerComment( - ctx context.Context, - client *github.Client, - owner, repo string, - prNumber int, - marker string, -) (*github.IssueComment, error) { - opts := &github.IssueListCommentsOptions{ - ListOptions: github.ListOptions{PerPage: 100}, - } - for { - comments, resp, err := client.Issues.ListComments( - ctx, owner, repo, prNumber, opts, - ) - if err != nil { - return nil, fmt.Errorf("list comments: %w", err) - } - for _, c := range comments { - if strings.Contains(c.GetBody(), marker) { - return c, nil - } - } - if resp.NextPage == 0 { - return nil, nil - } - opts.Page = resp.NextPage - } -} - -func upsertComment( - ctx context.Context, - client *github.Client, - owner, repo string, - prNumber int, - marker string, - dispatchCtx *oapi.DispatchContext, - agentID string, - newSection string, -) error { - ctx, span := tracer.Start(ctx, "upsertComment") - defer span.End() - - existing, err := findMarkerComment(ctx, client, owner, repo, prNumber, marker) - if err != nil { - span.RecordError(err) - return err - } - - if existing != nil { - span.AddEvent("updating existing comment", - trace.WithAttributes(attribute.Int64("comment_id", existing.GetID())), - ) - updated := replaceOrAppendAgentSection(existing.GetBody(), agentID, newSection) - _, _, err := client.Issues.EditComment( - ctx, owner, repo, existing.GetID(), - &github.IssueComment{Body: &updated}, - ) - if err != nil { - span.RecordError(err) - return fmt.Errorf("edit comment: %w", err) - } - return nil - } - - span.AddEvent("creating new comment") - wrapped := wrapAgentSection(agentID, newSection) - body := buildComment(marker, dispatchCtx, []string{wrapped}) - _, _, err = client.Issues.CreateComment( - ctx, owner, repo, prNumber, - &github.IssueComment{Body: &body}, - ) - if err != nil { - span.RecordError(err) - return fmt.Errorf("create comment: %w", err) - } - return nil -}