Skip to content

chore: support full rendered diff for new version#1072

Merged
adityachoudhari26 merged 1 commit intomainfrom
argo-plan-new-versions-supported
Apr 28, 2026
Merged

chore: support full rendered diff for new version#1072
adityachoudhari26 merged 1 commit intomainfrom
argo-plan-new-versions-supported

Conversation

@adityachoudhari26
Copy link
Copy Markdown
Member

@adityachoudhari26 adityachoudhari26 commented Apr 28, 2026

fixes #1068

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced error resilience in the planning process: missing current application states no longer cause planning failures. The planner now continues successfully and properly detects changes against an empty current state.
  • Tests

    • Added comprehensive test coverage for planning operations when current manifests are unavailable.

Copilot AI review requested due to automatic review settings April 28, 2026 16:29
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 96f0ec9c-baa8-474c-ace2-97f5e55a6384

📥 Commits

Reviewing files that changed from the base of the PR and between c31a58b and d7d11b5.

📒 Files selected for processing (2)
  • apps/workspace-engine/pkg/jobagents/argo/argoapp_test.go
  • apps/workspace-engine/pkg/jobagents/argo/argocd_plan.go

📝 Walkthrough

Walkthrough

The changes modify the Argo planner to treat missing current manifests (gRPC NotFound responses) as a valid state instead of an error condition, allowing plan operations to proceed by comparing proposed manifests against an empty baseline. A test case is added to verify this behavior.

Changes

Cohort / File(s) Summary
Planner Error Handling
apps/workspace-engine/pkg/jobagents/argo/argocd_plan.go
Modified the current manifest retrieval logic to treat gRPC codes.NotFound as a valid state by setting currentManifests to nil and continuing with diff computation, rather than returning an error.
Test Coverage
apps/workspace-engine/pkg/jobagents/argo/argoapp_test.go
Added new test case verifying that planning succeeds when fetching current manifests fails with codes.NotFound, ensuring HasChanges is true, Current is empty, and the temporary plan application is deleted exactly once.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 First versions feared the gRPC abyss,
NotFound meant failure—oh what a diss!
But now it's a state, not an error to dread,
The planner compares against nothing instead,
And Argo's first plans finally exist! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'chore: support full rendered diff for new version' is partially related but doesn't clearly convey the main change—handling missing current manifests for first deployments. Consider a more descriptive title like 'Handle missing current manifests in Argo plans for new deployments' to better reflect the technical change.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR correctly addresses issue #1068 by treating gRPC NotFound errors as valid states, allowing plans to proceed with missing current manifests and return full rendered manifests as the diff.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the linked issue—modifications to handle missing manifests in the planner and a corresponding test case covering the new behavior.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch argo-plan-new-versions-supported

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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes ArgoCD plan generation for deployments with no existing versions by treating “current manifests” as empty when the underlying ArgoCD application doesn’t exist yet, allowing the plan to return a full rendered diff (all proposed manifests).

Changes:

  • Handle grpc/codes.NotFound from GetManifests for the current app by continuing with an empty current manifest set.
  • Add a unit test covering the “first version / current app not found” behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
apps/workspace-engine/pkg/jobagents/argo/argocd_plan.go Treats NotFound when fetching current manifests as “no current manifests” so the diff returns all proposed output.
apps/workspace-engine/pkg/jobagents/argo/argoapp_test.go Adds coverage ensuring plans succeed and return full proposed output when current manifests return NotFound.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@adityachoudhari26 adityachoudhari26 merged commit 5b0d27b into main Apr 28, 2026
14 checks passed
@adityachoudhari26 adityachoudhari26 deleted the argo-plan-new-versions-supported branch April 28, 2026 16:36
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.

Argo plans do not work when creating the first version

2 participants