Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 17 additions & 3 deletions .github/workflows/qwen-triage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,10 @@ jobs:
concurrency:
# GitHub evaluates concurrency before the job `if`, but after `needs`.
# Keep non-runnable PR/comment triggers out of the shared per-number
# group so they cannot cancel or replace an authorized run.
# group so they cannot cancel or replace an authorized run — including
# bot-created issues runs (#9264): the job `if` skips them, but a run
# left in the shared group would still cancel an in-progress triage of
# the same issue before its own skip is evaluated.
group: >-
${{
(
Expand All @@ -459,7 +462,10 @@ jobs:
(github.event_name == 'issue_comment' &&
(github.event.issue.state != 'open' ||
needs.authorize.outputs.should_run != 'true' ||
!startsWith(github.event.comment.body, '@qwen-code /triage')))
!startsWith(github.event.comment.body, '@qwen-code /triage'))) ||
(github.event_name == 'issues' &&
github.event.issue.user.login ==
(vars.AUTOFIX_BOT_LOGIN || 'qwen-code-dev-bot'))
) &&
format('{0}-run-{1}', github.workflow, github.run_id) ||
format('{0}-{1}', github.workflow, github.event.issue.number || github.event.pull_request.number || github.event.inputs.number)
Expand Down Expand Up @@ -501,9 +507,17 @@ jobs:
# mention the phrase in quoted text or mid-sentence descriptions.
# always() so the job still evaluates when the upstream `authorize` job is
# skipped (issues / workflow_dispatch paths, which need no permission gate).
# The issues clause is conditioned on the creator NOT being the autofix
# bot (#9264): every PR that defers findings for the first time opens a
# tracking issue upserted by that bot, and the open issues trigger triaged
# the bookkeeping issue with a full agent run per deferral. The identity
# is the same one qwen-autofix.yml upserts under (AUTOFIX_BOT), so the
# guard tracks a rename on either side via the shared variable.
if: >-
always() && (
github.event_name == 'issues' ||
(github.event_name == 'issues' &&
github.event.issue.user.login !=
(vars.AUTOFIX_BOT_LOGIN || 'qwen-code-dev-bot')) ||
(github.event_name == 'workflow_dispatch' &&
github.event.inputs.number != '' &&
github.event.inputs.tmux_pr == '') ||
Expand Down
43 changes: 43 additions & 0 deletions scripts/tests/qwen-triage-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5982,3 +5982,46 @@ describe('triage job budget', () => {
}
});
});

describe('triage skips the autofix bot’s own bookkeeping issues (#9264)', () => {
// Every PR that defers findings for the first time opens a tracking issue
// upserted by the autofix bot, and `issues: [opened, edited, reopened]`
// triaged that bookkeeping issue with a full agent run per deferral. The
// guard keys on the same identity qwen-autofix.yml upserts under, so a
// rename on one side without the other silently re-opens the waste.
const botIdentityCore = "vars.AUTOFIX_BOT_LOGIN || 'qwen-code-dev-bot'";
const botIdentity = `(${botIdentityCore})`;

// The parsed expressions keep their YAML line breaks, so whitespace is
// normalized before matching — the pin must survive a re-wrap, not test it.
const flat = (value) => String(value).replace(/\s+/g, ' ');

it('conditions the triage job’s issues clause on the creator not being the bot', () => {
// Parsed, not raw-text containment: a commented-out guard would still
// match a substring pin.
const doc = parse(workflow);
expect(flat(doc.jobs.triage.if)).toContain(
`(github.event_name == 'issues' && github.event.issue.user.login != ${botIdentity}) || (github.event_name == 'workflow_dispatch'`,
);
Comment thread
yiliang114 marked this conversation as resolved.
});

it('routes bot-created issues runs to a per-run concurrency group', () => {
// GitHub evaluates concurrency BEFORE the job `if`: a bot bookkeeping run
// inside the shared per-number group cancels an in-progress triage of the
// same issue even though its own job skips.
const doc = parse(workflow);
expect(flat(doc.jobs.triage.concurrency.group)).toContain(
`!startsWith(github.event.comment.body, '@qwen-code /triage'))) || (github.event_name == 'issues' && github.event.issue.user.login == ${botIdentity}) ) && format('{0}-run-{1}', github.workflow, github.run_id)`,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This pin stops one clause short: it anchors the bot-side routing to the per-run group but not the complement — that every other event still falls through to the shared per-number group format('{0}-{1}', …). Grep confirms this pin is the only {0}- format string in scripts/tests, so nothing covers the fallback clause. — Failure scenario: demonstrated surviving mutant — changing the workflow's shared-group fallback from format('{0}-{1}', …) to format('{0}-run-{1}', …) routes every triage run into a unique per-run group, silently disabling the cancel-in-progress supersede the workflow relies on for issues/dispatch triggers (duplicate concurrent full agent runs on the same issue, no cancellation) — and all 134 tests still pass. Extending the pin by one clause kills the mutant. Witness (probe flip): baseline 134/134 pass; the fallback mutant survives 134/134; the extended pin below fails against the mutant (expected …format('{0}-{1}'…, received …format('{0}-run-{1}'…) and passes 134/134 against the restored tree — the suggested fix is verified sound, not hypothesized.

Suggested change
`!startsWith(github.event.comment.body, '@qwen-code /triage'))) || (github.event_name == 'issues' && github.event.issue.user.login == ${botIdentity}) ) && format('{0}-run-{1}', github.workflow, github.run_id)`,
`!startsWith(github.event.comment.body, '@qwen-code /triage'))) || (github.event_name == 'issues' && github.event.issue.user.login == ${botIdentity}) ) && format('{0}-run-{1}', github.workflow, github.run_id) || format('{0}-{1}', github.workflow, github.event.issue.number || github.event.pull_request.number || github.event.inputs.number)`,

— qwen3.8-max via Qwen Code /review (v0.21.12)

);
});

it('keeps the guard identity in sync with the autofix workflow', () => {
// qwen-autofix.yml defines AUTOFIX_BOT as the same variable-with-fallback
// (inside a bare `${{ }}`, so without the expression's parentheses).
expect(botIdentity).toContain(botIdentityCore);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Tautological assertion: botIdentity is defined two lines above as (${botIdentityCore}), and "(" + s + ")" contains s for every possible string — and this line performs no file I/O, so no mutation of either workflow file can falsify it. The test title's guarantee is carried by the adjacent toBe against parsed qwen-autofix.yml, not by this line. — Failure scenario: the assertion reads as verification but is unfalsifiable; a future refactor that weakens or breaks the adjacent load-bearing toBe leaves this line green, still appearing to vouch for the sync — this suite's own doctrine is that a control that cannot fail proves nothing about the assertions it vouches for. Deleting the line loses nothing: the toBe plus the identity embedded in the first two tests carry the guarantee.

Suggested change
expect(botIdentity).toContain(botIdentityCore);

— qwen3.8-max via Qwen Code /review (v0.21.12)

const autofixDoc = parse(
readFileSync('.github/workflows/qwen-autofix.yml', 'utf8'),
);
expect(autofixDoc.env.AUTOFIX_BOT).toBe(`\${{ ${botIdentityCore} }}`);
});
});
Loading