Skip to content
Closed
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
1 change: 1 addition & 0 deletions .github/PR_AUTOMATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ Risk labels express required maintainer scrutiny:
| `risk/unknown` | No valid classification was available. |

`cargo-semver-checks` incompatibility forces `risk/high`.
An exact base/head diff containing only nonempty `.github/**` paths records Cargo API compatibility as `not-applicable` without installing or running `cargo-semver-checks`.
A model-suspected breaking change promotes `risk/low` to `risk/medium`.
Path rules can add `scope/core`, `scope/web`, `scope/ffi`, and `scope/tooling`.
The classifier controls `scope/cross-cutting`, `kind/technical-debt`, and documentation-only classification.
Expand Down
55 changes: 55 additions & 0 deletions .github/pr-automation/automation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const {

const SHA = "a".repeat(40);
const OTHER_SHA = "b".repeat(40);
const BASE_SHA = "c".repeat(40);

function desiredLabels(state, owned) {
return state.labelSets.find((set) => set.owned.includes(owned)).desired;
Expand Down Expand Up @@ -1382,6 +1383,18 @@ test("evidence caps are trusted, bounded, and fail closed with guidance", () =>
assert.match(markerBody(reviewFailure.comments[0]), /runtime maximum/);
});

test("semver skips only a nonempty immutable GitHub-only diff", () => {
const semver = workflowJob(readWorkflow(), "semver");
assert.match(semver, /git diff --name-only --no-renames -z --no-ext-diff --no-textconv/);
assert.match(semver, /while IFS= read -r -d '' changed_path/);
assert.match(semver, /"\$changed_path" != \.github\/\*/);
assert.match(semver, /test "\$count" -gt 0 && test "\$github_only" = true/);
assert.match(semver, /status\\":\\"not-applicable/);
assert.match(semver, /base_sha\\":\\"\$BASE_SHA/);
assert.match(semver, /if: steps\.fetch\.outputs\.not-applicable != 'true'/);
assert.match(semver, /compatibility: \$\{\{ steps\.fetch\.outputs\.compatibility \|\| steps\.semver\.outputs\.compatibility \}\}/);
});

test("every deterministic label is declared and the repository rules classify tooling changes", () => {
const githubDirectory = path.join(__dirname, "..");
const rules = parseLabelerRules(fs.readFileSync(path.join(githubDirectory, "labeler.yml"), "utf8"));
Expand Down Expand Up @@ -2375,6 +2388,24 @@ test("deterministic semver outranks the model and a model-only break cannot stay
assert.deepEqual(risk({ risk: "low", breaking_change_suspected: true }, "not-suspected"), ["risk/medium"]);
assert.deepEqual(risk({ risk: "high", breaking_change_suspected: true }, "not-suspected"), ["risk/high"]);
assert.deepEqual(risk({ risk: "low" }, "not-suspected"), ["risk/low"]);
const notApplicable = resolveClassificationState({
expectedSha: SHA, expectedBaseSha: BASE_SHA, labels: [], deterministic, classifier: classifier(),
semver: { head_sha: SHA, base_sha: BASE_SHA, status: "not-applicable" },
});
assert.equal(notApplicable.failed, undefined);
assert.equal(notApplicable.expectedBaseSha, BASE_SHA);
for (const semver of [
{ head_sha: SHA, status: "not-applicable" },
{ head_sha: SHA, base_sha: OTHER_SHA, status: "not-applicable" },
{ head_sha: OTHER_SHA, base_sha: BASE_SHA, status: "not-applicable" },
]) {
const invalid = resolveClassificationState({
expectedSha: SHA, expectedBaseSha: BASE_SHA, labels: [], deterministic,
classifier: classifier(), semver,
});
assert.equal(invalid.failed, true);
assert.match(invalid.check.summary, /public API compatibility unavailable/);
}
const unavailable = resolveClassificationState({
expectedSha: SHA, labels: ["breaking-change"], deterministic, classifier: classifier(),
semver: { head_sha: SHA, status: "unavailable" },
Expand Down Expand Up @@ -3675,6 +3706,30 @@ test("writer does not dispatch a completed classification after the head changes
assert.equal(dispatches, 0);
});

test("writer rejects a GitHub-only classification after the PR base changes", async () => {
let mutations = 0;
const github = { rest: {
pulls: { get: async () => ({
data: { state: "open", head: { sha: SHA }, base: { sha: OTHER_SHA } },
}) },
issues: {
addLabels: async () => { mutations += 1; },
removeLabel: async () => { mutations += 1; },
},
checks: { create: async () => { mutations += 1; } },
} };

await assert.rejects(writeState({
github, owner: "Devolutions", repo: "IronRDP", prNumber: 1,
botLogin: "github-actions[bot]",
state: {
ok: true, mode: "classification", expectedSha: SHA, expectedBaseSha: BASE_SHA,
labelSets: [], addLabels: [], comments: [], removeCommentMarkers: [],
},
}), StaleHeadError);
assert.equal(mutations, 0);
});

test("writer deduplicates one forced review invocation but publishes a later one", async () => {
const existingMarker = `<!-- ironrdp-pr-automation:review:${SHA}:force:1234 -->`;
const publish = async (marker) => {
Expand Down
7 changes: 5 additions & 2 deletions .github/pr-automation/current-head.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ async function readPullRequest({ github, owner, repo, pullNumber }) {
}
}

async function assertCurrentHead({ github, owner, repo, pullNumber, expectedHeadSha }) {
async function assertCurrentHead({
github, owner, repo, pullNumber, expectedHeadSha, expectedBaseSha,
}) {
const data = await readPullRequest({ github, owner, repo, pullNumber });
if (data.state !== "open" || data.head?.sha !== expectedHeadSha) {
if (data.state !== "open" || data.head?.sha !== expectedHeadSha ||
(expectedBaseSha !== undefined && data.base?.sha !== expectedBaseSha)) {
Comment on lines +25 to +29
throw new StaleHeadError();
}
}
Expand Down
10 changes: 8 additions & 2 deletions .github/pr-automation/resolve-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,19 @@ function failedClassification(expectedSha, deterministic, reason, rateLimit, sem
}

function resolveClassificationState({
expectedSha, labels, deterministic, classifier, classificationGate,
expectedSha, expectedBaseSha, labels, deterministic, classifier, classificationGate,
classifierReason, changedPaths, overlapCandidates, prNumber, semver, rateLimit, force,
} = {}) {
const existing = labelsOf(labels);
const forced = force === true;
const failureRateLimit = forced ? undefined : rateLimit;
if (typeof expectedSha !== "string") return { ok: false, reason: "missing expected SHA" };
const semverStatus = boundStatus(semver, expectedSha, ["suspected", "not-suspected"]);
const boundSemverStatus = boundStatus(
semver, expectedSha, ["suspected", "not-suspected", "not-applicable"]);
const semverStatus = boundSemverStatus === "not-applicable" &&
typeof expectedBaseSha === "string" && semver?.base_sha === expectedBaseSha
? boundSemverStatus
: boundSemverStatus === "not-applicable" ? "unavailable" : boundSemverStatus;
if (!forced && rateLimit && rateLimit.status !== "allowed") {
return failedClassification(
expectedSha, deterministic, "fork LLM quota unavailable", failureRateLimit, semverStatus, forced);
Expand Down Expand Up @@ -205,6 +210,7 @@ function resolveClassificationState({
];
return {
ok: true, mode: "classification", expectedSha, forced, labelSets, addLabels, comments, auditComments,
...(semverStatus === "not-applicable" ? { expectedBaseSha } : {}),
dispatchReview: !forced,
removeCommentMarkers: [
// Remove notices that contradict the current classification.
Expand Down
42 changes: 27 additions & 15 deletions .github/pr-automation/write-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,17 @@ function markerBody(comment, owner, repo) {
}

async function upsertMarkedComment(
github, owner, repo, prNumber, expectedSha, botLogin, comment, canMutate,
github, owner, repo, prNumber, expectedSha, botLogin, comment, canMutate, expectedBaseSha,
) {
if (!botLogin || typeof botLogin !== "string") throw new Error("botLogin is required for comment ownership");
const body = markerBody(comment, owner, repo);
const existing = (await comments(github, owner, repo, prNumber)).find((item) =>
item.user?.login === botLogin && typeof item.body === "string" && item.body.includes(comment.marker));
if (existing?.body === body) return false;
await issueLabels(github, owner, repo, prNumber);
await assertCurrentHead({ github, owner, repo, pullNumber: prNumber, expectedHeadSha: expectedSha });
await assertCurrentHead({
github, owner, repo, pullNumber: prNumber, expectedHeadSha: expectedSha, expectedBaseSha,
});
if (canMutate && !await canMutate()) return false;
if (existing) {
await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body });
Expand All @@ -69,13 +71,15 @@ async function upsertMarkedComment(
}

async function deleteMarkedComment(
github, owner, repo, prNumber, expectedSha, botLogin, marker, canMutate,
github, owner, repo, prNumber, expectedSha, botLogin, marker, canMutate, expectedBaseSha,
) {
if (!botLogin || typeof botLogin !== "string") throw new Error("botLogin is required for comment ownership");
const existing = (await comments(github, owner, repo, prNumber)).find((item) =>
item.user?.login === botLogin && typeof item.body === "string" && item.body.includes(marker));
if (!existing) return false;
await assertCurrentHead({ github, owner, repo, pullNumber: prNumber, expectedHeadSha: expectedSha });
await assertCurrentHead({
github, owner, repo, pullNumber: prNumber, expectedHeadSha: expectedSha, expectedBaseSha,
});
if (canMutate && !await canMutate()) return false;
await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id });
return true;
Expand Down Expand Up @@ -184,7 +188,7 @@ function newestRun(runs) {
}

async function ensureCanonicalCheck({
github, owner, repo, prNumber, expectedSha, check, state,
github, owner, repo, prNumber, expectedSha, expectedBaseSha, check, state,
title, summary, canMutate, beforeWrite,
}) {
const conclusion = check.conclusion ?? "success";
Expand All @@ -203,7 +207,7 @@ async function ensureCanonicalCheck({
existing.output?.summary === summary)) return false;
if (beforeWrite) await beforeWrite();
await assertCurrentHead({
github, owner, repo, pullNumber: prNumber, expectedHeadSha: expectedSha,
github, owner, repo, pullNumber: prNumber, expectedHeadSha: expectedSha, expectedBaseSha,
});
if (canMutate && !await canMutate()) return false;
const payload = {
Expand All @@ -223,10 +227,10 @@ async function ensureCanonicalCheck({
}

async function ensureClassificationCheck(
github, owner, repo, prNumber, expectedSha, check, state, canMutate,
github, owner, repo, prNumber, expectedSha, expectedBaseSha, check, state, canMutate,
) {
return ensureCanonicalCheck({
github, owner, repo, prNumber, expectedSha, check, state, canMutate,
github, owner, repo, prNumber, expectedSha, expectedBaseSha, check, state, canMutate,
title: check.title,
summary: `${check.summary}\n\n${encodeCheckState(check.machineState)}`,
});
Expand All @@ -246,9 +250,11 @@ async function ensureReviewCheck(
});
}

async function dispatchClassificationComplete(github, owner, repo, prNumber, expectedSha) {
async function dispatchClassificationComplete(
github, owner, repo, prNumber, expectedSha, expectedBaseSha,
) {
await assertCurrentHead({
github, owner, repo, pullNumber: prNumber, expectedHeadSha: expectedSha,
github, owner, repo, pullNumber: prNumber, expectedHeadSha: expectedSha, expectedBaseSha,
});
await github.rest.repos.createDispatchEvent({
owner, repo, event_type: "pr-automation-classified",
Expand All @@ -273,6 +279,7 @@ async function applyLabels(github, owner, repo, prNumber, state, currentLabels,
if (additions.length === 0 && removals.length === 0) return false;
await assertCurrentHead({
github, owner, repo, pullNumber: prNumber, expectedHeadSha: state.expectedSha,
expectedBaseSha: state.expectedBaseSha,
});
// The actor labels are mutually exclusive. Remove a stale actor before adding its replacement so
// a failed add leaves no actor selected rather than two conflicting actors. Other labels retain
Expand Down Expand Up @@ -332,6 +339,7 @@ async function writeState({
}
await assertCurrentHead({
github, owner, repo, pullNumber: prNumber, expectedHeadSha: state.expectedSha,
expectedBaseSha: state.expectedBaseSha,
});
const check = state.check;
let existingCheck = null;
Expand Down Expand Up @@ -406,17 +414,21 @@ async function writeState({
if (!await canMutate()) return { ok: true, superseded: true };
await applyLabels(github, owner, repo, prNumber, state, undefined, canMutate);
for (const comment of state.comments || []) await upsertMarkedComment(
github, owner, repo, prNumber, state.expectedSha, botLogin, comment, canMutate);
github, owner, repo, prNumber, state.expectedSha, botLogin, comment, canMutate,
state.expectedBaseSha);
for (const comment of state.auditComments || []) await upsertMarkedComment(
github, owner, repo, prNumber, state.expectedSha, botLogin, comment, canMutate);
github, owner, repo, prNumber, state.expectedSha, botLogin, comment, canMutate,
state.expectedBaseSha);
for (const marker of new Set(state.removeCommentMarkers || [])) {
await deleteMarkedComment(
github, owner, repo, prNumber, state.expectedSha, botLogin, marker, canMutate);
github, owner, repo, prNumber, state.expectedSha, botLogin, marker, canMutate,
state.expectedBaseSha);
}
if (state.check) {
if (!await canMutate()) return { ok: true, superseded: true };
const changed = await ensureClassificationCheck(
github, owner, repo, prNumber, state.expectedSha, state.check, state, canMutate);
github, owner, repo, prNumber, state.expectedSha, state.expectedBaseSha,
state.check, state, canMutate);
const shouldDispatch = ["Classification complete", "Automation stopped"].includes(
state.check.title,
) && state.dispatchReview !== false && (state.lease
Expand All @@ -426,7 +438,7 @@ async function writeState({
(changed || reviewRequested || state.check.title === "Automation stopped"));
if (shouldDispatch) {
await dispatchClassificationComplete(
github, owner, repo, prNumber, state.expectedSha,
github, owner, repo, prNumber, state.expectedSha, state.expectedBaseSha,
);
}
}
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/pr-automation.intent.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ Block model review for likely non-legitimate changes and hand the green exact he
Label a suspected overlap with another pull request at confidence 0.85 or greater as `triage/overlap`, and keep it advisory: it never blocks review and never asks for maintainer handoff on its own.
Unavailable or invalid exact-head classification applies only `automation-failed`.
Risk and protocol relevance select reviewers but do not suppress review.
Treat Cargo API compatibility as `not-applicable` only when the semver job's immutable exact base/head diff contains a nonempty `.github/**`-only path set.

Fork pull requests share a quota of 50 per UTC day.
Exclude `OWNER` and `MEMBER` pull requests from enforcement and counting, and preserve the same-repository exemption.
Expand Down
29 changes: 26 additions & 3 deletions .github/workflows/pr-automation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ jobs:
runs-on: ubuntu-latest
permissions: {}
outputs:
compatibility: ${{ steps.semver.outputs.compatibility }}
compatibility: ${{ steps.fetch.outputs.compatibility || steps.semver.outputs.compatibility }}
env:
CARGO_SEMVER_CHECKS_VERSION: "0.49.0"
CARGO_SEMVER_CHECKS_SHA256: "72f6834d75d28a66e02c9fd6a230ce901bb30eee6067b85867a97445df040e4a"
Expand All @@ -358,8 +358,28 @@ jobs:
git fetch --no-tags origin "+$HEAD_SHA:refs/remotes/origin/pull-request-head"
git fetch --no-tags origin "+$BASE_SHA:refs/remotes/origin/pull-request-base"
git checkout --detach origin/pull-request-head
paths="$SEMVER_STATE_DIR/changed-paths"
install -d -m 700 "$SEMVER_STATE_DIR"
git diff --name-only --no-renames -z --no-ext-diff --no-textconv \
origin/pull-request-base...origin/pull-request-head > "$paths"
count=0
github_only=true
while IFS= read -r -d '' changed_path; do
count=$((count + 1))
if [[ "$changed_path" != .github/* ]]; then
github_only=false
fi
done < "$paths"
if test "$count" -gt 0 && test "$github_only" = true; then
echo "compatibility={\"head_sha\":\"$HEAD_SHA\",\"base_sha\":\"$BASE_SHA\",\"status\":\"not-applicable\"}" >> "$GITHUB_OUTPUT"
echo "not-applicable=true" >> "$GITHUB_OUTPUT"
echo "::notice title=PR automation semver decision::head_sha=$HEAD_SHA base_sha=$BASE_SHA status=not-applicable"
else
echo "not-applicable=false" >> "$GITHUB_OUTPUT"
fi

- id: install
if: steps.fetch.outputs.not-applicable != 'true'
shell: bash
run: |
set -euo pipefail
Expand All @@ -375,6 +395,7 @@ jobs:
sudo chmod 700 "$CARGO_SEMVER_CHECKS_DIR/cargo-semver-checks"

- id: semver
if: steps.fetch.outputs.not-applicable != 'true'
shell: bash
env:
HEAD_SHA: ${{ needs.resolve-pr.outputs.head-sha }}
Expand Down Expand Up @@ -415,8 +436,8 @@ jobs:
100) status=suspected ;;
*) exit "$status" ;;
esac
echo "compatibility={\"head_sha\":\"$HEAD_SHA\",\"status\":\"$status\"}" >> "$GITHUB_OUTPUT"
echo "::notice title=PR automation semver decision::head_sha=$HEAD_SHA status=$status"
echo "compatibility={\"head_sha\":\"$HEAD_SHA\",\"base_sha\":\"$BASE_SHA\",\"status\":\"$status\"}" >> "$GITHUB_OUTPUT"
echo "::notice title=PR automation semver decision::head_sha=$HEAD_SHA base_sha=$BASE_SHA status=$status"

classifier:
name: Classify pull request
Expand Down Expand Up @@ -547,6 +568,7 @@ jobs:
uses: actions/github-script@v8
env:
HEAD_SHA: ${{ needs.resolve-pr.outputs.head-sha }}
BASE_SHA: ${{ needs.resolve-pr.outputs.base-sha }}
LABELS: ${{ needs.resolve-pr.outputs.labels }}
PR_NUMBER: ${{ needs.resolve-pr.outputs.pr-number }}
DETERMINISTIC: ${{ needs.deterministic-analysis.outputs.analysis }}
Expand All @@ -566,6 +588,7 @@ jobs:
const parse = (value, fallback) => { try { return JSON.parse(value || ""); } catch { return fallback; } };
const state = resolveClassificationState({
expectedSha: process.env.HEAD_SHA,
expectedBaseSha: process.env.BASE_SHA,
labels: parse(process.env.LABELS, []),
prNumber: Number(process.env.PR_NUMBER),
deterministic: parse(process.env.DETERMINISTIC, {}),
Expand Down
Loading