-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(triage): finalize the status comment on cancellation too #8436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7ee7cea
8569cd7
4014085
d453112
b257a03
a2aa71b
90fc940
0958ea8
c67187c
1ec4150
fa10cfe
5c55b2f
8e130d0
1beca48
efda1e2
40a1ff0
3ce449b
f2fa5f5
ad440db
e475fa4
2d14f51
4708d1a
060c2c2
02aad77
b8ac04e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -540,10 +540,13 @@ jobs: | |
| # The agent runs as one long step, but its output streams live to the | ||
| # Actions log. Post a status comment up front carrying that run link so a | ||
| # maintainer can watch progress instead of waiting silently until the | ||
| # first stage comment lands; "Finalize triage status comment" flips it to | ||
| # a terminal state at the end. Upsert by marker so a re-run reuses the one | ||
| # comment (no clutter). Best-effort: never fail the job over a status post. | ||
| # first stage comment lands. Upsert by marker so a re-run reuses the one | ||
| # comment (no clutter), and export the comment id: "Finalize triage | ||
| # status comment" PATCHes exactly that id to flip it to a terminal state, | ||
| # so the two steps cannot disagree about which comment this run owns. | ||
| # Best-effort: never fail the job over a status post. | ||
| - name: 'Post triage status comment' | ||
| id: 'status' | ||
| if: "steps.resolve.outputs.number != ''" | ||
| shell: 'bash' | ||
| env: | ||
|
|
@@ -567,16 +570,24 @@ jobs: | |
| jq -rs --arg m "$MARKER" --arg legacy "$LEGACY_MARKER" --arg bot "$BOT_LOGIN" \ | ||
| '[.[][] | select(.user.login == $bot) | select((.body | startswith($m)) or (.body | startswith($legacy)))] | last | .id // empty' | ||
| )" || EXISTING_ID='' | ||
| COMMENT_ID='' | ||
| if [ -n "$EXISTING_ID" ]; then | ||
| gh api --method PATCH \ | ||
| if gh api --method PATCH \ | ||
| "repos/$GITHUB_REPOSITORY/issues/comments/$EXISTING_ID" \ | ||
| -f body="$BODY" >/dev/null || | ||
| -f body="$BODY" >/dev/null; then | ||
| COMMENT_ID="$EXISTING_ID" | ||
| else | ||
| echo "::warning::Failed to update triage status comment; continuing." >&2 | ||
| fi | ||
| else | ||
| gh api "repos/$GITHUB_REPOSITORY/issues/$NUMBER/comments" \ | ||
| -f body="$BODY" >/dev/null || | ||
| COMMENT_ID="$( | ||
| gh api "repos/$GITHUB_REPOSITORY/issues/$NUMBER/comments" \ | ||
| -f body="$BODY" --jq '.id' | ||
| )" || COMMENT_ID='' | ||
| [ -n "$COMMENT_ID" ] || | ||
| echo "::warning::Failed to post triage status comment; continuing." >&2 | ||
| fi | ||
| echo "comment_id=$COMMENT_ID" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - name: 'Inject model name into triage signature' | ||
|
|
@@ -950,47 +961,54 @@ jobs: | |
| # Flip the early status comment to a terminal state so a lingering | ||
| # "running" line doesn't outlive the run. Neutral wording — the verdict | ||
| # lives in the stage comments above; this only records that the run ended | ||
| # and keeps the run link. Same marker → edits the one comment, no second | ||
| # post. Best-effort, on both success and failure. | ||
| # and keeps the run link. PATCHes exactly the comment id the claim step | ||
| # exported. An empty id means this run never ended up owning a comment: | ||
| # a cancel landing before the claim posted, or a transient failure of the | ||
| # claim's write (both the fresh-POST and PATCH arms export an empty id on | ||
| # failure). Either way nothing is flipped and a previous run's terminal | ||
| # wording stays untouched. A failed claim therefore leaves no lifecycle | ||
| # breadcrumb even when the comments API recovers during the minutes-long | ||
| # agent run — accepted: the verdict still lands in the stage comments, | ||
| # and a list-then-POST recovery here would reintroduce exactly the | ||
| # selection this coupling removes. Best-effort, on success, failure, AND | ||
| # cancellation: always() (not success() || failure()) because | ||
| # cancellation — cancel-in-progress superseding this run on a newer push | ||
| # or comment, job timeout, or manual cancel — would skip this step and | ||
| # leave the comment claiming the run is still in progress — the verify | ||
| # lane has publish-verify to catch that case, this lane only has us. | ||
| - name: 'Finalize triage status comment' | ||
| if: "(success() || failure()) && steps.resolve.outputs.number != ''" | ||
| if: "always() && steps.resolve.outputs.number != ''" | ||
| shell: 'bash' | ||
| env: | ||
| GH_TOKEN: '${{ secrets.QWEN_CODE_BOT_TOKEN || secrets.CI_BOT_PAT }}' | ||
| NUMBER: '${{ steps.resolve.outputs.number }}' | ||
| STATUS_COMMENT_ID: '${{ steps.status.outputs.comment_id }}' | ||
| RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' | ||
| TRIAGE_OUTCOME: '${{ steps.triage.outcome }}' | ||
| JOB_STATUS: '${{ job.status }}' | ||
| run: |- | ||
| set -uo pipefail | ||
| MARKER='<!-- qwen-triage lifecycle -->' | ||
| LEGACY_MARKER='<!-- qwen-triage stage=status -->' | ||
| if ! BOT_LOGIN="$(gh api user --jq '.login')" || [ -z "$BOT_LOGIN" ]; then | ||
| echo "::warning::Cannot resolve bot identity; skipping final status comment upsert." | ||
| if [ -z "${STATUS_COMMENT_ID:-}" ]; then | ||
| echo "This run claimed no status comment; nothing to finalize." >&2 | ||
| else | ||
| if [ "${TRIAGE_OUTCOME:-}" = 'success' ]; then | ||
| # A step success is not a green job: 'Check triage response' exits | ||
| # 1 on an empty summary while steps.triage.outcome stays 'success', | ||
| # and a later step can still fail. Only a non-failed job may claim | ||
| # "finished". | ||
| if [ "${TRIAGE_OUTCOME:-}" = 'success' ] && [ "${JOB_STATUS:-}" != 'failure' ]; then | ||
| EN="✅ **Qwen Triage finished** — [view run]($RUN_URL). See the stage comments in this thread for the result." | ||
| ZH="✅ **Qwen Triage 已完成** —— [查看运行]($RUN_URL)。结果见本线程中的各阶段评论。" | ||
| elif [ "${JOB_STATUS:-}" = 'cancelled' ]; then | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The three-way message selection is pinned by the shape test only as unordered 中文说明三分支消息选择在形状测试中只以无序 — qwen3.8-max via Qwen Code /review (v0.21.4) |
||
| EN="🚫 **Qwen Triage was cancelled** — [view run]($RUN_URL). The run was cancelled before finishing. Check for a newer run before re-running." | ||
| ZH="🚫 **Qwen Triage 已取消** —— [查看运行]($RUN_URL)。运行未完成即被取消。重跑前请先确认是否有更新的运行。" | ||
| else | ||
| EN="⚠️ **Qwen Triage ended early** — [view run]($RUN_URL). It stopped before finishing; check the run log." | ||
| ZH="⚠️ **Qwen Triage 提前结束** —— [查看运行]($RUN_URL)。未跑完,请查看运行日志。" | ||
| fi | ||
| printf -v BODY '%s\n\n%s\n\n%s' "$MARKER" "$EN" "$ZH" | ||
| EXISTING_ID="$( | ||
| gh api "repos/$GITHUB_REPOSITORY/issues/$NUMBER/comments" \ | ||
| --method GET --paginate -F per_page=100 | | ||
| jq -rs --arg m "$MARKER" --arg legacy "$LEGACY_MARKER" --arg bot "$BOT_LOGIN" \ | ||
| '[.[][] | select(.user.login == $bot) | select((.body | startswith($m)) or (.body | startswith($legacy)))] | last | .id // empty' | ||
| )" || EXISTING_ID='' | ||
| if [ -n "$EXISTING_ID" ]; then | ||
| gh api --method PATCH \ | ||
| "repos/$GITHUB_REPOSITORY/issues/comments/$EXISTING_ID" \ | ||
| -f body="$BODY" >/dev/null || | ||
| echo "::warning::Failed to finalize triage status comment; continuing." >&2 | ||
| else | ||
| gh api "repos/$GITHUB_REPOSITORY/issues/$NUMBER/comments" \ | ||
| -f body="$BODY" >/dev/null || | ||
| echo "::warning::Failed to post final triage status comment; continuing." >&2 | ||
| ZH="⚠️ **Qwen Triage 提前结束** —— [查看运行]($RUN_URL)。未跑完,请查看运行日志。" | ||
| fi | ||
| printf -v BODY '%s\n\n%s\n\n%s' '<!-- qwen-triage lifecycle -->' "$EN" "$ZH" | ||
| gh api --method PATCH \ | ||
| "repos/$GITHUB_REPOSITORY/issues/comments/$STATUS_COMMENT_ID" \ | ||
| -f body="$BODY" >/dev/null || | ||
| echo "::warning::Failed to finalize triage status comment; continuing." >&2 | ||
| fi | ||
|
|
||
| # On-demand real-user testing: a write-permission user comments | ||
|
|
@@ -1181,7 +1199,7 @@ jobs: | |
| uses: 'actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830' # v4.3.0 | ||
| with: | ||
| path: '${{ runner.temp }}/npm-cache' | ||
| key: 'npm-ci-${{ hashFiles(''package-lock.json'') }}' | ||
| key: "npm-ci-${{ hashFiles('package-lock.json') }}" | ||
| restore-keys: 'npm-ci-' | ||
|
|
||
| - name: 'Report npm cache hit' | ||
|
|
@@ -2647,7 +2665,7 @@ jobs: | |
| uses: 'actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830' # v4.3.0 | ||
| with: | ||
| path: '${{ runner.temp }}/npm-cache' | ||
| key: 'npm-ci-${{ hashFiles(''package-lock.json'') }}' | ||
| key: "npm-ci-${{ hashFiles('package-lock.json') }}" | ||
| restore-keys: 'npm-ci-' | ||
|
|
||
| - name: 'Report npm cache hit' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R9-3: This empty-id no-op drops the pre-PR finalize's recovery path: the old finalize re-ran the marker lookup at the end and — only when no bot-owned marker comment existed — POSTed a fresh terminal comment (that arm clobbered nothing). Now a run whose claim write failed transiently (list-fallback POST or PATCH failure — both tested arms export an empty id) finalizes nothing, even though the API typically recovers during the minutes-long agent run: the thread gets no lifecycle breadcrumb with the run link at all. — Failure scenario: comments-API blip during
Post triage status comment(both write arms fail →comment_id=empty); API recovers; the run completes; no lifecycle comment ever lands — pre-PR finalize would have POSTed one.Optional fix: in the empty-id branch, list bot-owned marker comments and POST the terminal wording only when none exists (keeps the no-clobber property without reintroducing a selection race); or document the accepted loss in the step comment, which currently names only cancellation as the empty-id cause.
中文说明
[Suggestion] 这个空 id 的 no-op 分支丢掉了本 PR 之前 finalize 的恢复路径:旧 finalize 在收尾时会重新执行 marker 查找,且仅在不存在任何 bot 拥有的 marker 评论时才新发一条终态评论(该分支不会覆盖任何内容)。现在,claim 写入瞬时失败(list 兜底 POST 或 PATCH 失败——这两种失败路径都已被测试覆盖,均导出空 id)的 run 将完全不发终态评论,即使 comments API 通常会在随后数分钟的 agent 运行期间恢复:该线程永远不会得到带 run 链接的生命周期评论。失败场景:
Post triage status comment期间 comments API 瞬时故障(两个写分支均失败 →comment_id=为空);API 随后恢复;run 正常结束;线程里没有任何生命周期评论——本 PR 之前 finalize 会补发一条。可选修复:在空 id 分支中列出 bot 拥有的 marker 评论,仅当一条都不存在时才 POST 终态评论(保住不覆盖属性,也不重新引入选择竞态);或者在步骤注释中记录这一有意取舍——目前该注释只把取消列为空 id 的成因。
— qwen3.8-max via Qwen Code /review (v0.21.7)