Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions .qwen/skills/triage/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,14 @@ gh label list --repo "$REPO" --limit 200
defined in this skill's files. If a concern about scale or scope arises, raise it
as a question in the Stage 1 comment — never as a block or CHANGES_REQUESTED.
The escalation criteria are those defined in `references/pr-workflow.md`
(Stage 0, Stage 1b, and Stage 1c). Escalation means notifying the
(Stage 0, Stage 1-pre, Stage 1b, and Stage 1c). Escalation means notifying the
maintainer, not rejecting the PR, except where Stage 0 Tier 1 explicitly
prescribes a `CHANGES_REQUESTED` review for large core refactors.
prescribes a `CHANGES_REQUESTED` review for large core refactors, where
Stage 1-pre prescribes a `CHANGES_REQUESTED` review for a linked issue
closed as not planned or a remaining production delta against a merged
fix, or where Stage 1-pre prescribes closing a default-branch PR whose
Comment thread
yiliang114 marked this conversation as resolved.
Outdated
entire production diff is fully subsumed by a merged fix for its linked
issue.
- ⛔ **Never execute PR-derived code.** The review is static. Do not run
`npm`/`node`/`npx`/interpreters/build/test commands against a tree containing
the PR's changes; do not `gh pr checkout`, `git apply` the diff, or run any
Expand Down
149 changes: 145 additions & 4 deletions .qwen/skills/triage/references/pr-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,13 @@ COMMENT_ID=$(gh api "repos/$REPO/issues/$PR_NUMBER/comments" -F body=@/tmp/stage

**Terminal gate exception:** if any terminal exit triggers (Stage 0 core
module hard block, Stage 1a template failure, Stage 1b problem-does-not-exist,
or Stage 1c direction escalation), submit exactly one `CHANGES_REQUESTED`
review and stop. Do not also post or update a Stage 1 issue comment, and do not
continue to Stage 2, Stage 3, or approval.
Stage 1c direction escalation, or Stage 1-pre's two request-changes exits —
linked issue closed as not planned, or a remaining delta against a merged
fix), submit exactly one `CHANGES_REQUESTED` review and stop. Do not also post
or update a Stage 1 issue comment, and do not continue to Stage 2, Stage 3, or
approval. The Stage 1-pre duplicate-close exit is different: it posts the
terminal `stage=1-pre` comment and closes the PR instead of submitting a
review.

**Re-runs:** if the triage runs again on the same PR, update each comment in place. **Resolve the comment id by its stage marker AT PATCH TIME — never from memory, list position, or an earlier stage's bookkeeping.** On a re-run the thread holds four or more bot comments whose list order is not the stage order, and a wrong id silently overwrites another stage's comment (observed on a real re-run: the stage=3 comment clobbered with stage=1 content mid-run). The author filter matters too — the marker is public text anyone can paste into a comment, and the bot PAT may be able to edit other users' comments:

