mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
* fix(triage): assign one accountable maintainer when deferring a PR The defer path resolves the maintainer deterministically from the existing owner map (same area/labels/load/rotation logic as issue assignment) and assigns the PR to them, instead of an ambiguous $QWEN_MAINTAINER_HANDLE / most-recent-reviewer chain. Replaces the reviewer-request routing prototype entirely. * feat(triage): assign one area owner when a PR opens * ci: update workflow size baseline * feat(triage): route PRs to the module owner, with the area pool as fallback * test(triage): pin the assign-pr-owner bootstrap guard shape and ordering * test(triage): pin the assign-pr-owner sparse-checkout membership * fix(triage): reject never-matching paths entries in the owner map * test(triage): pin the re-throw of non-permission assign failures * fix(triage): reject empty paths lists in the owner map * test(triage): pin that a label-only area never path-matches * test(triage): pin the assign-pr-owner PR_NUMBER and DRY_RUN wiring * fix(triage): resolve the defer fallback to the newest human reviewer * docs(triage): make the defer resolver env chain and jq filter robust The heredoc read process.env.REPO, but no triage lane exports REPO — the agent step exports REPOSITORY and Actions always provides GITHUB_REPOSITORY, so repo resolved to undefined, the heredoc threw, 2>/dev/null swallowed the evidence, and the owner-map rung silently never fired. Resolve the repo as REPO -> REPOSITORY -> GITHUB_REPOSITORY and the number as PR_NUMBER -> ISSUE_NUMBER, and pass the session's shell variables through on the node invocation since an unexported variable never reaches the child. The latestReviews fallback jq filter also aborted outright when any review had a null author (a deleted account): piping null into endswith() errors during array construction, discarding every reviewer. Select non-null authors before the bot-suffix filter. * test(triage): pin the privileged trigger and job-level env absence The suite never asserted triggers.pull_request_target and its types or the concurrency shape, although the sibling assign-issue-owner.test.mjs pins both: rewriting the trigger to pull_request kept the suite green while the write token becomes read-only on fork PRs and the permission-tolerance catch silently disables routing for every one of them. Pin the trigger types, the concurrency group, and cancel-in-progress. Also pin the absence of job-level env, mirroring the sibling suite: a GH_TOKEN at jobs.assign.env would leak the pull-requests:write token into every step, including checkout, while every existing assertion stayed green. Verified both pins catch their mutants. * fix(triage): re-check PR coverage immediately before assigning The write ran on the snapshot taken at the start of the run, while up to ~30 sequential API calls (permission and open-issue load per candidate) sit between that snapshot and `gh pr edit --add-assignee`. A mapped owner landing on the PR in that window — a human assigning themselves, or a concurrent workflow run on a synchronize burst — got a second assignee stacked on top, the exact failure the coverage check exists to prevent. Re-fetch state, assignees, and latest reviews immediately before the write and re-run both skipPrReason and alreadyCovered, mirroring the pre-write re-fetch assign-issue-owner.mjs already performs. The stub now serves a distinct PR state to the second `pr view` so both race outcomes (covered mid-run, closed mid-run) are pinned; removing the re-fetch makes both new tests fail. Co-authored-by: Qwen-Coder <[email protected]> * docs(triage): reuse the exported open-issue counter in the defer resolver The heredoc asserted "Same load metric as issue assignment" while re-implementing the metric inline (gh issue list ... --limit 100 --json number --jq length). assign-issue-owner.mjs already exports that exact counter as openIssueCount; import it instead so the defer resolver and issue assignment cannot drift when the metric changes. Smoke-ran the edited heredoc from the repo root against a stubbed gh: it still resolves an eligible owner and drives the counter once per candidate. Co-authored-by: Qwen-Coder <[email protected]> * test(triage): pin the sparse checkout of the statically imported sibling The suite pinned sparse-checkout membership for issue-owners.json and assign-pr-owner.mjs, but not for assign-issue-owner.mjs, which the entry script statically imports (loadPolicy, openIssueCount, pickOwner). The bootstrap guard only checks for assign-pr-owner.mjs, so dropping the sibling entry would pass the guard and then fail node with a missing module. Dropping the YAML line makes the new pin fail; restoring it goes green. Co-authored-by: Qwen-Coder <[email protected]> * test(triage): pin the denied and failing push-access paths No test exercised the push-access check beyond the stub answering `write` for every collaborator lookup: neither a denied owner nor a failing lookup was covered, and the terminal "no eligible owner" exit was unreachable by any test. A regression that bypassed canWrite entirely kept the whole suite green. Extend the gh stub with a per-login denial and a global permission mode ('read' answers a non-write permission, 'error' fails the lookup), and pin three outcomes: a denied module owner falls back to the coarser area without ever being assigned, and when no owner passes the check the run ends in the terminal skip with a clean exit for both the denied route and the canWrite catch branch. Making canWrite always-true fails both new tests. Co-authored-by: Qwen-Coder <[email protected]> * fix(triage): decode changed filenames as base64 before area routing Changed filenames on a fork PR are attacker-controlled, and git accepts newlines in path components. changedFiles() re-split the files endpoint's rendered jq text on newlines, so a file named "x<LF>packages/core/poc" arrived as two entries — "x" and a phantom "packages/core/poc" — letting a fork author forge area routing (longest-prefix ranking) and pick which module owner gets assigned. Pull each filename with `.[].filename | @base64` and decode per line, so one filename always stays one entry. The guard suite's stub now answers the files call with base64-encoded names (keeping the legacy text rendering for the old filter), and a new regression test pins the exact forged probe: the newline-carrying name arrives as one entry that matches no area prefix, while a legit core file in the same PR still routes to core. Co-authored-by: Qwen-Coder <[email protected]> * fix(triage): skip PRs whose author account was deleted `gh pr view --json author` exports `"author": null` for deleted accounts; guard both author dereferences so every later trigger skips gracefully (exit 0) instead of throwing and running the assignment check red. Co-authored-by: Qwen-Coder <[email protected]> * test(triage): route a probe file through every mapped area prefix The routing tests only pinned core-skills and core-goals; a typo in any of the six other module prefixes would silently reroute those PRs to the generic fallback with the suite green. Probe every mapped prefix — fixed literals, so a typo'd prefix fails instead of shifting the probe with it. Co-authored-by: Qwen-Coder <[email protected]> * fix(triage): guard the defer heredoc against a deleted PR author The assign-on-defer heredoc dereferenced pr.author.login unguarded; with "author": null the eligibility filter threw and 2>/dev/null silently bypassed deterministic owner resolution. Null-safe exclusion keeps the resolver running, matching the jq fallback below it. Co-authored-by: Qwen-Coder <[email protected]> * fix(triage): keep PR owner routing current * fix(triage): keep the defer resolver heredoc backtick-free The triage lane's permissions.deny blocks any run_shell_command whose text contains a backtick, so the template literal in canWrite() and the backticked "author": null comment made the whole resolver command EXECUTION_DENIED before approval. Use string concatenation, de-backtick the comment, and pin the invariant with a witness test that turns red if a backtick is reintroduced into the resolver block. Co-authored-by: Qwen-Coder <[email protected]> * fix(triage): treat the agent-assignee GraphQL refusal as a graceful skip gh pr edit --add-assignee goes through replaceActorsForAssignable, which GitHub refuses for GitHub App installation tokens when the PR already carries a coding-agent assignee ("Assigning agents is not supported with GitHub App installation tokens"). The graceful-skip filter did not match that refusal, so main() rethrew and the assignment check ran red on the contributor's own PR. Recognize the refusal and skip like the other token-limit cases; the PR already has an accountable actor. Co-authored-by: Qwen-Coder <[email protected]> * fix(triage): tolerate a transient issue-list failure in the load fan-out The load lookup fans out one gh issue list per eligible owner (up to 15 for the core pool) with no guard; one transient failure — secondary rate limit, 5xx, issues disabled — threw straight up and ran the assignment check red, while the sibling canWrite() lookup deliberately warns and continues. Wrap the lookup in the same pattern: retry once, then warn and degrade to a zero load so the rotation still lands an owner — the load metric is a tie-break heuristic, not a gate on assigning. Co-authored-by: Qwen-Coder <[email protected]> * fix(triage): don't count dismissed reviews as owner coverage A dismissed review is a removed review in GitHub's model. Filtering it out of alreadyCovered() keeps a PR whose only mapped-owner involvement is a dismissed review eligible for assignment, instead of skipping the gate and staying ownerless on every later trigger. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
315 lines
9.9 KiB
JavaScript
315 lines
9.9 KiB
JavaScript
#!/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');
|
|
}
|
|
|
|
// A changed-file prefix for assign-pr-owner.mjs: relative, no `//`, no
|
|
// backslash, no `.`/`..` segments, and ending in `/` so startsWith cannot
|
|
// leak into a sibling directory (packages/core matching packages/coredump/).
|
|
function isPathPrefix(prefix) {
|
|
if (typeof prefix !== 'string' || prefix.length === 0) return false;
|
|
if (prefix.startsWith('/') || prefix.startsWith('./')) return false;
|
|
if (!prefix.endsWith('/')) return false;
|
|
if (prefix.includes('\\') || prefix.includes('//')) return false;
|
|
return !prefix
|
|
.split('/')
|
|
.some((segment) => segment === '.' || segment === '..');
|
|
}
|
|
|
|
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);
|
|
}
|
|
// A never-matching paths entry silently unroutes the area from PR
|
|
// assignment, so reject it here like other malformed config.
|
|
if (area.paths !== undefined && !Array.isArray(area.paths)) {
|
|
throw new Error(
|
|
`${OWNERS_FILE}: area ${area.name} paths must be an array`,
|
|
);
|
|
}
|
|
// An explicitly empty list can never route the area either, yet the
|
|
// entry loop below cannot catch it — reject it like the sibling
|
|
// labels/owners checks do.
|
|
if (Array.isArray(area.paths) && area.paths.length === 0) {
|
|
throw new Error(
|
|
`${OWNERS_FILE}: area ${area.name} paths must not be empty; omit paths for a label-only area`,
|
|
);
|
|
}
|
|
for (const prefix of area.paths ?? []) {
|
|
if (!isPathPrefix(prefix)) {
|
|
throw new Error(
|
|
`${OWNERS_FILE}: invalid paths entry in ${area.name}: ${JSON.stringify(prefix)}`,
|
|
);
|
|
}
|
|
}
|
|
}
|
|
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;
|
|
}
|
|
}
|
|
|
|
export 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);
|
|
}
|
|
}
|