ci(workflows): remind authors not to force-push active PRs - #6035
Conversation
Add a workflow that detects force-pushes (rebase/amend/reset) to open PRs via the pull_request_target synchronize event and posts a one-time, bilingual reminder that force-pushing invalidates existing review comments and that the integration bots squash all changes into a single commit automatically. A normal push (compare status "ahead") is ignored; the reminder is posted at most once per PR, bot-initiated pushes are skipped, and a failed compare is treated conservatively (no comment).
|
Thanks for the PR — and thanks for pushing the Round 3 follow-up with the test file and addressing all the inline threads! Template looks good ✓ On direction: this solves a real maintainer pain point — force-pushing silently invalidates inline review comments, and since we squash on merge anyway, there's no reason contributors should need to rewrite history. Lightweight CI automation like this is squarely within scope. On approach: scope is tight — one workflow file, one test file, does exactly what it says. The Round 3 additions are well-targeted: 12 structural assertions lock in all the critical design decisions (3-dot compare, marker author check, KNOWN_AUTOMATION sync), and the 中文说明感谢贡献——也感谢后续补上的第三轮跟进,测试文件和对 inline 讨论的处理都已到位! 模板完整 ✓ 方向:解决了维护者的真实痛点——force-push 会使行内评审评论静默失效,而既然合并时本就会 squash,贡献者没有必要重写历史。这类轻量 CI 自动化完全在项目范围内。 方案:范围精准——一个 workflow 文件、一个测试文件,只做描述中的事。第三轮的补充针对性很强:12 条结构断言锁定了所有关键设计决策(3-dot 比较、标记作者校验、KNOWN_AUTOMATION 同步), — Qwen Code · qwen3.7-max |
Code ReviewRe-reviewed both files against the latest Round 3 commits (through Workflow (
Test ( 12 assertions covering trigger, repo guard, permissions, concurrency absence, SHA pin, zero-SHA guard, bot skip, 3-dot compare, 404 handling, marker author check, error logging, and bilingual body. Wired into CI via No critical blockers. No AGENTS.md violations. Follows project YAML conventions (single-quoted strings, 2-space indent, matches style of existing workflows). Static ValidationThis is a CI-only workflow that cannot execute until it lands on the base branch ( KNOWN_AUTOMATION sync verified against live Matches the PR's KNOWN_AUTOMATION set exactly. ✓ tmux Real-Scenario TestingN/A — This workflow produces no local CLI output. Its only artifact is a PR comment posted by the GitHub API after a force-push event. Behavior can only be validated post-merge on a throwaway PR (as described in the Reviewer Test Plan). 中文说明代码审查基于最新的第三轮提交(至 Workflow(162 行): 测试(130 行):12 条断言覆盖触发器、仓库守卫、权限、并发缺失、SHA 锁定、零 SHA 守卫、机器人跳过、3-dot 比较、404 处理、标记作者校验、错误日志、双语正文。通过 无关键阻塞项。无 AGENTS.md 违规。 静态验证纯 CI workflow,在进入基准分支前无法执行。tmux 真实场景测试不适用。yamllint、JS 语法校验均通过,12 条结构测试全部通过。KNOWN_AUTOMATION 同步已对照线上 — Qwen Code · qwen3.7-max |
|
This PR has come a long way through the review cycles. The Round 3 follow-up (7 commits, through Stepping back: the motivation is genuine, the implementation is correct, and the test coverage is thorough for a workflow file (12 structural assertions is more than most CI changes get). The no-concurrency-group decision is well-reasoned and documented. The security model is sound — Approving. ✅ 中文说明这个 PR 经过多轮评审已经非常完善。第三轮跟进(7 个提交,至 回顾:动机真实、实现正确、测试覆盖对于一个 workflow 文件来说很充分(12 条结构断言比大多数 CI 改动都要多)。无并发组的决策经过充分论证并有文档说明。安全模型健全—— 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…ancel - Add `issues: write`: the listComments/createComment calls go through the Issues API; declaring it matches the repo's other PR-commenting workflows and avoids any risk of a 403 making the workflow inert. - Set `cancel-in-progress: false`: an in-flight run that already detected a force-push must finish and post. The concurrency group still serializes runs per PR, and the once-per-PR marker prevents duplicates, so later pushes queue and then no-op instead of cancelling (and silently dropping) a pending reminder.
- Marker dedup now requires the comment to be from github-actions[bot], so a user pasting the marker string into a comment can't suppress reminders. - Skip known automation logins (qwen-code-dev-bot et al.) that push via PAT as sender.type 'User', not just GitHub App bots (mirrors qwen-autofix KNOWN_BOTS). - Narrow the compare catch to 404 (orphaned old tip -> skip); rethrow other errors so auth/rate failures go red instead of silently no-op'ing. - Wrap createComment with structured error logging + rethrow. Kept 3-dot compare and base-repo owner: verified that 3-dot returns diverged/behind for force-pushes and that the base repo resolves fork-PR commits, while the suggested 2-dot syntax 404s in the REST API.
wenshao
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] No test file found at scripts/tests/pr-force-push-reminder-workflow.test.js. The repo has established precedent (qwen-autofix-workflow.test.js, qwen-triage-workflow.test.js, qwen-resolve-workflow.test.js) — all use structural YAML assertions against workflow files. Consider adding a test that asserts the concurrency group, repo guard, KNOWN_AUTOMATION sync with qwen-autofix.yml, zero-SHA guard, 404-vs-rethrow branching, marker idempotency check, and timeout-minutes.
— qwen3.7-max via Qwen Code /review
- Add scripts/tests/pr-force-push-reminder-workflow.test.js (runs under test:scripts, which CI chains into test:ci). It asserts the trigger, repo guard, permissions, serialized concurrency, KNOWN_AUTOMATION sync with qwen-autofix, the 3-dot compare on the base repo, 404-vs-rethrow, the marker author check, and the bilingual body — locking in the reviewed behaviors. - Wrap the listComments paginate call in the same core.error + rethrow the other two API calls already use. - Note that KNOWN_AUTOMATION must stay in sync with qwen-autofix.yml KNOWN_BOTS.
|
Round 3 follow-up — pushed
The re-raised 3-dot 中文第三轮跟进 —— 已推
被重新提出的 3-dot |
wenshao
left a comment
There was a problem hiding this comment.
— gpt-5 via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
— gpt-5 via Qwen Code /review
A concurrency group keeps at most one pending run per group, so a burst of pushes can cancel a still-pending force-push run before it reaches the script, dropping the reminder this workflow exists to post. Remove the group entirely: every synchronize event now runs independently and is always evaluated, and the once-per-PR marker provides idempotency. A rare double-post on two near-simultaneous first force-pushes is the acceptable cost of never silently missing one. Update the structural test to assert there is no concurrency block. The reviewer's suggested `queue: max` is not a valid GitHub Actions concurrency key (only `group`/`cancel-in-progress` are allowed) and fails actionlint.
DragonnZhang
left a comment
There was a problem hiding this comment.
Review pass (round 4): All previously raised findings (cancel-in-progress race, marker author verification, bot sender coverage, error swallowing, paginate/createComment error handling, missing issues: write, fork compare, 3-dot syntax) have been addressed in commits through 48b6c46. The workflow and test file are structurally sound. No new high-confidence findings. LGTM.
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
- Switch the copyright header to the prevailing `Qwen Team` (14 of 17 sibling test files use it; this file had copied an older Google LLC header). - Assert the idempotency skip log line so removing the marker guard fails a test.
3ba3169
Read qwen-autofix.yml's KNOWN_BOTS and assert each login is also skipped here, so adding a bot there without updating this workflow fails the test instead of silently drifting. Replaces the hardcoded login list whose comment overclaimed that the sync was verified.
|
@qwen-code /triage |
yiliang114
left a comment
There was a problem hiding this comment.
LGTM from code review. I re-checked the latest head (be9e2db) and found no remaining blockers.
Targeted validation passed: actionlint, yamllint, embedded github-script syntax parsing, and the new structural workflow test (12/12). CI still has pending jobs, but the workflow/test changes look ready from code review.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
|
||
| it('only trusts the dedup marker on its own bot comment', () => { | ||
| // Otherwise any user could suppress all future reminders by pasting the | ||
| // marker string into a comment. |
There was a problem hiding this comment.
[Suggestion] The dedup-marker test asserts the marker string, bot identity checks, and the skip message, but never asserts that comment listing uses github.paginate() rather than a single-page listComments call. The workflow deliberately chose paginate to handle PRs with >100 comments — if someone "simplifies" the call to a single-page fetch, the dedup check would silently miss the marker on busy PRs and post duplicate reminders.
expect(workflow).toContain('github.paginate(');— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Fixed in e9fcc31 — added expect(workflow).toContain('github.paginate(') to the dedup test, so simplifying the listing to a single-page fetch (which would miss a marker buried past comment 100 and post a duplicate) now fails the test.
中文
已在 e9fcc31 修复——给去重测试加了 expect(workflow).toContain('github.paginate('),这样把列举简化成单页拉取(会漏掉第 100 条之后被淹没的标记并重复发帖)现在会让测试失败。
What this PR does
Adds a lightweight GitHub Actions workflow that watches open pull requests and, when a contributor rebases or force-pushes the PR branch, posts a one-time reminder comment asking them not to force-push (because it invalidates existing inline review comments) and noting that the integration bots squash all changes into a single commit automatically on merge. The reminder is bilingual — English with a collapsed Chinese translation.
It distinguishes a force-push from a normal push by comparing the
beforeandafterSHAs carried on thepull_request_targetsynchronizeevent: abehindordivergedcompare status means history was rewritten (force-push), whileahead/identicalis a normal push and is ignored. The comment is posted at most once per PR (guarded by a hidden HTML marker and full comment-list pagination), pushes initiated by bots are skipped, and a failed comparison is treated conservatively (no comment) so the bot never makes a false accusation.Why it's needed
Contributors frequently rebase or force-push their PR branches, which silently invalidates existing inline review comments and makes re-review harder. Since the integration bots squash everything into a single commit on merge anyway, force-pushing to "clean up" history is unnecessary. An automatic, friendly, once-per-PR reminder reduces this churn without a maintainer having to leave the same comment by hand each time.
Reviewer Test Plan
How to verify
This is a CI-only change (a single new workflow file). It cannot execute until it lands on the base branch, because
pull_request_targetalways runs the workflow definition from the target branch — so a reviewer confirms it via static validation plus a post-merge smoke test. Static validation performed locally (macOS):actionlint .github/workflows/pr-force-push-reminder.yml→ passes.yamllint --format githubwith the repo's.yamllintconfig → passes (same result as the existing workflows).github-scriptbody, wrapped in an async function exactly asgithub-scriptruns it, passesnode --check.After merge, behavior can be confirmed on a throwaway PR: pushing additional commits produces no comment (compare status
ahead);git commit --amend && git push --forceor a rebase produces exactly one reminder comment; a second force-push produces no duplicate (the marker is detected).Evidence (Before & After)
N/A — non–user-visible CI workflow; its only output is a PR comment posted after the workflow is on the base branch.
Tested on
node --check)ubuntu-latest)Environment (optional)
N/A — static lint validation only; the workflow itself has no local runtime.
Risk & Scope
pull_request_targetbut never checks out or executes PR code (it only calls the GitHub REST API), so there is no pwn-request exposure. Permissions are minimal:contents: read,pull-requests: write.Linked Issues
N/A
中文说明
这个 PR 做了什么
新增一个轻量的 GitHub Actions workflow,监听处于打开状态的 PR;当贡献者对 PR 分支执行 rebase 或 force-push 时,自动发一条一次性提醒评论,请其不要 force-push(因为会使已有的行内评审评论失效),并说明集成机器人在合并时本就会自动把所有改动压缩(squash)成单个提交。提醒为双语——英文正文 + 折叠的中文翻译。
它通过比较
pull_request_target的synchronize事件所携带的before与after两个 SHA 来区分 force-push 与普通推送:compare 状态为behind或diverged表示历史被改写(force-push),而ahead/identical是普通推送、直接忽略。评论每个 PR 最多发一次(用隐藏 HTML 标记 + 全量评论分页去重)、跳过由机器人发起的推送、比较失败时保守处理(不发评论),以确保不会误伤。为什么需要
贡献者经常 rebase 或 force-push 自己的 PR 分支,这会悄无声息地使已有的行内评审评论失效、增加重新评审的成本。既然集成机器人在合并时本就会把所有改动 squash 成单个提交,为"整理"历史而 force-push 是没必要的。一条自动、友好、每个 PR 仅一次的提醒可以减少这类来回,而不需要维护者每次手动留同样的评论。
评审验证计划
如何验证
这是一个纯 CI 改动(仅新增一个 workflow 文件)。在它进入基准分支之前无法运行,因为
pull_request_target始终使用目标分支上的 workflow 定义——因此评审者通过静态校验 + 合并后冒烟测试来确认。本地(macOS)已做的静态校验:actionlint .github/workflows/pr-force-push-reminder.yml→ 通过。.yamllint配置跑yamllint --format github→ 通过(与现有 workflow 结果一致)。github-script脚本体按github-script实际运行方式用 async 函数包裹后,node --check→ 通过。合并后可在一个临时 PR 上确认:追加普通提交不会产生评论(compare 状态
ahead);执行git commit --amend && git push --force或 rebase 会产生恰好一条提醒评论;再次 force-push 不会重复发(标记被检测到)。证据(前/后对比)
N/A — 非用户可见的 CI workflow;其唯一输出是在 workflow 进入基准分支后于 PR 上发的评论。
测试平台
见上方表格:macOS 已做静态 lint 校验;workflow 本身只在 GitHub 托管的
ubuntu-latest上运行。运行环境(可选)
N/A — 仅静态 lint 校验,workflow 本身没有本地运行时。
风险与范围
pull_request_target,但从不 checkout 或执行 PR 代码(只调用 GitHub REST API),因此不存在 pwn-request 风险。权限最小化:contents: read、pull-requests: write。关联 Issue
N/A