Fix dev deployment

[lukaszgryglicki]

Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io

Assisted by OpenAI

Assisted by GitHub Copilot

Assisted by Claude

[coderabbitai]

Walkthrough

The DEV deployment workflow now triggers on closed pull requests targeting dev; the deploy job derives DD_VERSION from the PR merge commit SHA when available, checks out the merged commit, enforces concurrency (group deploy-dev), and only runs for merged PRs (or non-PR events).

Changes

Dev Deployment Workflow

Layer / File(s)	Summary
Pull request event trigger configuration .github/workflows/deploy-dev.yml	Workflow trigger configuration is extended to include pull_request events of type closed targeting dev, in addition to existing push and manual dispatch.
Deploy job execution with merge commit handling .github/workflows/deploy-dev.yml	Job computes DD_VERSION from the PR's merge_commit_sha when present, adds a concurrency group (deploy-dev) to prevent overlapping deploys, gates execution to merged PRs only (for PR events), and updates actions/checkout to reference the merged commit (falling back to github.sha) and sets persist-credentials: false.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix dev deployment' is vague and generic, using non-specific language that doesn't convey meaningful details about what was actually fixed in the workflow.	Provide a more specific title that describes the actual workflow changes, such as 'Update dev deployment workflow to handle PR merges with concurrency controls' or similar.
Description check	❓ Inconclusive	The description lacks any meaningful information about the changeset, containing only sign-off and AI assistance attribution without describing what was changed or why.	Add a description that explains the workflow changes, their purpose, and any relevant context for reviewers to understand the modifications.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch unicron-fix-dev-deployment

---

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

[Copilot]

Pull request overview

This PR adjusts the DEV deployment GitHub Actions workflow to deploy the actual merged commit on dev (instead of the synthetic PR merge ref), and adds additional triggers/guards intended to support deployments on merge and manual dispatch.

Changes:

• Add pull_request: closed and workflow_dispatch triggers for the dev deployment workflow.
• Use github.event.pull_request.merge_commit_sha (fallback to github.sha) for DD_VERSION and actions/checkout ref.
• Add workflow-level concurrency and gate the deploy job to only run on merged PR closures.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/deploy-dev.yml:
- Around line 10-14: The workflow currently defines both a pull_request trigger
(types: [closed], branches: - dev) and a push trigger (branches: - dev) so
merged PRs run twice; remove the pull_request trigger block (or alternatively
remove the push trigger) so only one of the triggers remains, and ensure the
job-level conditional (if: github.event_name != 'pull_request' ||
github.event.pull_request.merged == true) is still correct for the remaining
trigger to avoid duplicate runs.
- Around line 42-46: In the deploy-dev workflow's build-deploy-dev job, replace
the floating actions/checkout@v4 with a pinned commit SHA and add
persist-credentials: false under the checkout step; specifically, update the
uses entry for actions/checkout to a specific commit (instead of `@v4`) and add a
with block containing persist-credentials: false while keeping the existing ref:
${{ github.event.pull_request.merge_commit_sha || github.sha }} so checkout does
not write credentials into .git/config when using OIDC
(id-token/role-to-assume).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dbf07200-3869-47f9-a3c6-a1c12fb61ba1

📥 Commits

Reviewing files that changed from the base of the PR and between cdfef47 and 5c1d630.

📒 Files selected for processing (1)

• .github/workflows/deploy-dev.yml

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

.github/workflows/deploy-dev.yml (1)

29-32: 💤 Low value

cancel-in-progress: true may leave partial deployments if cancelled mid-run.

If both triggers fire (should push events start working again), the first deploy could be cancelled mid-way—after some lambdas are updated but before others—leaving services in an inconsistent state until the second deploy completes. Consider whether cancel-in-progress: false (serialize instead of cancel) would be safer for deployment workflows, or add a comment noting this is an accepted trade-off while push events remain unreliable.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/deploy-dev.yml around lines 29 - 32, The concurrency
setting currently uses cancel-in-progress: true which can abort an in-progress
deploy partway and leave partial lambda updates; change cancel-in-progress to
false on the concurrency block (group: deploy-dev) to serialize runs instead of
cancelling, or if cancelling is an accepted trade-off, add an explicit comment
above the concurrency block explaining the decision and risks so reviewers know
this is intentional.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/deploy-dev.yml:
- Line 29: Fix the typo in the workflow comment that currently reads "nsure
duplicate deploys triggered by both push and pull_request:closed for the same
merge are cancelled instead of queued back-to-back." — update "nsure" to
"Ensure" so the comment reads "Ensure duplicate deploys triggered by both push
and pull_request:closed for the same merge are cancelled instead of queued
back-to-back." This change targets the comment text "nsure duplicate deploys
triggered by both push and pull_request:closed..." in the workflow file.

---

Nitpick comments:
In @.github/workflows/deploy-dev.yml:
- Around line 29-32: The concurrency setting currently uses cancel-in-progress:
true which can abort an in-progress deploy partway and leave partial lambda
updates; change cancel-in-progress to false on the concurrency block (group:
deploy-dev) to serialize runs instead of cancelling, or if cancelling is an
accepted trade-off, add an explicit comment above the concurrency block
explaining the decision and risks so reviewers know this is intentional.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 89074c02-4aab-4bf8-af8d-d7f4da0d65ec

📥 Commits

Reviewing files that changed from the base of the PR and between 5c1d630 and 3a7e46f.

📒 Files selected for processing (1)

• .github/workflows/deploy-dev.yml

[Copilot]

Pull request overview

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/deploy-dev.yml (1)

231-231: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Cypress job condition references wrong event name.

This condition checks github.event_name != 'pull_request', but the trigger is now pull_request_target. For pull_request_target events, this condition is always true (since the event name is pull_request_target, not pull_request), making the fork check dead code.

The security risk is mitigated by the merged == true gate upstream (line 34), but the condition should be updated for clarity.

Suggested fix

-    if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false }}
+    if: ${{ github.event_name != 'pull_request_target' || github.event.pull_request.head.repo.fork == false }}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/deploy-dev.yml at line 231, The Cypress job's if-condition
currently only checks github.event_name != 'pull_request', which misses the
'pull_request_target' trigger and makes the fork check moot; update the
conditional expression that uses github.event_name to also account for
'pull_request_target' (e.g., treat both pull_request and pull_request_target the
same) so that github.event.pull_request.head.repo.fork is only evaluated for PR
contexts, keeping the existing fork check
(github.event.pull_request.head.repo.fork) intact and consistent with the
upstream merged gate.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In @.github/workflows/deploy-dev.yml:
- Line 231: The Cypress job's if-condition currently only checks
github.event_name != 'pull_request', which misses the 'pull_request_target'
trigger and makes the fork check moot; update the conditional expression that
uses github.event_name to also account for 'pull_request_target' (e.g., treat
both pull_request and pull_request_target the same) so that
github.event.pull_request.head.repo.fork is only evaluated for PR contexts,
keeping the existing fork check (github.event.pull_request.head.repo.fork)
intact and consistent with the upstream merged gate.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d6ece192-d93e-4112-b4df-ba1dcf84b962

📥 Commits

Reviewing files that changed from the base of the PR and between 7ab3e53 and 0477547.

📒 Files selected for processing (1)

• .github/workflows/deploy-dev.yml

[Copilot]

Pull request overview

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