From 0d246c8727e81ed1b03ee7eb33c23e2dbeea3596 Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Sun, 2 Aug 2026 13:41:28 +0800 Subject: [PATCH] fix(ci): classify mixed AutoFix feedback per item Resolves #8358 --- .qwen/skills/autofix/SKILL.md | 10 ++++- .../2026-08-02-autofix-mixed-feedback.md | 45 +++++++++++++++++++ scripts/tests/qwen-autofix-workflow.test.js | 14 ++++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 docs/plans/2026-08-02-autofix-mixed-feedback.md diff --git a/.qwen/skills/autofix/SKILL.md b/.qwen/skills/autofix/SKILL.md index 833d7c10f65..67f7b249ed3 100644 --- a/.qwen/skills/autofix/SKILL.md +++ b/.qwen/skills/autofix/SKILL.md @@ -285,6 +285,12 @@ Read `git diff origin/...HEAD` first, then `/feedback.md`. Classify every feedback point: +Treat merge readiness and each feedback point's actionability as independent +decisions. An overall `APPROVE` verdict or wording such as "non-blocking", +"follow-up", or "not a regression" does not make every item optional. A +reproduced current correctness defect remains Required. Classify adjacent +diagnostics, comments, tests, and hardening independently. + Address each the way AGENTS.md's Simplicity First and Comments rules demand: the smallest change that resolves the point, no error handling for a condition that cannot occur, no comment that restates the code. Review rounds ratchet @@ -402,6 +408,8 @@ Finish with exactly one outcome: disposition and the reason in a sentence or two, plus the question you need answered when you escalated. Each body is bilingual per GitHub Actions Rules. Omit the file when every inline finding was resolved. -- No change: write `/no-action.md` (bilingual per GitHub Actions Rules). +- No change: write `/no-action.md` only after every actionable + feedback point has an explicit disposition and no verified Required item + remains unresolved. Follow GitHub Actions Rules' bilingual requirement. - The GitHub Actions Rules' objective stop condition applies: write `/failure.md` and do not commit. diff --git a/docs/plans/2026-08-02-autofix-mixed-feedback.md b/docs/plans/2026-08-02-autofix-mixed-feedback.md new file mode 100644 index 00000000000..8a50154fcb9 --- /dev/null +++ b/docs/plans/2026-08-02-autofix-mixed-feedback.md @@ -0,0 +1,45 @@ +# AutoFix Mixed-Feedback Classification Plan + +## Goal + +Prevent takeover review rounds from treating an overall approval or +non-blocking merge verdict as a reason to ignore a reproduced current +correctness defect in the same feedback batch. + +## Root cause + +The PR feedback scanner already included the issue-level review comment from +PR #8301 in `feedback.md`. The address-review agent then applied the review's +overall `APPROVE` verdict to the whole batch and emitted `no-action.md`, instead +of classifying the reproduced correctness defect separately from the adjacent +logging, comment, and coverage suggestions. + +## Implementation + +1. In the shared `address-review` instructions, make review-level merge + readiness and item-level actionability independent decisions. +2. Keep a reproduced current correctness defect Required even when it is + described as non-blocking, a follow-up, or not a regression. Continue to + classify adjacent diagnostics, comments, tests, and hardening independently. +3. Permit the no-change outcome only after every actionable feedback point has + an explicit disposition and no verified Required item remains unresolved. +4. Add focused contract assertions that pin these rules in the existing AutoFix + workflow test without adding a second parser or duplicating workflow policy. + +## Non-goals + +- Do not change feedback ingestion, watermarking, round budgets, or takeover + lifecycle logic; the reported comment already reached the agent. +- Do not make formal `APPROVED` review bodies trigger AutoFix. Ordinary approval + text such as `LGTM` should not create no-op rounds. +- Do not automatically implement every suggestion in an approved review. + +## Verification + +- Prove the new contract test fails against the current skill text, then passes + after the instruction change. +- Run the focused AutoFix workflow test and the full workflow test file. +- Run Prettier, targeted ESLint, `git diff --check`, repository build, and + typecheck. +- Review the final diff for correctness, security, maintainability, test value, + and unnecessary scope. diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 03145080a90..23d9cc8320e 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -4581,6 +4581,20 @@ describe('qwen-autofix workflow', () => { expect(flat).toContain('Simplicity First'); expect(flat).toContain('added no bloat'); expect(flat).toContain('never a reason to bloat the code'); + // Merge readiness applies to the review as a whole, while actionability + // applies to each finding. An approval must not hide a verified defect. + expect(flat).toContain( + "Treat merge readiness and each feedback point's actionability as independent decisions", + ); + expect(flat).toContain( + 'A reproduced current correctness defect remains Required', + ); + expect(flat).toContain( + 'Classify adjacent diagnostics, comments, tests, and hardening independently', + ); + expect(flat).toContain( + 'write `/no-action.md` only after every actionable feedback point has an explicit disposition and no verified Required item remains unresolved', + ); // The rationale is structural, not etiquette: the gate re-runs the same // commands, so skipping them only moves the rejection later. Pin that // framing so the requirement is not softened back into "please verify".