chore: ensures that associate-pr action fetches it from proper ref#2819
Merged
chore: ensures that associate-pr action fetches it from proper ref#2819
Conversation
Contributor
📝 WalkthroughWalkthroughChange the associated-pr action to accept a Changes
Sequence Diagram(s)sequenceDiagram
participant WF as Workflow
participant AP as associated-pr Action
participant GH as GitHub API
participant SN as Slack Notification Action
WF->>AP: invoke with ref (e.g., app/vX.Y.Z)
AP->>GH: resolve ref -> commit SHA
GH-->>AP: commit SHA
AP->>GH: listPullRequestsAssociatedWithCommit(sha)
alt PR found
GH-->>AP: PR(s)
AP->>GH: if merged & missing merged_by -> pulls.get(pr_number)
GH-->>AP: PR details
AP-->>WF: set PR outputs (number,title,head_ref,head_repo,labels,closed,slack_user)
WF->>SN: consume outputs to build message
else no PR / retries exhausted
AP-->>WF: warn and return early (no PR outputs set)
WF->>SN: send fallback notification (no PR)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/console-api-release.yml (1)
60-79:⚠️ Potential issue | 🔴 Critical
needs.setupis inaccessible — pipeline failure confirmed.
test-beta-sandboxonly listsneeds: deploy-beta-sandbox, soneeds.setup.outputs.image_tagat Line 75 is out of scope. This matches the reported linting error.setupmust be added to the job's directneeds, mirroring the fix already applied inconsole-web-release.yml.🐛 Proposed fix
test-beta-sandbox: permissions: contents: read pull-requests: read - needs: deploy-beta-sandbox + needs: + - setup + - deploy-beta-sandbox name: Test beta sandbox🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/console-api-release.yml around lines 60 - 79, The job test-beta-sandbox references needs.setup.outputs.image_tag but only declares needs: deploy-beta-sandbox, causing the output to be inaccessible; update the test-beta-sandbox job to include setup in its needs array (i.e., add setup alongside deploy-beta-sandbox) so needs.setup.outputs.image_tag is in scope, keeping the rest of the job (runs-on, steps, env, with) unchanged.
♻️ Duplicate comments (1)
.github/workflows/all-ci-unsafe.yml (1)
113-117: Same raw-SHA concern asauto-approval.yml.
${{ github.event.workflow_run.head_sha }}is passed asref, while theassociated-praction's ref-to-SHA resolution step is designed for named tag refs. Confirm the action handles SHA inputs as noted in theauto-approval.ymlcomment above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/all-ci-unsafe.yml around lines 113 - 117, The associated-pr step is being passed a raw SHA via the ref input (id associated_pr, uses ./.github/actions/associated-pr), but the action's ref-to-SHA resolution expects named tag/branch refs; update the associated-pr action to accept raw SHAs (detect a 40‑char hex and skip ref-to-SHA resolution) or normalize the workflow input by passing a branch/ref instead (e.g., use github.event.workflow_run.head_branch or construct refs/heads/<branch>) so that associate_pr's ref handling (the ref-to-SHA resolution step) correctly handles both named refs and raw SHA inputs.
🧹 Nitpick comments (1)
.github/workflows/provider-proxy-release.yml (1)
106-121: Notest-prod-mainnetjob exists for the prod-mainnet deployment.The workflow tests beta-sandbox, beta-mainnet, and prod-sandbox, but
deploy-prod-mainnethas no corresponding test job — unlike the symmetrictest-prod-sandboxthat followsdeploy-prod-sandbox. This is a pre-existing gap, not introduced by this PR, but worth tracking.Would you like me to open a separate issue to track adding
test-prod-mainnetfor consistency with the other deployment lanes?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/provider-proxy-release.yml around lines 106 - 121, The workflow is missing a test-prod-mainnet job and deploy-prod-mainnet doesn't depend on a pre-deploy test; add a new job named test-prod-mainnet modeled after test-prod-sandbox/test-beta-mainnet (same permissions, steps, and using the same test matrix for prod/mainnet), then update the needs array of deploy-prod-mainnet (in the deploy-prod-mainnet job) to include test-prod-mainnet so the prod mainnet deployment waits for its test to complete.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/associated-pr/action.yml:
- Around line 69-70: The code currently assumes process.env.INPUT_REF is already
an immutable commit SHA when calling core.info and core.setOutput; instead
detect if INPUT_REF is not a 40-char SHA and resolve it to a commit SHA first
(e.g., run git rev-parse --verify or call Octokit/repos.getCommit with the
provided ref) and then call core.info and core.setOutput with the resolved SHA;
update the logic around core.info(`Using provided ref as sha:
${process.env.INPUT_REF}`) and core.setOutput('sha', process.env.INPUT_REF) to
use the resolved SHA variable so downstream steps always receive an immutable
commit SHA.
- Line 45: Remove the invalid shell attribute from the uses step: locate the
step that contains "uses: actions/github-script@v7" (or any other "uses:" entry)
and delete the "shell: bash" line because shell is only valid on run steps;
ensure that any needed configuration for the action remains under "with:" and do
not move shell to the uses step.
---
Outside diff comments:
In @.github/workflows/console-api-release.yml:
- Around line 60-79: The job test-beta-sandbox references
needs.setup.outputs.image_tag but only declares needs: deploy-beta-sandbox,
causing the output to be inaccessible; update the test-beta-sandbox job to
include setup in its needs array (i.e., add setup alongside deploy-beta-sandbox)
so needs.setup.outputs.image_tag is in scope, keeping the rest of the job
(runs-on, steps, env, with) unchanged.
---
Duplicate comments:
In @.github/workflows/all-ci-unsafe.yml:
- Around line 113-117: The associated-pr step is being passed a raw SHA via the
ref input (id associated_pr, uses ./.github/actions/associated-pr), but the
action's ref-to-SHA resolution expects named tag/branch refs; update the
associated-pr action to accept raw SHAs (detect a 40‑char hex and skip
ref-to-SHA resolution) or normalize the workflow input by passing a branch/ref
instead (e.g., use github.event.workflow_run.head_branch or construct
refs/heads/<branch>) so that associate_pr's ref handling (the ref-to-SHA
resolution step) correctly handles both named refs and raw SHA inputs.
---
Nitpick comments:
In @.github/workflows/provider-proxy-release.yml:
- Around line 106-121: The workflow is missing a test-prod-mainnet job and
deploy-prod-mainnet doesn't depend on a pre-deploy test; add a new job named
test-prod-mainnet modeled after test-prod-sandbox/test-beta-mainnet (same
permissions, steps, and using the same test matrix for prod/mainnet), then
update the needs array of deploy-prod-mainnet (in the deploy-prod-mainnet job)
to include test-prod-mainnet so the prod mainnet deployment waits for its test
to complete.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/actions/associated-pr/action.yml.github/actions/console-web-ui-testing/action.yml.github/actions/run-e2e-tests/action.yml.github/actions/slack-pending-deployment-approval/action.yml.github/actions/slack-tests-failed-notification/action.yml.github/workflows/all-ci-unsafe.yml.github/workflows/auto-approval.yml.github/workflows/console-api-release.yml.github/workflows/console-web-release.yml.github/workflows/provider-proxy-release.yml
stalniy
force-pushed
the
chore/fix-associated-pr-refs
branch
from
39f9423 to
e18e979
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/associated-pr/action.yml (1)
95-106:⚠️ Potential issue | 🟠 MajorRetry loop takes
response.data[0]without verifyinghead.sha === commitSha.
listPullRequestsAssociatedWithCommitcan return multiple PRs (e.g., when a commit is cherry-picked or appears in several PR branches). Taking index[0]without a SHA guard means that — in a concurrent-merge scenario — the wrong PR could be selected on retry. The outer condition at line 81 already checkspr.head?.sha !== commitSha, making it inconsistent to skip this check on the API path.💡 Proposed fix
if (response.data && response.data.length > 0) { - pr = response.data[0]; - core.info(`Found PR #${pr.number} on attempt ${attempt + 1}`); - break; + const matched = response.data.find(p => p.head.sha === commitSha); + if (matched) { + pr = matched; + core.info(`Found PR #${pr.number} with matching head SHA on attempt ${attempt + 1}`); + break; + } + core.info(`Found ${response.data.length} PR(s) on attempt ${attempt + 1} but none matched head SHA ${commitSha}, retrying...`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/associated-pr/action.yml around lines 95 - 106, The retry logic currently assigns response.data[0] to pr without verifying the PR's head SHA matches commitSha; update the block that calls listPullRequestsAssociatedWithCommit so it searches response.data for an entry whose pr.head?.sha === commitSha (or equivalent) before setting pr, e.g. filter or find on response.data, assign that matching PR to pr, and only break when a match is found; if no match in this API response, log a message and continue retrying instead of taking response.data[0].
♻️ Duplicate comments (1)
.github/actions/associated-pr/action.yml (1)
68-69: Non-tag, non-SHA refs (e.g., branch names) are passed through as-is to downstream SHA consumers.The action description says "Git ref or commit SHA", but branch names or other arbitrary refs will not resolve correctly via
listPullRequestsAssociatedWithCommit. For existing callers this is not an issue (they pass commit SHAs or app/version tags), but the input contract is misleading. Either narrowing the description or adding a resolution fallback would prevent future misuse.
🧹 Nitpick comments (1)
.github/actions/associated-pr/action.yml (1)
56-66: Tag regex silently ignoresgetReffailures — consider wrapping in try/catch.If
git.getReffails (e.g., the tag doesn't exist yet due to a release race), the step throws and the entire action fails hard. Given that this action is on a notification/approval path, a graceful fallback tocontext.shaon404would be preferable.💡 Proposed fix
if (/^[\w-]+\/v{1,2}[\d.]+(-\w+\.\d+)?$/.test(process.env.INPUT_REF)) { core.info(`Ref input looks like app version tag, trying to resolve its sha: ${process.env.INPUT_REF}`); - const { data } = await github.rest.git.getRef({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: `tags/${process.env.INPUT_REF.replace(/\/v{2,}/, '/v')}`, - }); - core.setOutput('sha', data.object.sha); - return; + try { + const { data } = await github.rest.git.getRef({ + owner: context.repo.owner, + repo: context.repo.repo, + ref: `tags/${process.env.INPUT_REF.replace(/\/v{2,}/, '/v')}`, + }); + core.setOutput('sha', data.object.sha); + return; + } catch (err) { + core.warning(`Failed to resolve tag ${process.env.INPUT_REF}: ${err.message}. Falling back to context.sha.`); + core.setOutput('sha', context.sha); + return; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/associated-pr/action.yml around lines 56 - 66, The tag-resolution block using github.rest.git.getRef can throw and currently bubbles up; wrap the call to github.rest.git.getRef in a try/catch around the block that tests process.env.INPUT_REF, and on error check for a 404 (or any failure) and fallback to core.info logging and setting core.setOutput('sha', context.sha) instead of throwing; keep successful behavior unchanged (use the returned data.object.sha) and ensure you still normalize the tag string with the existing replace before calling getRef and log both attempts and fallbacks using core.info/core.warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/slack-pending-deployment-approval/action.yml:
- Line 61: The Slack message line that includes the run link should be guarded
so it only renders when inputs.linked-workflow-run-id is non-empty: change the
template segment that builds "Triggered by the following PR in run <${{
github.server_url }}/${{ github.repository }}/actions/runs/${{
inputs.linked-workflow-run-id }}|#${{ inputs.linked-workflow-run-id }}>:" to be
conditional on inputs.linked-workflow-run-id (or provide a safe default string)
so you avoid emitting an empty <...|...> token; use a GitHub Actions expression
(e.g. a ternary or && check on inputs.linked-workflow-run-id) to output the full
link only when inputs.linked-workflow-run-id != '' and otherwise omit or replace
with a neutral message.
In @.github/workflows/console-api-release.yml:
- Around line 76-77: In the test-beta-sandbox job change the app value from
"api" to "console-api" so the run-e2e-tests action and associated-pr receive the
same app identifier as other jobs; specifically update the app: api entry
referenced in the test-beta-sandbox job to app: console-api to ensure the
constructed ref `${{ inputs.app }}/v${{ inputs.app-version }}` matches the
console-api/v{version} git tag naming and allows associated-pr to resolve the
correct PR.
---
Outside diff comments:
In @.github/actions/associated-pr/action.yml:
- Around line 95-106: The retry logic currently assigns response.data[0] to pr
without verifying the PR's head SHA matches commitSha; update the block that
calls listPullRequestsAssociatedWithCommit so it searches response.data for an
entry whose pr.head?.sha === commitSha (or equivalent) before setting pr, e.g.
filter or find on response.data, assign that matching PR to pr, and only break
when a match is found; if no match in this API response, log a message and
continue retrying instead of taking response.data[0].
---
Nitpick comments:
In @.github/actions/associated-pr/action.yml:
- Around line 56-66: The tag-resolution block using github.rest.git.getRef can
throw and currently bubbles up; wrap the call to github.rest.git.getRef in a
try/catch around the block that tests process.env.INPUT_REF, and on error check
for a 404 (or any failure) and fallback to core.info logging and setting
core.setOutput('sha', context.sha) instead of throwing; keep successful behavior
unchanged (use the returned data.object.sha) and ensure you still normalize the
tag string with the existing replace before calling getRef and log both attempts
and fallbacks using core.info/core.warning.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/actions/associated-pr/action.yml.github/actions/console-web-ui-testing/action.yml.github/actions/run-e2e-tests/action.yml.github/actions/slack-pending-deployment-approval/action.yml.github/actions/slack-tests-failed-notification/action.yml.github/workflows/all-ci-unsafe.yml.github/workflows/auto-approval.yml.github/workflows/console-api-release.yml.github/workflows/console-web-release.yml.github/workflows/provider-proxy-release.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/actions/console-web-ui-testing/action.yml
- .github/actions/run-e2e-tests/action.yml
- .github/actions/slack-tests-failed-notification/action.yml
- .github/workflows/console-web-release.yml
- .github/workflows/all-ci-unsafe.yml
ygrishajev
approved these changes
Feb 26, 2026
stalniy
force-pushed
the
chore/fix-associated-pr-refs
branch
from
d1941f5 to
4a550b9
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/actions/associated-pr/action.yml (1)
68-69:⚠️ Potential issue | 🟠 MajorNon-tag refs still treated as literal SHA without resolution.
If
INPUT_REFdoesn't match the version-tag regex (e.g., a branch name likefeature/xyz), it's passed directly as the SHA without resolving to an actual commit. This can cause incorrect PR association for mutable refs.💡 Proposed fix – resolve arbitrary refs via repos.getCommit
- core.info(`Using provided ref as sha: ${process.env.INPUT_REF}`); - core.setOutput('sha', process.env.INPUT_REF); + const { data: commit } = await github.rest.repos.getCommit({ + owner: context.repo.owner, + repo: context.repo.repo, + ref: process.env.INPUT_REF, + }); + core.info(`Resolved ref ${process.env.INPUT_REF} to commit SHA: ${commit.sha}`); + core.setOutput('sha', commit.sha);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/associated-pr/action.yml around lines 68 - 69, The action treats any INPUT_REF as a literal SHA (core.setOutput('sha', process.env.INPUT_REF)) which breaks for branch or other non-tag refs; update the logic to detect when process.env.INPUT_REF does not match the tag/sha pattern and, in that case, call the GitHub REST API (repos.getCommit) to resolve the ref to the actual commit SHA, then set that resolved SHA via core.setOutput('sha', resolvedSha) so mutable refs (e.g., feature branches) are converted to concrete commit SHAs before using them elsewhere..github/actions/slack-pending-deployment-approval/action.yml (1)
61-61:⚠️ Potential issue | 🟡 MinorMalformed Slack link when
linked-workflow-run-idis empty.
linked-workflow-run-idis optional with no default. When omitted, the Slack mrkdwn renders as<.../actions/runs/|#>— a broken link.💡 Proposed fix – guard the run link with a conditional
- Triggered by the following PR in run <${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ inputs.linked-workflow-run-id }}|#${{ inputs.linked-workflow-run-id }}>: + Triggered by the following PR${{ inputs.linked-workflow-run-id != '' && format(' in run <{0}/{1}/actions/runs/{2}|#{2}>', github.server_url, github.repository, inputs.linked-workflow-run-id) || '' }}:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/slack-pending-deployment-approval/action.yml at line 61, The Slack message line rendering the run link uses the optional input inputs.linked-workflow-run-id and produces a malformed `<.../actions/runs/|#>` when empty; update the action.yml message template so the run link is only included when inputs.linked-workflow-run-id is non-empty (e.g., wrap the link generation in a conditional expression or choose an alternate text when empty), targeting the line containing "Triggered by the following PR in run <${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ inputs.linked-workflow-run-id }}|#${{ inputs.linked-workflow-run-id }}>" and ensuring Slack mrkdwn does not receive a broken link when inputs.linked-workflow-run-id is omitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/actions/associated-pr/action.yml:
- Around line 68-69: The action treats any INPUT_REF as a literal SHA
(core.setOutput('sha', process.env.INPUT_REF)) which breaks for branch or other
non-tag refs; update the logic to detect when process.env.INPUT_REF does not
match the tag/sha pattern and, in that case, call the GitHub REST API
(repos.getCommit) to resolve the ref to the actual commit SHA, then set that
resolved SHA via core.setOutput('sha', resolvedSha) so mutable refs (e.g.,
feature branches) are converted to concrete commit SHAs before using them
elsewhere.
In @.github/actions/slack-pending-deployment-approval/action.yml:
- Line 61: The Slack message line rendering the run link uses the optional input
inputs.linked-workflow-run-id and produces a malformed `<.../actions/runs/|#>`
when empty; update the action.yml message template so the run link is only
included when inputs.linked-workflow-run-id is non-empty (e.g., wrap the link
generation in a conditional expression or choose an alternate text when empty),
targeting the line containing "Triggered by the following PR in run <${{
github.server_url }}/${{ github.repository }}/actions/runs/${{
inputs.linked-workflow-run-id }}|#${{ inputs.linked-workflow-run-id }}>" and
ensuring Slack mrkdwn does not receive a broken link when
inputs.linked-workflow-run-id is omitted.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
.github/actions/associated-pr/action.yml.github/actions/console-web-ui-testing/action.yml.github/actions/run-e2e-tests/action.yml.github/actions/slack-pending-deployment-approval/action.yml.github/actions/slack-tests-failed-notification/action.yml.github/workflows/all-ci-unsafe.yml.github/workflows/auto-approval.yml.github/workflows/console-api-release.yml.github/workflows/console-web-release.yml.github/workflows/provider-proxy-release.yml
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/console-web-release.yml
- .github/actions/console-web-ui-testing/action.yml
- .github/actions/slack-tests-failed-notification/action.yml
- .github/actions/run-e2e-tests/action.yml
- .github/workflows/console-api-release.yml
- .github/workflows/auto-approval.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Right now, associated-pr always fetches PR from the main branch. This may cause incorrect identification when multiple PRs has been merged to main at about the same time
What
Summary by CodeRabbit
Chores
New Features