fix: skip duplicate Analysis stage in approve & upgrade flow

[jrangelramos]

Stacked on #15 — this PR is rebased on top of the Proposal → AgenticRun rename. The diff will show all changes from #15 until it merges; after that, only this PR's single commit will remain.

Summary

• When an ApprovalPolicy auto-approves the Analysis stage, clicking "Approve & upgrade" failed with Duplicate value: {"type":"Analysis"} because approveStage('Analysis') was called unconditionally
• Now checks if the Analysis stage already exists in the AgenticRunApproval before calling approveStage, preventing the duplicate key API error

Test plan

• With an ApprovalPolicy that auto-approves Analysis, click "Approve & upgrade" — should succeed without duplicate error
• Without an ApprovalPolicy (manual approval), click "Approve & upgrade" — should still call approveStage('Analysis') normally
• yarn test passes (56/56)

🤖 Generated with Claude Code

[jhadvig]

Thanks for catching this @jrangelramos 🏆 the bug is real and the fix is correct for the DecisionActions path.

However, there's a second unguarded caller of approveStage('Analysis') in UpdatePlanTab.tsx:177 (handleAnalyse) that has the same problem.... if an ApprovalPolicy auto-approves Analysis, the "Analyse" button in the Active Update Plans tab will also hit the duplicate error.

Rather than adding the same guard at every call site, could we move the dedup check into useApprovalActions.approveStage() itself? A one-liner at the top of the function:

const existingStages = currentApproval.spec?.stages ?? [];
if (existingStages.some((s) => s.type === stageType)) return true;  // already approved

This makes approveStage idempotent... calling it when the stage already exists just returns true , which is the right semantic for all callers, current and future.

[jhadvig]

LGTM 👍 the idempotent guard in the hook looks great, thanks for moving it there @jrangelramos

One small nit.... approval?.spec?.stages in the handleApproveClick dependency array (DecisionActions.tsx:114) is no longer needed since the guard moved into the hook. It won't break anything, just unnecessary callback re-creation when stages change. Feel free to drop it if you're doing another push, otherwise no big deal.

[jrangelramos]

just dropped that! thanks

[jrangelramos]

@jhadvig could you PTAL? thanks

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jrangelramos
Once this PR has been reviewed and has the lgtm label, please assign wking for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jrangelramos]

/hold

[jrangelramos]

wait for #15 to get merged

[openshift-ci]

@jrangelramos: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-scos-images	d62271d	link	true	/test okd-scos-images

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.