Copilot commented on code in PR #8678:
URL: https://github.com/apache/texera/pull/8678#discussion_r4189151380
##########
.github/workflows/pr-assignment.yml:
##########
@@ -24,12 +24,20 @@ permissions:
pull-requests: write
jobs:
- # All three behaviors live as steps under one job so the PR Checks
- # tab shows a single entry per event instead of two-or-three skipped
- # siblings. Step-level if-guards keep the actual work scoped.
+ # All behaviors live as steps under one job so the PR Checks tab shows
+ # a single entry per event instead of several skipped siblings.
+ # Step-level if-guards keep the actual work scoped.
pr-assignment:
runs-on: ubuntu-latest
steps:
+ # pull_request_target resolves to the trusted base branch, never the
+ # fork head, so requiring this script from the steps below is safe.
+ - uses: actions/checkout@v7
Review Comment:
This new checkout uses the workflow token, but the explicit permissions
block grants no `contents` access. `actions/checkout` will fail before either
`github-script` step can require this module; add `contents: read` to the
workflow permissions.
##########
.github/workflows/pr-assignment.yml:
##########
@@ -139,6 +147,97 @@ jobs:
core.info(`Body unchanged on edit; skipping removal-detection.`);
}
+ - name: Flag linked issues someone else has claimed
+ # A PR that closes an issue someone else has claimed (they are
+ # assigned, or have an open PR on it) gets one sticky comment
+ # naming them and their PR, so the overlap is settled in review.
+ # Otherwise the first merge closes the issue and can strand the
+ # other PR: #8339 merged over #8267 on #8149 (see #8676).
+ #
+ # It comments instead of failing a check. Both PRs match (the
+ # claimant's own PR flags this one back), so a red check would
+ # also land on the person being protected. Only a human can tell
+ # an agreed takeover from a duplicate. And nothing re-runs this
+ # when the other PR closes, so a failure would go stale.
+ if: >-
+ contains(fromJSON('["opened","edited"]'), github.event.action)
+ && github.event.pull_request.state == 'open'
+ && github.event.pull_request.user.type != 'Bot'
+ uses: actions/github-script@v9
+ with:
+ script: |
+ const { CLAIM_MARKER, findClaimConflicts, renderClaimComment } =
+ require('./.github/scripts/issue-claims.js');
+ const { owner, repo } = context.repo;
+ const pr = context.payload.pull_request;
+ const issue_number = pr.number;
+
+ const { repository: { pullRequest: prq } } = await github.graphql(`
+ query($owner: String!, $repo: String!, $pr: Int!) {
+ repository(owner: $owner, name: $repo) {
+ pullRequest(number: $pr) {
+ closingIssuesReferences(first: 50) {
+ nodes {
+ number
+ state
+ repository { nameWithOwner }
+ assignees(first: 20) { nodes { login } }
+ closedByPullRequestsReferences(first: 20) {
+ nodes { number state author { login } repository {
nameWithOwner } }
Review Comment:
This only reads the first 20 linked PRs. If an issue has more than 20
`closedByPullRequestsReferences`, an open claimant after that page is silently
omitted, so this PR can merge without warning and the claimant can still be
removed by the credit step. Paginate this connection (using `pageInfo`/cursors)
rather than relying on a fixed first page.
This issue also appears on line 314 of the same file.
##########
.github/workflows/pr-assignment.yml:
##########
@@ -139,6 +147,97 @@ jobs:
core.info(`Body unchanged on edit; skipping removal-detection.`);
}
+ - name: Flag linked issues someone else has claimed
+ # A PR that closes an issue someone else has claimed (they are
+ # assigned, or have an open PR on it) gets one sticky comment
+ # naming them and their PR, so the overlap is settled in review.
+ # Otherwise the first merge closes the issue and can strand the
+ # other PR: #8339 merged over #8267 on #8149 (see #8676).
+ #
+ # It comments instead of failing a check. Both PRs match (the
+ # claimant's own PR flags this one back), so a red check would
+ # also land on the person being protected. Only a human can tell
+ # an agreed takeover from a duplicate. And nothing re-runs this
+ # when the other PR closes, so a failure would go stale.
+ if: >-
+ contains(fromJSON('["opened","edited"]'), github.event.action)
+ && github.event.pull_request.state == 'open'
+ && github.event.pull_request.user.type != 'Bot'
+ uses: actions/github-script@v9
+ with:
+ script: |
+ const { CLAIM_MARKER, findClaimConflicts, renderClaimComment } =
+ require('./.github/scripts/issue-claims.js');
+ const { owner, repo } = context.repo;
+ const pr = context.payload.pull_request;
+ const issue_number = pr.number;
+
+ const { repository: { pullRequest: prq } } = await github.graphql(`
+ query($owner: String!, $repo: String!, $pr: Int!) {
+ repository(owner: $owner, name: $repo) {
+ pullRequest(number: $pr) {
+ closingIssuesReferences(first: 50) {
+ nodes {
+ number
+ state
+ repository { nameWithOwner }
+ assignees(first: 20) { nodes { login } }
+ closedByPullRequestsReferences(first: 20) {
+ nodes { number state author { login } repository {
nameWithOwner } }
+ }
+ }
+ }
+ }
+ }
+ }`, { owner, repo, pr: issue_number });
+
+ const conflicts =
findClaimConflicts(prq.closingIssuesReferences.nodes, {
+ repo: `${owner}/${repo}`, prNumber: issue_number, opener:
pr.user.login,
+ });
+ for (const { issue, claimants } of conflicts) {
+ core.info(`Issue #${issue} claimed by: ${claimants.map((c) =>
`${c.login} (assigned=${c.assigned}, prs=[${c.prs.join(',')}])`).join('; ')}`);
+ }
+ if (!conflicts.length && context.payload.action === 'opened') {
+ core.info(`No linked issue is claimed by someone else.`);
+ return;
+ }
+
+ let existing = null;
+ try {
+ const comments = await github.paginate(
+ github.rest.issues.listComments,
+ { owner, repo, issue_number, per_page: 100 },
+ );
+ existing = comments.find((c) => c.user?.type === 'Bot' &&
(c.body || '').includes(CLAIM_MARKER));
+ } catch (e) {
+ // Without the comment list we can't safely de-dupe; bail to
+ // avoid posting a second copy.
+ core.warning(`listComments on #${issue_number} failed:
${e.message}`);
+ return;
+ }
+
+ try {
+ if (conflicts.length) {
+ const body = renderClaimComment(conflicts);
+ if (!existing) {
+ await github.rest.issues.createComment({ owner, repo,
issue_number, body });
Review Comment:
The read-then-create sequence is not protected against concurrent
`opened`/`edited` runs. Two runs can both observe no marker at line 211 and
then both call `createComment`, leaving duplicate warning comments even though
this step promises one sticky comment. Add a PR-scoped workflow/job concurrency
group or otherwise make the upsert idempotent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