Expand Down Expand Up @@ -75,7 +79,7 @@ Every staged comment (Stage 1 gate-pass, Stage 2, Stage 3) ends with the signatu
<sub>Reviewed at `<HEAD_SHA>` · re-run with `@qwen-code /triage`</sub>
```

**If `HEAD_SHA` comes back empty** (API failure or a null `headRefOid`): **fail closed.** Do not PATCH an existing staged comment — the update rewrites the whole body, so a dropped footer erases the previously valid `Reviewed at` line just as an empty-backtick footer would. Retry the capture, or leave the prior comment (with its footer) untouched until a full OID is available; only a brand-new post that never had a footer may go out without one. Terminal-gate reviews (Stage 1a/1b/1c, submitted via `gh pr review --request-changes`) use the signature only — no footer; they reject before a real review pass.
**If `HEAD_SHA` comes back empty** (API failure or a null `headRefOid`): **fail closed.** Do not PATCH an existing staged comment — the update rewrites the whole body, so a dropped footer erases the previously valid `Reviewed at` line just as an empty-backtick footer would. Retry the capture, or leave the prior comment (with its footer) untouched until a full OID is available; only a brand-new post that never had a footer may go out without one. Terminal-gate reviews (Stage 1-pre request-changes exits and Stage 1a/1b/1c, submitted via `gh pr review --request-changes`) use the signature only — no footer; they reject before a real review pass.

**Approval:** the approve step runs **after** the Stage 3 comment. Comment first, then approve **pinned to the reviewed commit** — `gh pr review --approve` does not bind to a SHA, so a force-push in the check-then-act gap would approve unseen code. Use the reviews API with `commit_id` instead, which records the approval against the exact commit you reviewed (branch protection that requires approval of the latest push then won't count it if the head moved):

Expand Down Expand Up @@ -147,6 +151,141 @@ Save the `worktreePath`. All `read_file`, `grep_search`, `glob` calls below must

This is the most important stage — catch problems before anyone spends time reviewing code.

**1-pre. Duplicate / already-fixed check (run before the template check):**

A PR opened after its linked issue was already fixed stays open forever — no
Comment thread
yiliang114 marked this conversation as resolved.
other gate looks at the linked issue's state. Check it deterministically
before investing in a review. Scope note: the gate executes inside the
triage agent session, so it covers healthy runs only — a run whose agent
cannot reach the model produces no triage at all; that failure shape belongs
to the workflow's response check, not here.

**Default-branch scope.** Run 1-pre only when the PR targets the default
branch:

```bash
BASE_REF=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json baseRefName --jq '.baseRefName')
DEFAULT_BRANCH=$(gh repo view "$REPO" --json defaultBranchRef --jq '.defaultBranchRef.name')
```

- `BASE_REF` != `DEFAULT_BRANCH` (e.g. a backport to a `release/*` branch) →
skip 1-pre and proceed to 1a: such PRs legitimately carry changes that
already exist on the default branch, so the subsumption check below cannot
judge them.

**Linked issues.** Read them from GitHub's own closing-reference parser — it
understands all nine closing-keyword forms (`close`/`closes`/`closed`,
`fix`/`fixes`/`fixed`, `resolve`/`resolves`/`resolved`), URL references, and
cross-repo references; a keyword grep misses most of them:

```bash
ISSUES=$(gh pr view "$PR_NUMBER" --repo "$REPO" \
--json closingIssuesReferences --jq '.closingIssuesReferences[].number' | sort -u)
Comment thread
yiliang114 marked this conversation as resolved.
Outdated
```

The parser is not intent-aware — prose like "resolves #123's closer" links
#123 too — so treat the linkage as input to verify against the issue's
actual state, never as proof by itself.
Comment thread
yiliang114 marked this conversation as resolved.
Outdated

```bash
# Branch on each linked issue's state; $N feeds the closer query below.
for N in $ISSUES; do
SR=$(gh issue view "$N" --repo "$REPO" --json state,stateReason \
--jq '.state + " " + (.stateReason // "")')
# "OPEN" -> proceed to 1a; "CLOSED NOT_PLANNED" -> request changes, stop;
# "CLOSED COMPLETED" -> run the closer query below with this $N
Comment thread
yiliang114 marked this conversation as resolved.
Outdated
done
```

- No linked issues, or every linked issue **open** → proceed to 1a.
Comment thread
yiliang114 marked this conversation as resolved.
- Any linked issue **closed as not planned** → the fix target was rejected:
submit exactly one `CHANGES_REQUESTED` review asking them to reach
agreement in the issue first (bilingual body whose first line is the
`<!-- qwen-triage stage=1-pre -->` marker, @mention the author), and stop:

```bash
gh pr review "$PR_NUMBER" --repo "$REPO" --request-changes --body-file /tmp/stage-1pre-not-planned.md
Comment thread
yiliang114 marked this conversation as resolved.
```

- Any linked issue **closed as completed** → find what closed it (GraphQL —
the REST timeline's `closed` event carries no reliable closer reference):

```bash
gh api graphql -f query='
query($owner: String!, $name: String!, $n: Int!) {
repository(owner: $owner, name: $name) {
issue(number: $n) {
timelineItems(last: 20, itemTypes: [CLOSED_EVENT]) {
nodes {
... on ClosedEvent {
closer {
... on PullRequest { number state merged }
... on Commit { oid }
}
}
}
}
}
}
}' -f owner="${REPO%%/*}" -f name="${REPO##*/}" -F n="$N" \
--jq '.data.repository.issue.timelineItems.nodes // [] | last | .closer | select(. != null and .number != null) | "\(.number) \(.merged)"'
```

Only the LAST (most recent) close event counts — earlier closes belong to
reopen cycles and their closers are stale. If the query fails or emits
nothing (the number is a PR, not an issue; the issue does not exist; the
latest close was manual), treat the closer as unresolved.

- Closed by a **merged PR** → compare this PR's production diff (exclude
test/generated files per the Stage 0 size rules) against the default
branch (`$DEFAULT_BRANCH` — this PR's base, per the scope check above):
- **Fully subsumed** — applying this PR's ENTIRE diff to the default
branch would change nothing: every production line this PR adds already
exists there, AND every production line this PR deletes is already
absent there (check per file via
`gh api "repos/$REPO/contents/<path>?ref=$DEFAULT_BRANCH"`). A diff
with NO production changes (e.g. tests-only) is never fully subsumed —
any file it adds outside the production set is itself a remaining
delta. → post the terminal comment below, then close the PR. This is
the ONLY place triage closes a PR.
- **Any remaining delta** — everything else: an added production line
that is missing there, a deleted production line that still exists
there, or any non-production addition → submit exactly one
`CHANGES_REQUESTED` review: name the merged PR, name the remaining
delta, ask the author to rebase onto the default branch and reduce the
PR to that delta (bilingual body whose first line is the
`<!-- qwen-triage stage=1-pre -->` marker, @mention the author). Stop:

```bash
gh pr review "$PR_NUMBER" --repo "$REPO" --request-changes --body-file /tmp/stage-1pre-remaining-delta.md
```

- Closed manually (no close commit) or the closer cannot be resolved →
never close on ambiguity: flag it in the Stage 1 comment and escalate to
the maintainer.

```bash
cat > /tmp/stage-1pre-duplicate.md <<'EOF'
<!-- qwen-triage stage=1-pre -->

