diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 2c93ed87397..61c91fb3478 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -1105,21 +1105,21 @@ jobs: | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) | select((.state // "") | IN("CHANGES_REQUESTED", "COMMENTED")) ] | length' \ "${WORKDIR}/rv.json")" - # /review posts Suggestion-level findings as inline comments prefixed - # `**[Suggestion]**`. They are recommendations, not blockers — keep them - # out of the autofix loop, exactly as the suggestion-summary comment they - # replaced always was. Anchored to the body start and paired with the - # /review footer so a human comment that merely quotes the prefix stays - # actionable. jq's `^` is string-anchored, so tolerate leading whitespace - # the review model may emit; `**[Critical]**` can never match either way. - QWEN_SUGGESTION_FILTER='^[[:space:]]*\*\*\[Suggestion\]\*\*' + # Per AGENTS.md's review policy, Suggestion-level findings ARE + # addressed during a PR's first ~5 review rounds; only past that are + # they deferred (with a recorded reason). The loop's MAX_ROUNDS cap is + # that same boundary — every round the loop actually runs is within + # the address-Suggestions window — so /review `**[Suggestion]**` + # inline comments count as actionable feedback here. The agent's + # triage still decides implement-vs-defer per finding and records the + # decision in its summary comment, and the MAX_ROUNDS handoff is the + # defer-to-a-human boundary. N_COMMENTS="$(jq --arg wm "${EFF_WM}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ - --argjson trust "${TRUSTED_ASSOC}" --arg sf "${QWEN_SUGGESTION_FILTER}" ' + --argjson trust "${TRUSTED_ASSOC}" ' [ .[] | select((.created_at // "") > $wm) | select((.user.login // "") != $ab) - | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) - | select((((.body // "") | test($sf)) and ((.body // "") | test("via Qwen Code /review"))) | not) ] | length' \ + | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) ] | length' \ "${WORKDIR}/rc.json")" # Issue-level PR comments are also actionable feedback. Exclude the # bot's own eval markers, and known non-actionable bot comments @@ -1363,15 +1363,16 @@ jobs: "${WORKDIR}/rv.json" echo echo "## Inline comments" - # Mirrors the review-scan gate: /review `**[Suggestion]**` findings are - # recommendations, not blockers, so they never become autofix work. + # Mirrors the review-scan gate: per AGENTS.md, Suggestion-level + # findings are actionable within the first ~5 review rounds (the + # loop's whole operating range), so they are rendered for the agent + # to triage — implement if valuable, otherwise record the deferral. jq -r --arg wm "${WATERMARK}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ - --argjson trust "${TRUSTED_ASSOC}" --arg sf '^[[:space:]]*\*\*\[Suggestion\]\*\*' ' + --argjson trust "${TRUSTED_ASSOC}" ' .[] | select((.created_at // "") > $wm) | select((.user.login // "") != $ab) | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) - | select((((.body // "") | test($sf)) and ((.body // "") | test("via Qwen Code /review"))) | not) | "- \(.path // "?"):\(.line // "?") @\(.user.login): \(.body // "" | gsub("\r"; ""))"' \ "${WORKDIR}/rc.json" echo diff --git a/.qwen/skills/autofix/SKILL.md b/.qwen/skills/autofix/SKILL.md index a0bb6653085..37501256f56 100644 --- a/.qwen/skills/autofix/SKILL.md +++ b/.qwen/skills/autofix/SKILL.md @@ -127,9 +127,13 @@ Classify every feedback point: - Required: correctness bug, broken build/test, security issue, or a `CHANGES_REQUESTED` item naming a real defect. Verify it, then fix minimally. -- Optional: suggestion, nit, or hardening. Prefer NOT to deviate from this PR's - original direction and scope. Implement only if valuable, - codebase-consistent, and in scope; otherwise explain why no action is needed. +- Optional: suggestion, nit, or hardening — including `**[Suggestion]**` + findings from the automated reviewer. Per AGENTS.md's review policy these ARE + addressed during a PR's early review rounds: implement each one that is + valuable, codebase-consistent, and in scope. Decline only with a recorded + reason per finding (out of scope, conflicts with the PR's direction, or not + worth the diff growth) so the deferral is visible in the PR thread — never + drop one silently. If `--conflict true`, merge `origin/` and resolve conflicts by understanding both sides, never blindly taking one side. If false, do not merge diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index fc34e73a6ca..f0592557de7 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -427,6 +427,23 @@ describe('qwen-autofix workflow', () => { ); }); + it('treats Suggestion-level review findings as actionable feedback', () => { + // AGENTS.md: Suggestions ARE addressed during a PR's first ~5 review + // rounds; only past that are they deferred with a recorded reason. The + // loop's MAX_ROUNDS cap is that same boundary, so every round the loop + // runs is within the address-Suggestions window — the scan and the + // feedback rendering must NOT filter `**[Suggestion]**` /review comments. + expect(workflow).not.toContain('QWEN_SUGGESTION_FILTER'); + // The filter REGEX (escaped form only ever appears in filter code, not in + // prose comments) must be gone from both the scan and the feedback render. + expect(workflow).not.toContain('\\*\\*\\[Suggestion\\]\\*\\*'); + // The agent-facing policy lives in the SKILL: implement valuable + // suggestions, decline only with a recorded per-finding reason. + const skill = readAutofixSkill(); + expect(skill).toContain('never'); + expect(skill).toContain('drop one silently'); + }); + it('includes issue-level comments in review feedback scanning', () => { const reviewScanStep = workflow.match(