mirror of
https://github.com/esengine/DeepSeek-Reasonix.git
synced 2026-11-07 10:45:59 -10:00
chore(ci): remove cross-boundary review gate (#8211)
Problem: The independent cross-boundary review status created recurring red PR statuses and forced every qualifying head through another manual approval cycle.\n\nRoot cause: Protected review workflows published a failing commit status until a non-author maintainer approved the exact head.\n\nFix: Remove the evaluator, workflows, tests, PR template guidance, and CI invocation while retaining a negative contract against stale references.\n\nVerification: Full pull-request CI passed on Linux, macOS, and Windows, including lint, race, CodeQL, desktop builds, and WebView2 smoke.
This commit is contained in:
1 parent
0139b48287
commit
0cfda51ca4
7 files changed
+13
-372
No files matched your search
@@ -21,11 +21,6 @@ cancellation, reduced-motion behavior, and failure fallback. Verification must
|
||||
assert that the target API was actually called; a mock that silently skips it
|
||||
does not count.
|
||||
|
||||
For a large PR spanning multiple architectural owners, split independent
|
||||
changes when practical. If it must remain combined, obtain a non-author
|
||||
maintainer approval on the current head so the `independent cross-boundary
|
||||
review` gate can pass.
|
||||
|
||||
## Documentation impact
|
||||
|
||||
Documentation-impact: TODO
|
||||
|
||||
@@ -1,184 +0,0 @@
|
||||
"use strict";
|
||||
|
||||
const STATUS_CONTEXT = "independent cross-boundary review";
|
||||
const ALLOWED_PERMISSIONS = new Set(["admin", "maintain", "write"]);
|
||||
|
||||
function pullNumberFromContext(context) {
|
||||
const direct = context.payload.pull_request?.number;
|
||||
const fromWorkflowRun = context.payload.workflow_run?.pull_requests?.[0]?.number;
|
||||
const fromDispatch = Number(context.payload.inputs?.pull_number ?? 0);
|
||||
return direct ?? fromWorkflowRun ?? (Number.isInteger(fromDispatch) && fromDispatch > 0 ? fromDispatch : undefined);
|
||||
}
|
||||
|
||||
function changeSummary(files) {
|
||||
const groups = new Set();
|
||||
for (const { filename } of files) {
|
||||
if (filename.startsWith("desktop/frontend/")) groups.add("frontend");
|
||||
else if (filename.startsWith("desktop/")) groups.add("desktop-native");
|
||||
else if (/^(?:internal|cmd)\//.test(filename)) groups.add("kernel");
|
||||
else if (/^(?:\.github|scripts|tools)\//.test(filename)) groups.add("control-plane");
|
||||
else if (/^(?:sdk|npm|site|docs|workers)\//.test(filename)) groups.add("distribution");
|
||||
}
|
||||
const changedLines = files.reduce((sum, file) => sum + file.changes, 0);
|
||||
return {
|
||||
changedLines,
|
||||
groups: [...groups],
|
||||
required: files.length >= 25 || changedLines >= 1000 || groups.size >= 3,
|
||||
};
|
||||
}
|
||||
|
||||
function currentHeadApprovalCandidates(reviews, pull) {
|
||||
const latestByReviewer = new Map();
|
||||
for (const review of reviews) {
|
||||
const login = review.user?.login;
|
||||
if (!login || login === pull.user.login || review.user?.type === "Bot") continue;
|
||||
// Comments do not revoke an approval; only explicit review decisions do.
|
||||
if (review.state === "COMMENTED" || review.state === "PENDING") continue;
|
||||
latestByReviewer.set(login, { state: review.state, commitId: review.commit_id });
|
||||
}
|
||||
return [...latestByReviewer.entries()]
|
||||
.filter(([, review]) => review.state === "APPROVED" && review.commitId === pull.head.sha)
|
||||
.map(([login]) => login);
|
||||
}
|
||||
|
||||
async function publishStatus({ github, context, sha, state, description }) {
|
||||
await github.rest.repos.createCommitStatus({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
sha,
|
||||
state,
|
||||
context: STATUS_CONTEXT,
|
||||
description: description.slice(0, 140),
|
||||
target_url: `${context.serverUrl ?? "https://github.com"}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`,
|
||||
});
|
||||
}
|
||||
|
||||
async function run({ github, context, core }) {
|
||||
let pullNumber = pullNumberFromContext(context);
|
||||
if (!pullNumber && context.payload.workflow_run?.head_sha) {
|
||||
const associated = await github.rest.repos.listPullRequestsAssociatedWithCommit({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
commit_sha: context.payload.workflow_run.head_sha,
|
||||
});
|
||||
const pull = associated.data.find((candidate) =>
|
||||
candidate.state === "open" && candidate.base?.repo?.full_name === `${context.repo.owner}/${context.repo.repo}`,
|
||||
);
|
||||
pullNumber = pull?.number;
|
||||
}
|
||||
if (!pullNumber) {
|
||||
core.info("No pull request is associated with this event; nothing to reevaluate.");
|
||||
return;
|
||||
}
|
||||
|
||||
let pull;
|
||||
try {
|
||||
({ data: pull } = await github.rest.pulls.get({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: pullNumber,
|
||||
}));
|
||||
if (pull.state !== "open") {
|
||||
core.info(`Pull request #${pull.number} is not open; nothing to reevaluate.`);
|
||||
return;
|
||||
}
|
||||
if (pull.draft) {
|
||||
await publishStatus({
|
||||
github,
|
||||
context,
|
||||
sha: pull.head.sha,
|
||||
state: "success",
|
||||
description: "Draft PR; independent review is deferred",
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const files = await github.paginate(github.rest.pulls.listFiles, {
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: pull.number,
|
||||
per_page: 100,
|
||||
});
|
||||
const summary = changeSummary(files);
|
||||
if (!summary.required) {
|
||||
await publishStatus({
|
||||
github,
|
||||
context,
|
||||
sha: pull.head.sha,
|
||||
state: "success",
|
||||
description: [
|
||||
`${files.length} files`,
|
||||
`${summary.changedLines} lines`,
|
||||
`${summary.groups.length} owner groups; review gate not required`,
|
||||
].join(", "),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const reviews = await github.paginate(github.rest.pulls.listReviews, {
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: pull.number,
|
||||
per_page: 100,
|
||||
});
|
||||
const approvers = [];
|
||||
for (const login of currentHeadApprovalCandidates(reviews, pull)) {
|
||||
try {
|
||||
const permission = await github.rest.repos.getCollaboratorPermissionLevel({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
username: login,
|
||||
});
|
||||
if (ALLOWED_PERMISSIONS.has(permission.data.permission)) approvers.push(login);
|
||||
} catch (error) {
|
||||
core.info(`Ignoring approval from ${login}: repository permission could not be verified (${error.status ?? "unknown"}).`);
|
||||
}
|
||||
}
|
||||
|
||||
if (approvers.length === 0) {
|
||||
await publishStatus({
|
||||
github,
|
||||
context,
|
||||
sha: pull.head.sha,
|
||||
state: "failure",
|
||||
description: "Large cross-boundary change needs current-head approval from a non-author maintainer",
|
||||
});
|
||||
const areas = summary.groups.join(", ") || "one uncategorized area";
|
||||
core.warning(
|
||||
`PR changes ${files.length} files and ${summary.changedLines} lines across ${areas}; ` +
|
||||
"split it or obtain current-head approval from a non-author maintainer.",
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
await publishStatus({
|
||||
github,
|
||||
context,
|
||||
sha: pull.head.sha,
|
||||
state: "success",
|
||||
description: `Current-head independent review approved by ${approvers.join(", ")}`,
|
||||
});
|
||||
core.info(`Independent cross-boundary review approved by ${approvers.join(", ")}.`);
|
||||
} catch (error) {
|
||||
if (pull?.head?.sha) {
|
||||
try {
|
||||
await publishStatus({
|
||||
github,
|
||||
context,
|
||||
sha: pull.head.sha,
|
||||
state: "error",
|
||||
description: "Independent review policy could not be evaluated",
|
||||
});
|
||||
} catch (statusError) {
|
||||
core.info(`Could not publish evaluation error status: ${statusError.message}`);
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = run;
|
||||
module.exports.changeSummary = changeSummary;
|
||||
module.exports.currentHeadApprovalCandidates = currentHeadApprovalCandidates;
|
||||
module.exports.pullNumberFromContext = pullNumberFromContext;
|
||||
module.exports.STATUS_CONTEXT = STATUS_CONTEXT;
|
||||
@@ -1,106 +0,0 @@
|
||||
"use strict";
|
||||
|
||||
const test = require("node:test");
|
||||
const assert = require("node:assert/strict");
|
||||
const gate = require("./cross-boundary-review.cjs");
|
||||
const {
|
||||
STATUS_CONTEXT,
|
||||
changeSummary,
|
||||
currentHeadApprovalCandidates,
|
||||
pullNumberFromContext,
|
||||
} = require("./cross-boundary-review.cjs");
|
||||
|
||||
function gateHarness(reviews) {
|
||||
const listFiles = () => {};
|
||||
const listReviews = () => {};
|
||||
const statuses = [];
|
||||
const github = {
|
||||
rest: {
|
||||
pulls: {
|
||||
get: async () => ({ data: { number: 7, state: "open", draft: false, user: { login: "author" }, head: { sha: "current" } } }),
|
||||
listFiles,
|
||||
listReviews,
|
||||
},
|
||||
repos: {
|
||||
createCommitStatus: async (status) => statuses.push(status),
|
||||
getCollaboratorPermissionLevel: async () => ({ data: { permission: "write" } }),
|
||||
},
|
||||
},
|
||||
paginate: async (endpoint) => endpoint === listFiles
|
||||
? [
|
||||
{ filename: "desktop/frontend/src/App.tsx", changes: 10 },
|
||||
{ filename: "desktop/main.go", changes: 10 },
|
||||
{ filename: ".github/workflows/ci.yml", changes: 10 },
|
||||
]
|
||||
: reviews,
|
||||
};
|
||||
const context = {
|
||||
payload: { pull_request: { number: 7 } },
|
||||
repo: { owner: "example", repo: "reasonix" },
|
||||
runId: 99,
|
||||
serverUrl: "https://github.example",
|
||||
};
|
||||
const core = { info() {}, warning() {} };
|
||||
return { github, context, core, statuses };
|
||||
}
|
||||
|
||||
test("large multi-owner changes require independent review", () => {
|
||||
const files = [
|
||||
{ filename: "desktop/frontend/src/App.tsx", changes: 10 },
|
||||
{ filename: "desktop/main.go", changes: 10 },
|
||||
{ filename: ".github/workflows/ci.yml", changes: 10 },
|
||||
];
|
||||
assert.deepEqual(changeSummary(files), {
|
||||
changedLines: 30,
|
||||
groups: ["frontend", "desktop-native", "control-plane"],
|
||||
required: true,
|
||||
});
|
||||
});
|
||||
|
||||
test("approval must target the current pull request head", () => {
|
||||
const pull = { user: { login: "author" }, head: { sha: "current" } };
|
||||
const reviews = [
|
||||
{ user: { login: "stale", type: "User" }, state: "APPROVED", commit_id: "previous" },
|
||||
{ user: { login: "current", type: "User" }, state: "APPROVED", commit_id: "current" },
|
||||
{ user: { login: "author", type: "User" }, state: "APPROVED", commit_id: "current" },
|
||||
{ user: { login: "automation", type: "Bot" }, state: "APPROVED", commit_id: "current" },
|
||||
];
|
||||
assert.deepEqual(currentHeadApprovalCandidates(reviews, pull), ["current"]);
|
||||
});
|
||||
|
||||
test("later explicit review decisions replace approval without comments revoking it", () => {
|
||||
const pull = { user: { login: "author" }, head: { sha: "current" } };
|
||||
const reviews = [
|
||||
{ user: { login: "maintainer", type: "User" }, state: "APPROVED", commit_id: "current" },
|
||||
{ user: { login: "maintainer", type: "User" }, state: "COMMENTED", commit_id: "current" },
|
||||
{ user: { login: "maintainer", type: "User" }, state: "CHANGES_REQUESTED", commit_id: "current" },
|
||||
];
|
||||
assert.deepEqual(currentHeadApprovalCandidates(reviews, pull), []);
|
||||
});
|
||||
|
||||
test("protected events resolve the intended pull request", () => {
|
||||
assert.equal(pullNumberFromContext({ payload: { pull_request: { number: 12 } } }), 12);
|
||||
assert.equal(pullNumberFromContext({ payload: { workflow_run: { pull_requests: [{ number: 34 }] } } }), 34);
|
||||
assert.equal(pullNumberFromContext({ payload: { inputs: { pull_number: "56" } } }), 56);
|
||||
assert.equal(STATUS_CONTEXT, "independent cross-boundary review");
|
||||
});
|
||||
|
||||
test("stale approval publishes a failing current-head status", async () => {
|
||||
const harness = gateHarness([
|
||||
{ user: { login: "maintainer", type: "User" }, state: "APPROVED", commit_id: "previous" },
|
||||
]);
|
||||
await gate(harness);
|
||||
assert.equal(harness.statuses.length, 1);
|
||||
assert.equal(harness.statuses[0].sha, "current");
|
||||
assert.equal(harness.statuses[0].state, "failure");
|
||||
assert.equal(harness.statuses[0].context, STATUS_CONTEXT);
|
||||
});
|
||||
|
||||
test("current-head maintainer approval publishes success", async () => {
|
||||
const harness = gateHarness([
|
||||
{ user: { login: "maintainer", type: "User" }, state: "APPROVED", commit_id: "current" },
|
||||
]);
|
||||
await gate(harness);
|
||||
assert.equal(harness.statuses.length, 1);
|
||||
assert.equal(harness.statuses[0].state, "success");
|
||||
});
|
||||
@@ -578,11 +578,8 @@ jobs:
|
||||
.github/workflows/release-desktop.yml \
|
||||
.github/workflows/release-verify-issues.yml \
|
||||
.github/workflows/docs-impact.yml \
|
||||
.github/workflows/ci.yml \
|
||||
.github/workflows/cross-boundary-review.yml \
|
||||
.github/workflows/cross-boundary-review-signal.yml
|
||||
.github/workflows/ci.yml
|
||||
bash scripts/release-workflows.test.sh
|
||||
node --test .github/scripts/cross-boundary-review.test.cjs
|
||||
node scripts/check-single-release-public-contract.mjs
|
||||
|
||||
# The site/ auth client has security-sensitive redirect-validation logic
|
||||
|
||||
@@ -1,18 +0,0 @@
|
||||
name: Cross-boundary review signal
|
||||
|
||||
# This candidate-context workflow carries no write permission and executes no
|
||||
# repository code. Its completion only asks the protected workflow_run policy
|
||||
# to fetch current reviews and recompute the real status.
|
||||
on:
|
||||
pull_request_review:
|
||||
types: [submitted, edited, dismissed]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
signal:
|
||||
name: signal cross-boundary review reevaluation
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo "Review state changed; protected policy will reevaluate the pull request."
|
||||
@@ -1,42 +0,0 @@
|
||||
name: Cross-boundary review gate
|
||||
|
||||
# Large changes or changes spanning multiple architectural owners are easy to
|
||||
# validate by aggregate CI while missing a small contract migration. Evaluate
|
||||
# them only from the protected default-branch control plane. Review workflows
|
||||
# on the candidate ref may request reevaluation, but they cannot decide or
|
||||
# publish the result.
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [opened, synchronize, reopened, ready_for_review, converted_to_draft]
|
||||
workflow_run:
|
||||
workflows: [Cross-boundary review signal]
|
||||
types: [completed]
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
pull_number:
|
||||
description: Pull request number to reevaluate
|
||||
required: true
|
||||
type: number
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read
|
||||
statuses: write
|
||||
|
||||
jobs:
|
||||
independent-review:
|
||||
name: publish independent cross-boundary review status
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout protected review policy
|
||||
uses: actions/checkout@v7
|
||||
with:
|
||||
ref: ${{ github.workflow_sha }}
|
||||
path: review-control
|
||||
persist-credentials: false
|
||||
|
||||
- uses: actions/github-script@v9
|
||||
with:
|
||||
script: |
|
||||
const gate = require(`${process.env.GITHUB_WORKSPACE}/review-control/.github/scripts/cross-boundary-review.cjs`);
|
||||
await gate({ github, context, core });
|
||||
@@ -117,19 +117,18 @@ grep -Fq 'name: Smoke-test Wails approval in WebView2' "$repo_root/.github/workf
|
||||
grep -Fq '../scripts/test-webview2-approval-smoke.ps1' "$repo_root/.github/workflows/ci.yml"
|
||||
grep -Fq 'wails build -clean -s -skipbindings -nopackage -platform windows/amd64 -webview2 embed' \
|
||||
"$repo_root/.github/workflows/ci.yml"
|
||||
review_gate="$repo_root/.github/workflows/cross-boundary-review.yml"
|
||||
review_signal="$repo_root/.github/workflows/cross-boundary-review-signal.yml"
|
||||
grep -Eq '^ pull_request_target:$' "$review_gate"
|
||||
grep -Eq '^ workflow_run:$' "$review_gate"
|
||||
grep -Eq '^ workflow_dispatch:$' "$review_gate"
|
||||
! grep -Eq '^ pull_request_review:$' "$review_gate"
|
||||
grep -Fq 'ref: ${{ github.workflow_sha }}' "$review_gate"
|
||||
grep -Fq 'persist-credentials: false' "$review_gate"
|
||||
grep -Fq 'review-control/.github/scripts/cross-boundary-review.cjs' "$review_gate"
|
||||
grep -Eq '^ pull_request_review:$' "$review_signal"
|
||||
! grep -Eq 'statuses: write|pull-requests: write|contents: write' "$review_signal"
|
||||
grep -Fq 'review.commit_id' "$repo_root/.github/scripts/cross-boundary-review.cjs"
|
||||
grep -Fq 'pull.head.sha' "$repo_root/.github/scripts/cross-boundary-review.cjs"
|
||||
for retired_review_gate in \
|
||||
"$repo_root/.github/workflows/cross-boundary-review.yml" \
|
||||
"$repo_root/.github/workflows/cross-boundary-review-signal.yml" \
|
||||
"$repo_root/.github/scripts/cross-boundary-review.cjs" \
|
||||
"$repo_root/.github/scripts/cross-boundary-review.test.cjs"; do
|
||||
if [ -e "$retired_review_gate" ]; then
|
||||
echo "Retired cross-boundary review gate still exists: $retired_review_gate" >&2
|
||||
exit 1
|
||||
fi
|
||||
done
|
||||
! grep -Fq 'cross-boundary-review' "$repo_root/.github/workflows/ci.yml"
|
||||
! grep -Fq 'independent cross-boundary review' "$repo_root/.github/pull_request_template.md"
|
||||
desktop_build_line="$(grep -n -m1 'name: Build and package' "$repo_root/.github/workflows/release-desktop.yml" | cut -d: -f1)"
|
||||
webview2_smoke_line="$(grep -n -m1 'name: Smoke-test Wails approval in WebView2' "$repo_root/.github/workflows/release-desktop.yml" | cut -d: -f1)"
|
||||
signpath_upload_line="$(grep -n -m1 'name: Upload unsigned Windows payload for SignPath' "$repo_root/.github/workflows/release-desktop.yml" | cut -d: -f1)"
|
||||
|
||||
Reference in new issue
Block a user