The linked issue #N was already fixed by #M, and every production change in
this PR is already on the default branch — closing as a duplicate of #M. If
something here is NOT covered by #M, say so and this can be reopened.

<details>
<summary>中文说明</summary>

关联 issue #N 已由 #M 修复,本 PR 的生产代码改动均已存在于默认分支,
现作为 #M 的重复 PR 关闭。如本 PR 有 #M 未覆盖的内容,请说明,可以重新打开。

</details>

— _Qwen Code · qwen3.7-max_
EOF
gh pr comment "$PR_NUMBER" --repo "$REPO" --body-file /tmp/stage-1pre-duplicate.md
Comment thread
yiliang114 marked this conversation as resolved.
gh pr close "$PR_NUMBER" --repo "$REPO"
Comment thread
yiliang114 marked this conversation as resolved.
Comment thread
yiliang114 marked this conversation as resolved.
Comment thread
yiliang114 marked this conversation as resolved.
```

**1a. Template check:**

PR body missing required headings from `.github/pull_request_template.md` (read from worktree) → request changes, @mention author, link the template, stop. This is the only public output for this terminal gate.
Expand Down Expand Up @@ -301,6 +440,8 @@ Risk: <if Stage 1e matched, list the high-risk paths and recommended review dept

Save this comment's ID. Terminal exits — stop here if any applies:

- Duplicate of a merged fix, no remaining delta (Stage 1-pre) → closed.
Comment thread
yiliang114 marked this conversation as resolved.
- Duplicate with remaining delta, or issue closed as not planned (Stage 1-pre) → request changes, stopped.
- Core module hard block (Stage 0) → rejected, do not proceed.
- Template failure (Stage 1a) → stopped.
- Problem does not exist (Stage 1b) → request changes, do not proceed to Stage 2.
Expand Down
36 changes: 36 additions & 0 deletions scripts/tests/qwen-triage-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6805,3 +6805,39 @@ describe('triage skips the autofix bot’s own bookkeeping issues (#9264)', () =
expect(autofixDoc.env.AUTOFIX_BOT).toBe(`\${{ ${botIdentityCore} }}`);
});
});

describe('stage 1-pre duplicate gate', () => {
const section = prSkill.slice(
prSkill.indexOf('**1-pre. Duplicate / already-fixed check'),
Comment thread
yiliang114 marked this conversation as resolved.
prSkill.indexOf('**1a. Template check:**'),
);

it('reads linked issues from GitHub closing references, not a keyword grep', () => {
// A keyword grep misses 6 of the 9 closing-keyword forms and matches
// substrings like "prefixes"; GitHub's own parser is the linkage source.
expect(section).toContain('--json closingIssuesReferences');
expect(section).not.toContain("grep -oiE '(fixes|closes|resolves)");
});

it('runs only for PRs targeting the default branch', () => {
// Backports to release/* branches legitimately carry changes that already
// exist on the default branch; without this scope the gate closes them.
expect(section).toContain('Run 1-pre only when the PR targets the default');
expect(section).toContain('defaultBranchRef');
expect(section).not.toContain('?ref=main');
});

it('defines subsumption over the full diff, never over added lines alone', () => {
// Added-lines-only quantification closes deletions-only diffs vacuously;
// the deleted-lines clause must stay.
expect(section).toContain('every production line this PR adds');
expect(section).toContain('every production line this PR deletes');
});

it('never closes a diff with no production changes', () => {
// Stage 0 exclusions empty the comparison set for tests-only PRs; such a
// diff must be a remaining delta, never "Fully subsumed".
Comment thread
yiliang114 marked this conversation as resolved.
expect(section).toContain('NO production changes');
expect(section).toContain('never fully subsumed');
});
});
Loading