Skip to content

feat: add github action to self-assign the issue - #3774

Open
Phoenix500526 wants to merge 1 commit into
apache:mainfrom
Phoenix500526:feat/self-assign-issue
Open

feat: add github action to self-assign the issue#3774
Phoenix500526 wants to merge 1 commit into
apache:mainfrom
Phoenix500526:feat/self-assign-issue

Conversation

@Phoenix500526

Copy link
Copy Markdown

Summary

Add a github action to enable self-assignment.

Fixes #3773

Verification

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found blocking issues.

[P2] Actor-wide concurrency will drop pending commands

take.yml:30-31 groups only by github.actor without queue, so a second take from the same user on another issue cancels the pending job for the first issue. Commands are silently dropped. Must isolate by issue and use queue: max.

[P2] Mutation HTTP failures still show green

take.yml:47,54 POST/DELETE curl lacks --fail and status checks; any 403/404/422/5xx still exits 0 and the workflow remains green while assignee state is unchanged. Add failure handling and verification.

Standards hard: PR template says behavior=No but the workflow does change behavior (issue assignees), and verification is empty yet marked as passed — needs correction.

Checks on 6a0cf5557 are no checks — gate not met.

简体中文存在并发与 HTTP 失败两项阻断及模板问题。

CLOSES apache#3773

Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
@Phoenix500526
Phoenix500526 force-pushed the feat/self-assign-issue branch from 6a0cf55 to 12f52a3 Compare August 25, 2026 09:07
@Phoenix500526

Copy link
Copy Markdown
Author

I reviewed this head and found blocking issues.

[P2] Actor-wide concurrency will drop pending commands

take.yml:30-31 groups only by github.actor without queue, so a second take from the same user on another issue cancels the pending job for the first issue. Commands are silently dropped. Must isolate by issue and use queue: max.

[P2] Mutation HTTP failures still show green

take.yml:47,54 POST/DELETE curl lacks --fail and status checks; any 403/404/422/5xx still exits 0 and the workflow remains green while assignee state is unchanged. Add failure handling and verification.

Standards hard: PR template says behavior=No but the workflow does change behavior (issue assignees), and verification is empty yet marked as passed — needs correction.

Checks on 6a0cf5557 are no checks — gate not met.

简体中文
存在并发与 HTTP 失败两项阻断及模板问题。

Done

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found no blocking issues.

The workflow now correctly isolates concurrency per issue with queue: max and handles HTTP failures with --fail and assignee verification. No injection, minimal permissions, and checklist now honest.

Checks on 12f52a3113 have no required test (workflow-only change) — gate is not applicable.

简体中文该头未发现阻断。

Astro-Han
Astro-Han previously approved these changes Aug 25, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

APPROVE — 12f52a3, no P0-P2, concurrency and HTTP handling fixed.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The approach is right and it fits ASF norms: take/untake self-assignment is standard practice across ASF projects, and this uses no third-party actions — only gh — which sidesteps the INFRA restrictions on Action provenance entirely. permissions: issues: write is minimal and secrets.GITHUB_TOKEN is the correct token.

It also gets the part that usually goes wrong right: issue_comment plus a write token is the classic script-injection shape, and this passes comment.body and user.login through env instead of interpolating ${{ }} into run:. ubuntu-slim and concurrency.queue: max both checked out too — I was initially suspicious of each and both are correct, ubuntu-slim being GA since January and scoped exactly at issue automation.

Dismissing my earlier approval: test is red, and two behavioural gaps below are worth settling before this lands rather than after contributors start relying on it. Neither is hard to fix.

One non-code note: the PR checklist marks "Tests cover the change and fail without it", but there are none. That is a defensible choice for a 69-line workflow — scripts/asf-source-workflow-policy.test.mjs only covers two specific files and does not require registering new ones — so just uncheck the box rather than adding a test.

简体中文

解法方向对,也符合 ASF 规范:take/untake 认领是 ASF 项目通行做法;不引入任何第三方 action、只用 gh,正好绕开 INFRA 对 Action 来源的限制;最小授权和 GITHUB_TOKEN 都对。最容易出问题的地方它做对了——issue_comment 加写权限是典型的脚本注入形状,而这里把 comment.bodyuser.login 走 env 传递,没有内联 ${{ }}run:ubuntu-slimqueue: max 我一开始都怀疑过,查证后两个都是对的。

撤回我之前的 approve:CI 是红的,另外下面两条行为缺口值得在合并前定掉,而不是等贡献者开始依赖之后。都不难改。

另外 PR 勾了“Tests cover the change and fail without it”,但没有测试。对一个 69 行的 workflow 这个选择说得通(仓库那个 workflow 策略测试只覆盖两个特定文件,不要求登记新的),取消勾选即可。

Reviewed with help from Claude.

# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] This is what test is failing on. check:asf-headers reports "1 file(s) carry the ASF license text in a form this policy does not render" — the only difference from every other workflow in the repo is the indentation here: the canonical form is five spaces, # http://www.apache.org/licenses/LICENSE-2.0. Compare ci.yml:9. Fixing that one line turns the check green.

ASSIGNEES_ENDPOINT="repos/$REPO/issues/$ISSUE_NUMBER/assignees"

if [[ "$COMMENT_BODY" == "take" ]]; then
gh api --silent \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] The assignability probe uses --silent under set -euo pipefail, so a user who cannot be assigned just fails the job. Because this is issue_comment-triggered, that failure only ever surfaces in the Actions tab, which contributors have no reason to visit — from their side they commented take and nothing happened, with no way to find out why.

The same applies to the two jq -e guards below: they are good verifications, but their failure is equally invisible. Worth posting a short comment on the issue for each failure path.

"$ASSIGNEES_ENDPOINT/$USER_LOGIN"

echo "Assigning issue $ISSUE_NUMBER to $USER_LOGIN"
gh api \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] POST /assignees appends rather than replaces — GitHub's own wording is "Users already assigned to an issue are not replaced." So if Alice has taken an issue, Bob commenting take leaves both assigned, which defeats the coordination this feature exists to provide (#3773 frames it as replacing "ask a committer to assign it to me").

Suggest reading the current assignees first and refusing when the list is non-empty and does not already contain the commenter, with a comment saying who holds it.

@Astro-Han
Astro-Han dismissed their stale review August 25, 2026 20:13

Dismissing my approval — the test check is red (ASF header rendering, one indentation character) and two behavioural gaps are worth settling before merge: take does not guard against an already-claimed issue, and every failure path is invisible to the contributor. Details in the review above.

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.

feat: add a github action to self-assign the issue

3 participants