From 1cbf2e8fc74ac87b76fb240cf59e6dc1c23e707a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Sun, 9 Aug 2026 07:01:03 +0800 Subject: [PATCH] feat(ci): auto-assign issues to area owners from labels (#8668) * feat(ci): auto-assign issues to area owners from labels Route labelled issues to a maintainer with push access, without putting issue text in the path of a write token. Assignment is a pure function of the issue's labels and a checked-in label -> owner map, evaluated by a standalone workflow on issues:labeled. No model runs in the assignment path and the script never reads issue title, body, or comments, so untrusted issue text cannot select an assignee. Owners come from CODEOWNERS (asserted by test) and every candidate is re-checked against the collaborator permission API before the write, so editing the map cannot grant access. Among eligible owners the least loaded wins, rotating by issue number to break ties. * fix(ci): decouple issue owner map from CODEOWNERS CODEOWNERS answers who owns a code path, which is narrower than who may be assigned an issue in an area: the repository has ~44 collaborators with push access against 7 CODEOWNERS entries, so the membership test would have rejected legitimate additions such as admins and maintainers who own no path. Drop that assertion and document the actual process for adding owners. The live collaborator permission check remains the boundary. Add the validation the map does need: duplicate owners would skew load balancing, and duplicate area names would silently shadow each other under first-match-wins. * fix(ci): keep issue ownership triggers disjoint * fix(ci): recheck issue owner assignment before write * test(ci): cover issue owner label recheck * docs(ci): Correct CODEOWNERS count in issue assignment rationale * fix(automation): preserve autofix issue ownership * feat(ci): widen core issue owner pool to active repository maintainers * feat(ci): add four more collaborators to core issue owner pool * fix(ci): tighten issue-owner map validation and sync trigger docs * fix(ci): tighten issue-owner assignment tests and login validation (#8668) --------- Co-authored-by: qwen-code-dev-bot --- .github/issue-owners.json | 32 ++ .github/scripts/assign-issue-owner.mjs | 279 +++++++++++++ .github/scripts/assign-issue-owner.test.mjs | 385 ++++++++++++++++++ .github/workflows/assign-issue-owner.yml | 64 +++ .github/workflows/ci.yml | 2 +- .../2026-08-07-issue-auto-assignment.md | 119 ++++++ 6 files changed, 880 insertions(+), 1 deletion(-) create mode 100644 .github/issue-owners.json create mode 100644 .github/scripts/assign-issue-owner.mjs create mode 100644 .github/scripts/assign-issue-owner.test.mjs create mode 100644 .github/workflows/assign-issue-owner.yml create mode 100644 docs/design/2026-08-07-issue-auto-assignment.md diff --git a/.github/issue-owners.json b/.github/issue-owners.json new file mode 100644 index 0000000000..936cea0fa7 --- /dev/null +++ b/.github/issue-owners.json @@ -0,0 +1,32 @@ +{ + "$comment": "Label-driven issue assignment map; areas are keyed on the existing issue label taxonomy. Assignment is a pure function of an issue's labels — no model output is involved. Owners need push access but do NOT need a CODEOWNERS entry; every candidate is re-checked against the collaborator API at write time, so adding a login here cannot grant access to someone who lacks it. Areas match in file order, first match wins. See docs/design/2026-08-07-issue-auto-assignment.md.", + "requireLabels": ["need-discussion"], + "skipLabels": [ + "welcome-pr", + "feature/need-help", + "good first issue", + "help wanted", + "autofix/approved", + "autofix/in-progress" + ], + "areas": [ + { + "name": "core", + "labels": ["category/core", "scope/core"], + "owners": [ + "wenshao", + "tanzhenxin", + "yiliang114", + "LaZzyMan", + "doudouOUC", + "pomelo-nwu", + "DennisYu07", + "jifeng", + "chiga0", + "qqqys", + "ytahdn", + "BenGuanRan" + ] + } + ] +} diff --git a/.github/scripts/assign-issue-owner.mjs b/.github/scripts/assign-issue-owner.mjs new file mode 100644 index 0000000000..7916aba621 --- /dev/null +++ b/.github/scripts/assign-issue-owner.mjs @@ -0,0 +1,279 @@ +#!/usr/bin/env node +// Assign an issue to an area owner, derived purely from the issue's labels. +// +// This script never reads issue title, body, or comments, so untrusted issue +// text cannot steer the assignment. The triage agent's only influence is the +// labels it applies, drawn from the repository's existing label taxonomy; the +// label -> owner map lives in .github/issue-owners.json and is reviewed like +// any other checked-in file. Push access is re-verified against the live +// collaborator API before every write, so an edit to that map cannot assign +// someone who does not already have permission. +import { appendFileSync, readFileSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import { pathToFileURL } from 'node:url'; + +const OWNERS_FILE = '.github/issue-owners.json'; +const WRITE_PERMISSIONS = new Set(['admin', 'maintain', 'write']); +const LOGIN = /^(?!.*--)[A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?$/; + +function isStringArray(value) { + return Array.isArray(value) && value.every((v) => typeof v === 'string'); +} + +export function loadPolicy(raw) { + const policy = JSON.parse(raw); + if (!policy || typeof policy !== 'object' || Array.isArray(policy)) { + throw new Error(`${OWNERS_FILE}: not an object`); + } + // An empty label entry can never match; in requireLabels it would silently + // skip every issue on a green run, so reject it like other malformed config. + if ( + !isStringArray(policy.requireLabels) || + !isStringArray(policy.skipLabels) || + policy.requireLabels.some((label) => label.length === 0) || + policy.skipLabels.some((label) => label.length === 0) + ) { + throw new Error( + `${OWNERS_FILE}: requireLabels/skipLabels must be non-empty strings`, + ); + } + if (!Array.isArray(policy.areas) || policy.areas.length === 0) { + throw new Error(`${OWNERS_FILE}: areas must be a non-empty array`); + } + const areaNames = new Set(); + for (const area of policy.areas) { + if (typeof area?.name !== 'string' || area.name.length === 0) { + throw new Error(`${OWNERS_FILE}: every area needs a name`); + } + // First match wins, so two areas sharing a name silently shadow one another. + if (areaNames.has(area.name)) { + throw new Error(`${OWNERS_FILE}: duplicate area ${area.name}`); + } + areaNames.add(area.name); + if ( + !isStringArray(area.labels) || + area.labels.length === 0 || + area.labels.some((label) => label.length === 0) + ) { + throw new Error(`${OWNERS_FILE}: area ${area.name} needs labels`); + } + if (!Array.isArray(area.owners) || area.owners.length === 0) { + throw new Error(`${OWNERS_FILE}: area ${area.name} needs owners`); + } + const seen = new Set(); + for (const owner of area.owners) { + // Rejected here rather than at the gh call so a typo fails the config, + // not a single assignment attempt. + if (typeof owner !== 'string' || !LOGIN.test(owner)) { + throw new Error(`${OWNERS_FILE}: invalid login in ${area.name}`); + } + // A repeated login would be counted twice and win ties unfairly. + const normalizedOwner = owner.toLowerCase(); + if (seen.has(normalizedOwner)) { + throw new Error(`${OWNERS_FILE}: duplicate owner ${owner}`); + } + seen.add(normalizedOwner); + } + } + return policy; +} + +// Returns a human-readable reason to skip, or null to proceed. Ordered so the +// most informative reason wins when several apply. +export function skipReason(policy, issue) { + const labels = new Set(issue.labels.map((label) => label.name)); + if (issue.state !== 'OPEN') return 'issue is not open'; + if (issue.assignees.length > 0) return 'issue already has an assignee'; + const skipped = policy.skipLabels.filter((label) => labels.has(label)); + if (skipped.length > 0) return `carries ${skipped.join(', ')}`; + const missing = policy.requireLabels.filter((label) => !labels.has(label)); + if (missing.length > 0) return `missing ${missing.join(', ')}`; + return null; +} + +// First matching area wins, so file order is the documented precedence. +export function matchArea(policy, issue) { + const labels = new Set(issue.labels.map((label) => label.name)); + return ( + policy.areas.find((area) => + area.labels.some((label) => labels.has(label)), + ) ?? null + ); +} + +// Rotate by issue number before the stable minimum so a set of equally loaded +// owners spreads round-robin instead of always landing on the first entry. +export function pickOwner(owners, loadByOwner, issueNumber) { + const offset = issueNumber % owners.length; + const rotated = [...owners.slice(offset), ...owners.slice(0, offset)]; + return rotated.reduce((best, owner) => + loadByOwner.get(owner) < loadByOwner.get(best) ? owner : best, + ); +} + +function gh(args) { + const result = spawnSync('gh', args, { + encoding: 'utf8', + maxBuffer: 10 * 1024 * 1024, + }); + if (result.status !== 0) { + throw new Error(result.stderr.trim() || `gh ${args.join(' ')} failed`); + } + return result.stdout.trim(); +} + +function record(lines) { + const body = `${lines.join('\n')}\n`; + process.stdout.write(body); + if (process.env.GITHUB_STEP_SUMMARY) { + appendFileSync(process.env.GITHUB_STEP_SUMMARY, body); + } +} + +// A candidate who lost push access, renamed, or deleted their account makes +// the permission lookup fail; warn and drop them rather than failing the run +// over one stale entry. +function canWrite(repository, login) { + try { + return WRITE_PERMISSIONS.has( + gh([ + 'api', + `repos/${repository}/collaborators/${login}/permission`, + '--jq', + '.permission', + ]), + ); + } catch (error) { + console.warn( + `::warning::Cannot verify push access for @${login}: ${error.message}`, + ); + return false; + } +} + +function openIssueCount(repository, login) { + return Number( + gh([ + 'issue', + 'list', + '--repo', + repository, + '--state', + 'open', + '--assignee', + login, + '--limit', + '100', + '--json', + 'number', + '--jq', + 'length', + ]), + ); +} + +function main() { + const repository = process.env.GITHUB_REPOSITORY; + const issueNumber = Number(process.env.ISSUE_NUMBER); + const dryRun = process.env.DRY_RUN === 'true'; + if (!repository || !/^[^/]+\/[^/]+$/.test(repository)) { + throw new Error('Invalid repository'); + } + if (!Number.isSafeInteger(issueNumber) || issueNumber < 1) { + throw new Error('Invalid issue number'); + } + + const policy = loadPolicy(readFileSync(OWNERS_FILE, 'utf8')); + const issue = JSON.parse( + gh([ + 'issue', + 'view', + String(issueNumber), + '--repo', + repository, + '--json', + 'state,labels,assignees', + ]), + ); + + const skip = skipReason(policy, issue); + if (skip) { + record([`Assignment: skipped — ${skip}`]); + return; + } + + const area = matchArea(policy, issue); + if (!area) { + record(['Assignment: skipped — no area label matched']); + return; + } + + const eligible = area.owners.filter((owner) => canWrite(repository, owner)); + if (eligible.length === 0) { + console.warn( + `::warning::No owner of area ${area.name} has push access; check ${OWNERS_FILE}.`, + ); + record([`Assignment: skipped — no eligible owner for area ${area.name}`]); + return; + } + + const loadByOwner = new Map( + eligible.map((owner) => [owner, openIssueCount(repository, owner)]), + ); + const assignee = pickOwner(eligible, loadByOwner, issueNumber); + + if (dryRun) { + record([ + `Area: ${area.name}`, + `Assignment: dry-run — would assign @${assignee} (${loadByOwner.get(assignee)} open)`, + ]); + return; + } + + const latestIssue = JSON.parse( + gh([ + 'issue', + 'view', + String(issueNumber), + '--repo', + repository, + '--json', + 'state,labels,assignees', + ]), + ); + const latestSkip = skipReason(policy, latestIssue); + if (latestSkip) { + record([`Assignment: skipped — ${latestSkip}`]); + return; + } + if (matchArea(policy, latestIssue)?.name !== area.name) { + record(['Assignment: skipped — issue labels changed']); + return; + } + + gh([ + 'issue', + 'edit', + String(issueNumber), + '--repo', + repository, + '--add-assignee', + assignee, + ]); + record([ + `Area: ${area.name}`, + `Assignment: assigned @${assignee} (${loadByOwner.get(assignee)} open)`, + ]); +} + +if ( + process.argv[1] && + import.meta.url === pathToFileURL(process.argv[1]).href +) { + try { + main(); + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exit(1); + } +} diff --git a/.github/scripts/assign-issue-owner.test.mjs b/.github/scripts/assign-issue-owner.test.mjs new file mode 100644 index 0000000000..ff5695e35b --- /dev/null +++ b/.github/scripts/assign-issue-owner.test.mjs @@ -0,0 +1,385 @@ +// Guards for label-driven issue assignment. Two things are load-bearing and +// have no other test: the pure policy functions that decide *whether* and *to +// whom* an issue is assigned, and the workflow invariants (repository guard, +// permission split, step-scoped token) that keep the write token narrow. +import assert from 'node:assert/strict'; +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { afterEach, describe, it } from 'node:test'; +import { fileURLToPath } from 'node:url'; +import { parse } from 'yaml'; + +import { + loadPolicy, + matchArea, + pickOwner, + skipReason, +} from './assign-issue-owner.mjs'; + +const scriptsDir = dirname(fileURLToPath(import.meta.url)); +const repoRoot = join(scriptsDir, '..', '..'); +const script = join(scriptsDir, 'assign-issue-owner.mjs'); +const ownersRaw = readFileSync( + join(repoRoot, '.github', 'issue-owners.json'), + 'utf8', +); +const policy = loadPolicy(ownersRaw); +const tempDirs = []; + +const coreIssue = { + state: 'OPEN', + assignees: [], + labels: [{ name: 'category/core' }, { name: 'need-discussion' }], +}; + +afterEach(() => { + while (tempDirs.length > 0) { + rmSync(tempDirs.pop(), { recursive: true, force: true }); + } +}); + +describe('assign-issue-owner: owner map', () => { + it('parses the checked-in map', () => { + assert.ok(policy.areas.length > 0); + assert.ok(policy.areas.every((area) => area.owners.length > 0)); + }); + + // Deliberately NOT asserted against CODEOWNERS: that file answers "who owns + // this code path", which is a narrower question than "who may be assigned an + // issue in this area". The repository has ~44 collaborators with push access + // and only 4 CODEOWNERS path rules, so that check would reject legitimate + // additions. Push access is verified against the live API at write time. + it('rejects a duplicated owner that would skew load balancing', () => { + const broken = JSON.parse(ownersRaw); + broken.areas[0].owners = ['wenshao', 'WENSHAO']; + assert.throws(() => loadPolicy(JSON.stringify(broken)), /duplicate owner/); + }); + + it('rejects two areas sharing a name', () => { + const broken = JSON.parse(ownersRaw); + broken.areas = [...broken.areas, structuredClone(broken.areas[0])]; + assert.throws(() => loadPolicy(JSON.stringify(broken)), /duplicate area/); + }); + + it('rejects a malformed login rather than passing it to gh', () => { + // GitHub logins cannot start or end with a hyphen or contain consecutive + // hyphens; a typo'd owner is only dropped at the runtime permission + // check, so reject the whole config up front instead. + for (const login of ['not a login', 'alice-', '-alice', 'a--b']) { + const broken = JSON.parse(ownersRaw); + broken.areas[0].owners = [login]; + assert.throws(() => loadPolicy(JSON.stringify(broken)), /invalid login/); + } + }); + + it('rejects an empty label entry that could never match', () => { + const requireBroken = JSON.parse(ownersRaw); + requireBroken.requireLabels = ['']; + assert.throws( + () => loadPolicy(JSON.stringify(requireBroken)), + /non-empty strings/, + ); + + const skipBroken = JSON.parse(ownersRaw); + skipBroken.skipLabels = ['welcome-pr', '']; + assert.throws( + () => loadPolicy(JSON.stringify(skipBroken)), + /non-empty strings/, + ); + + const areaBroken = JSON.parse(ownersRaw); + areaBroken.areas[0].labels = ['']; + assert.throws(() => loadPolicy(JSON.stringify(areaBroken)), /needs labels/); + }); + + it('rejects an area with no labels', () => { + const broken = JSON.parse(ownersRaw); + broken.areas[0].labels = []; + assert.throws(() => loadPolicy(JSON.stringify(broken)), /needs labels/); + }); + + it('rejects a root that is not an object', () => { + for (const raw of ['null', '[]', '"policy"']) { + assert.throws(() => loadPolicy(raw), /not an object/); + } + }); + + it('rejects non-array label lists', () => { + for (const key of ['requireLabels', 'skipLabels']) { + const broken = JSON.parse(ownersRaw); + broken[key] = 'need-discussion'; + assert.throws( + () => loadPolicy(JSON.stringify(broken)), + /non-empty strings/, + ); + } + }); + + it('rejects a missing, empty, or non-array areas list', () => { + for (const areas of [undefined, [], 'core']) { + const broken = JSON.parse(ownersRaw); + broken.areas = areas; + assert.throws( + () => loadPolicy(JSON.stringify(broken)), + /areas must be a non-empty array/, + ); + } + }); + + it('rejects an unnamed area or an area with no owners', () => { + const nameless = JSON.parse(ownersRaw); + delete nameless.areas[0].name; + assert.throws(() => loadPolicy(JSON.stringify(nameless)), /needs a name/); + + const ownerless = JSON.parse(ownersRaw); + ownerless.areas[0].owners = []; + assert.throws(() => loadPolicy(JSON.stringify(ownerless)), /needs owners/); + }); + + it('rejects a non-string owner entry', () => { + const broken = JSON.parse(ownersRaw); + broken.areas[0].owners = [42]; + assert.throws(() => loadPolicy(JSON.stringify(broken)), /invalid login/); + }); +}); + +describe('assign-issue-owner: skip policy', () => { + it('assigns an open, unassigned, correctly labelled issue', () => { + assert.equal(skipReason(policy, coreIssue), null); + }); + + it('leaves a closed issue alone', () => { + assert.match( + skipReason(policy, { ...coreIssue, state: 'CLOSED' }), + /not open/, + ); + }); + + it('never reassigns an issue that already has an assignee', () => { + assert.match( + skipReason(policy, { ...coreIssue, assignees: [{ login: 'someone' }] }), + /already has an assignee/, + ); + }); + + it('leaves community-facing issues to the community', () => { + assert.match( + skipReason(policy, { + ...coreIssue, + labels: [...coreIssue.labels, { name: 'welcome-pr' }], + }), + /welcome-pr/, + ); + }); + + it('leaves autofix-owned issues to autofix', () => { + for (const label of ['autofix/approved', 'autofix/in-progress']) { + assert.equal( + skipReason(policy, { + ...coreIssue, + labels: [...coreIssue.labels, { name: label }], + }), + `carries ${label}`, + ); + } + }); + + it('waits for every required label', () => { + assert.match( + skipReason(policy, { ...coreIssue, labels: [{ name: 'category/core' }] }), + /missing need-discussion/, + ); + }); +}); + +describe('assign-issue-owner: area matching', () => { + it('matches an area on any of its labels', () => { + assert.equal(matchArea(policy, coreIssue).name, 'core'); + assert.equal( + matchArea(policy, { ...coreIssue, labels: [{ name: 'scope/core' }] }) + .name, + 'core', + ); + }); + + it('returns no area when nothing matches', () => { + assert.equal( + matchArea(policy, { ...coreIssue, labels: [{ name: 'category/ui' }] }), + null, + ); + }); +}); + +describe('assign-issue-owner: owner selection', () => { + const owners = ['a', 'b', 'c']; + + it('picks the least loaded owner', () => { + const load = new Map([ + ['a', 7], + ['b', 1], + ['c', 4], + ]); + assert.equal(pickOwner(owners, load, 1), 'b'); + assert.equal(pickOwner(owners, load, 2), 'b'); + }); + + it('rotates between equally loaded owners instead of always picking the first', () => { + const load = new Map(owners.map((owner) => [owner, 0])); + const picks = [0, 1, 2, 3].map((n) => pickOwner(owners, load, n)); + assert.deepEqual(picks, ['a', 'b', 'c', 'a']); + }); +}); + +// The stub reports wenshao as the least loaded owner so the pick is +// unambiguous regardless of the rotation offset for issue 42. +function runAssign(dryRun, secondIssueJson = '') { + const dir = mkdtempSync(join(tmpdir(), 'assign-issue-owner-')); + tempDirs.push(dir); + const log = join(dir, 'gh.log'); + const gh = join(dir, 'gh'); + writeFileSync( + gh, + `#!/bin/sh +printf '%s\\n' "$*" >> "$GH_STUB_LOG" +case "$*" in + "issue view 42 "*) + count=$(cat "$GH_STUB_VIEW_COUNT" 2>/dev/null || echo 0) + count=$((count + 1)) + printf '%s' "$count" > "$GH_STUB_VIEW_COUNT" + if [ "$count" = 2 ] && [ -n "$GH_STUB_SECOND_ISSUE" ]; then + printf '%s' "$GH_STUB_SECOND_ISSUE" + else + printf '%s' '{"state":"OPEN","labels":[{"name":"category/core"},{"name":"need-discussion"}],"assignees":[]}' + fi + ;; + *"/collaborators/"*"/permission"*) printf '%s' 'write' ;; + *"--assignee wenshao"*"--json number"*) printf '%s' '0' ;; + *"issue list"*"--json number"*) printf '%s' '5' ;; +esac +`, + ); + chmodSync(gh, 0o755); + const result = spawnSync(process.execPath, [script], { + cwd: repoRoot, + encoding: 'utf8', + env: { + ...process.env, + PATH: `${dir}:${process.env.PATH}`, + GH_STUB_LOG: log, + GH_STUB_VIEW_COUNT: join(dir, 'view-count'), + GH_STUB_SECOND_ISSUE: secondIssueJson, + GITHUB_REPOSITORY: 'QwenLM/qwen-code', + GITHUB_STEP_SUMMARY: '', + ISSUE_NUMBER: '42', + DRY_RUN: String(dryRun), + }, + }); + assert.equal(result.status, 0, result.stderr); + return { log: readFileSync(log, 'utf8'), stdout: result.stdout }; +} + +describe('assign-issue-owner: apply boundary', () => { + it('verifies push access before assigning', () => { + const { log } = runAssign(false); + assert.match(log, /collaborators\/wenshao\/permission/); + }); + + it('performs no mutation in dry-run mode', () => { + const { log, stdout } = runAssign(true); + assert.doesNotMatch(log, /issue edit/); + assert.match(stdout, /dry-run — would assign @wenshao/); + }); + + it('assigns the least loaded eligible owner', () => { + const { log, stdout } = runAssign(false); + assert.match(log, /issue edit 42 .*--add-assignee wenshao/); + assert.match(stdout, /assigned @wenshao/); + }); + + it('re-checks issue state immediately before assigning', () => { + const { log, stdout } = runAssign( + false, + '{"state":"OPEN","labels":[{"name":"category/core"},{"name":"need-discussion"}],"assignees":[{"login":"someone"}]}', + ); + assert.doesNotMatch(log, /issue edit/); + assert.match(stdout, /skipped — issue already has an assignee/); + }); + + it('re-checks area labels immediately before assigning', () => { + const { log, stdout } = runAssign( + false, + '{"state":"OPEN","labels":[{"name":"category/ui"},{"name":"need-discussion"}],"assignees":[]}', + ); + assert.doesNotMatch(log, /issue edit/); + assert.match(stdout, /skipped — issue labels changed/); + }); +}); + +const doc = parse( + readFileSync( + join(repoRoot, '.github', 'workflows', 'assign-issue-owner.yml'), + 'utf8', + ), +); +const assignJob = doc.jobs.assign; +const checkoutStep = assignJob.steps.find((s) => + s.uses?.startsWith('actions/checkout@'), +); +const assignStep = assignJob.steps.find((s) => s.name === 'Assign area owner'); + +describe('assign-issue-owner: workflow invariants', () => { + it('runs only on the canonical repository', () => { + assert.equal( + String(assignJob.if), + "${{ github.repository == 'QwenLM/qwen-code' }}", + ); + }); + + it('grants issues:write to the job, not the whole workflow', () => { + assert.deepEqual(doc.permissions, { contents: 'read' }); + assert.deepEqual(assignJob.permissions, { + contents: 'read', + issues: 'write', + }); + }); + + it('scopes the write token to the step and keeps checkout credential-free', () => { + assert.equal( + assignJob.env, + undefined, + 'job-level env exposes GH_TOKEN to every step', + ); + assert.equal(assignStep.env.GH_TOKEN, '${{ github.token }}'); + assert.equal(assignStep.env.DRY_RUN, "${{ inputs.dry_run || 'false' }}"); + assert.equal( + assignStep.env.ISSUE_NUMBER, + '${{ github.event.issue.number || inputs.number }}', + ); + assert.equal(checkoutStep.with['persist-credentials'], false); + }); + + it('never runs a model or reads issue text', () => { + const serialized = JSON.stringify(doc); + assert.doesNotMatch( + serialized, + /OPENAI_API_KEY|qwen --|github\.event\.issue\.(title|body)/, + ); + }); + + it('fires on label changes without cancelling an in-flight assignment', () => { + assert.deepEqual(doc.on.issues.types, ['labeled', 'unlabeled']); + assert.equal( + doc.concurrency.group, + 'assign-issue-owner-${{ github.event.issue.number || inputs.number }}', + ); + assert.equal(doc.concurrency['cancel-in-progress'], false); + }); +}); diff --git a/.github/workflows/assign-issue-owner.yml b/.github/workflows/assign-issue-owner.yml new file mode 100644 index 0000000000..ef6a35268c --- /dev/null +++ b/.github/workflows/assign-issue-owner.yml @@ -0,0 +1,64 @@ +name: 'Assign issue owner' + +# Auto-assign an issue to an area owner once triage has labelled it. +# +# Assignment is a pure function of the issue's labels and .github/issue-owners.json +# — no model runs in this workflow and the script never reads issue title, body, +# or comments, so untrusted issue text cannot select an assignee. Push access is +# re-verified against the collaborator API before every write. +# +# Trigger note: label events only fire for label writes made with a PAT. +# The triage agent labels with QWEN_CODE_BOT_TOKEN/CI_BOT_PAT (see +# qwen-triage.yml), so this chains correctly. If triage ever falls back to the +# default GITHUB_TOKEN for labelling, this workflow stops firing and must be +# rehung on `workflow_run` instead — GITHUB_TOKEN writes do not trigger +# downstream workflow runs. +# +# Each label change fires once, so a triage run that applies five labels +# queues five runs. Every run after the first sees an assignee and no-ops. + +on: + issues: + types: ['labeled', 'unlabeled'] + workflow_dispatch: + inputs: + number: + description: 'Issue number to evaluate' + required: true + type: 'string' + dry_run: + description: 'Report the chosen assignee without assigning' + required: false + default: true + type: 'boolean' + +permissions: + contents: 'read' + +concurrency: + group: 'assign-issue-owner-${{ github.event.issue.number || inputs.number }}' + cancel-in-progress: false + +jobs: + assign: + if: "${{ github.repository == 'QwenLM/qwen-code' }}" + runs-on: 'ubuntu-latest' + timeout-minutes: 5 + permissions: + contents: 'read' + issues: 'write' + steps: + - name: 'Checkout owner map' + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 + with: + sparse-checkout: |- + .github/issue-owners.json + .github/scripts/assign-issue-owner.mjs + persist-credentials: false + + - name: 'Assign area owner' + env: + GH_TOKEN: '${{ github.token }}' + ISSUE_NUMBER: '${{ github.event.issue.number || inputs.number }}' + DRY_RUN: "${{ inputs.dry_run || 'false' }}" + run: 'node .github/scripts/assign-issue-owner.mjs' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0efa02bb26..ae311d193c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,7 @@ env: # BOTH the github_ci_only helper step and the full-profile Test step, so a # new helper test can't be added to one path and silently dropped from the # other. - HELPER_TESTS: '.github/scripts/pr-safety-precheck.test.mjs .github/scripts/cap-release-notes.test.mjs .github/scripts/ci/classify-profile.test.mjs .github/scripts/ci/classify-pr-profile.test.mjs .github/scripts/upsert-bot-comment.test.mjs .github/scripts/ci/main-failure-signature.test.mjs .github/scripts/classify-release-notes.test.mjs .github/scripts/dsw-swe-verified/make-manifest.test.mjs .github/scripts/resolve-sandbox-image.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/serve-ab-diff.test.mjs .github/scripts/qwen-triage-workflow.test.mjs .github/scripts/auto-minimize-spam.test.mjs .github/scripts/ci-runner-routing.test.mjs' + HELPER_TESTS: '.github/scripts/pr-safety-precheck.test.mjs .github/scripts/cap-release-notes.test.mjs .github/scripts/ci/classify-profile.test.mjs .github/scripts/ci/classify-pr-profile.test.mjs .github/scripts/upsert-bot-comment.test.mjs .github/scripts/ci/main-failure-signature.test.mjs .github/scripts/classify-release-notes.test.mjs .github/scripts/dsw-swe-verified/make-manifest.test.mjs .github/scripts/resolve-sandbox-image.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/serve-ab-diff.test.mjs .github/scripts/qwen-triage-workflow.test.mjs .github/scripts/assign-issue-owner.test.mjs .github/scripts/auto-minimize-spam.test.mjs .github/scripts/ci-runner-routing.test.mjs' jobs: classify_pr: diff --git a/docs/design/2026-08-07-issue-auto-assignment.md b/docs/design/2026-08-07-issue-auto-assignment.md new file mode 100644 index 0000000000..16fb9dd329 --- /dev/null +++ b/docs/design/2026-08-07-issue-auto-assignment.md @@ -0,0 +1,119 @@ +# Label-Driven Issue Auto-Assignment + +## Problem + +Issues that need a maintainer sit unassigned until someone notices them. We want +them routed automatically to a person who has permission to act on them. + +The obvious approach — let the triage agent pick an assignee — puts an untrusted +issue body in the path of a write token: whoever writes the issue text can try to +steer who gets assigned. Defending that requires transporting a model decision +across a trust boundary (schema, validation, staleness checks, a separate apply +job). All of that machinery exists only because the model chose the login. + +## Design + +The model does not choose. Assignment is a pure function of the issue's labels. + +Triage already applies labels from the repository's existing taxonomy. A separate +workflow triggers on `issues: labeled` and `issues: unlabeled`, reads the label +names, and maps them to owners via `.github/issue-owners.json`. The assignment +script never reads issue title, body, or comments, so issue text cannot influence +the outcome — not because it is filtered, but because it is never in scope. + +``` +issues: labeled/unlabeled ──► assign-issue-owner.mjs ──► labels → area → owner + (no model, no plan, no schema) +``` + +`.github/issue-owners.json` holds three things: + +- `requireLabels` — every one must be present. Ships as `["need-discussion"]`, a + deliberately conservative rollout. Widen or empty it to assign more issues. +- `skipLabels` — any one blocks assignment. `welcome-pr`, `feature/need-help`, + `good first issue`, `help wanted` keep community-facing issues open to the + community; `autofix/approved` and `autofix/in-progress` keep autofix-owned + issues clear of a competing human assignment (autofix selects candidates + with `no:assignee`). +- `areas` — ordered label→owners entries. First match wins. + +The initial `core` owners were seeded from `.github/CODEOWNERS`, but the map is +not constrained to it — see "Adding owners" below. The security boundary is the +live permission check: before each write the script calls +`GET /repos/{repo}/collaborators/{login}/permission` and keeps only +`admin`/`maintain`/`write`. Editing the map therefore cannot assign someone who +does not already have push access. + +Among eligible owners it picks the one with the fewest open assigned issues, +rotating by issue number to break ties so a set of equally loaded owners spreads +round-robin rather than always landing on the first entry. + +## Adding owners + +Edit `.github/issue-owners.json` and open a PR. No code change is required, and +nothing else in the repository needs updating. + +An owner does **not** need a `.github/CODEOWNERS` entry. CODEOWNERS answers "who +owns this code path", which is narrower than "who may be assigned an issue in +this area" — the repository has roughly 44 collaborators with push access and 4 +CODEOWNERS path rules, so requiring one would reject legitimate additions. The +permission check enforces the property that actually matters. + +To add a new area, append an entry with its labels and owners. Areas are matched +in file order and the first match wins, so put specific areas above general ones. +Area labels must already exist in the repository; the assignment workflow never +creates labels. `.github/CODEOWNERS` currently maps `packages/cua-driver/` and +`packages/mobile-mcp/` to an owner, but no corresponding issue label exists, so +those areas cannot be added until the labels are created. + +Validation on the map is deliberately thin — shape, login syntax, duplicate +owners, duplicate area names. Everything else is caught at runtime: a login that +is not a collaborator, or has lost push access, fails the permission lookup and +is skipped with a `::warning::` in the step summary rather than failing the run. +Removing an owner is likewise just a file edit; in-flight assignments are not +revisited. + +## Why no plan/apply split + +The trigger _is_ the state change, which removes the reasons a split would exist: + +| Concern | Resolution | +| ------------------------------------- | ---------------------------------------------------------------------------------------- | +| Stale decision overwrites newer state | The script reads live state at write time; a later label change re-fires the workflow | +| Model output reaching a write token | No model runs in this workflow | +| Model selecting an arbitrary login | The model emits labels only; logins come from a reviewed file and are permission-checked | +| Model selecting an arbitrary label | Labels are matched against the map, not applied by this workflow | + +## Trigger dependency + +`issues: labeled` and `issues: unlabeled` only fire for label writes made with +a PAT — writes made with the default `GITHUB_TOKEN` do not trigger downstream +workflow runs. The triage agent labels with `QWEN_CODE_BOT_TOKEN`/`CI_BOT_PAT`, +so the chain holds. If triage ever falls back to `GITHUB_TOKEN` for labelling, +this workflow must be rehung on `workflow_run` of the triage workflow. This is +noted in the workflow header where it would be needed. + +Label events fire once per label change, so one triage run that applies several +labels queues several runs. Each is idempotent — every run after the first sees +an assignee and stops — and a per-issue concurrency group serialises them. +Because `unlabeled` is also a trigger, removing a skip label from an already +labelled issue can make it newly eligible for assignment. + +## Failure behavior + +A malformed owner map fails the run before any GitHub call. An owner whose +permission lookup fails (renamed, deleted, access revoked) is warned about and +skipped rather than failing the run. If no owner of a matched area has push +access, the run warns and makes no change. Closed, already-assigned, and +non-matching issues are no-ops. + +## Validation + +Unit tests cover map validation, the skip policy, area matching, and owner +selection. A stubbed-`gh` integration test asserts that dry-run performs no +mutation, that push access is verified before assigning, and that the least +loaded eligible owner is the one assigned. Workflow tests guard the repository +guard, the job-scoped `issues: write`, the step-scoped token, +`persist-credentials: false`, and the absence of any model invocation. + +Roll out with the `workflow_dispatch` entry point, which defaults to dry-run.