Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/fork-governance.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ Internal pull requests targeting `internal/main` use one of three mutually exclu

- **Upstream import** (default): identifies its upstream issue, pull request, head repository, and upstream pull-request head SHA. The **Upstream provenance** check verifies that metadata against the public upstream API, confirms the base repository, actual head repository, and SHA, compares every upstream and fork pull-request patch ID in order, and uploads reconciliation evidence. Direct imports need no human approval; reconciled imports (with `Fork main:` and `Reconciliation merge:` metadata) retain one human approval because conflict resolution requires review.
- **Downstream governance**: uses the exact body marker `Downstream governance: true` and is restricted to the governance allowlist below. It does not assert upstream patch equivalence and does not require a pull-request review.
- **Downstream feature**: uses exactly `Downstream feature: true` plus one non-empty `Downstream rationale:` line. It is fork-only, requires one non-author human `APPROVED` review of the current pull-request head, and cannot change governance-allowlisted files or any `.github/workflows/` file.
- **Downstream feature**: uses exactly `Downstream feature: true` plus one non-empty `Downstream rationale:` line. It is fork-only, requires one human `APPROVED` review of the current pull-request head (self-approval counts — this is a single-maintainer fork), and cannot change governance-allowlisted files or any `.github/workflows/` file.

Feature and governance markers cannot coexist with each other or with upstream-provenance metadata. For a downstream feature, a review on an older head, a dismissed review, an author review, or a bot review does not satisfy the exception. Any new commit invalidates prior feature approval until a reviewer approves the new head.
Feature and governance markers cannot coexist with each other or with upstream-provenance metadata. For a downstream feature, a review on an older head, a dismissed review, or a bot review does not satisfy the exception. Any new commit invalidates prior feature approval until a reviewer approves the new head.

### Approval phases

Downstream feature exceptions have two approval phases. Before merge, the **pre-merge** phase requires one non-author human `APPROVED` review whose commit SHA exactly matches the pull-request head. After merge, the **post-merge** phase accepts the human actor recorded in `merged_by` as approval evidence only when the pull request is both `closed` and `merged`. Missing or bot merger data, and a closed-but-unmerged pull request, fail provenance; closure alone never bypasses review. Downstream governance exceptions do not require approval in either phase.
Downstream feature exceptions have two approval phases. Before merge, the **pre-merge** phase requires one human `APPROVED` review whose commit SHA exactly matches the pull-request head. After merge, the **post-merge** phase accepts the human actor recorded in `merged_by` as approval evidence only when the pull request is both `closed` and `merged`. Missing or bot merger data, and a closed-but-unmerged pull request, fail provenance; closure alone never bypasses review. Downstream governance exceptions do not require approval in either phase.

The provenance evidence for feature exceptions records the phase and evidence type that qualified: an exact-head review before merge or a human merger after merge. Manual provenance dispatch selects the same phase from current pull-request state. Governance exceptions record their downstream-governance outcome without approval evidence.

Expand Down
7 changes: 2 additions & 5 deletions scripts/check-upstream-provenance.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function changedFiles(current, repository, pullRequest, mode) {
return { currentHead, changedFiles: validateChangedPaths(changed, mode) };
}

function resolveApproval(reviews, currentHead, authorLogin) {
function resolveApproval(reviews, currentHead) {
const latestByReviewer = new Map();
for (const review of reviews) {
const login = review?.user?.login;
Expand All @@ -195,14 +195,12 @@ function resolveApproval(reviews, currentHead, authorLogin) {
(review) =>
review.state?.toUpperCase() === "APPROVED" &&
review.user?.type === "User" &&
authorLogin &&
review.user.login.toLowerCase() !== authorLogin.toLowerCase() &&
typeof review.commit_id === "string" &&
review.commit_id.toLowerCase() === currentHead,
);
if (!approval) {
throw new Error(
"Downstream feature exception requires a non-author human approval of the current pull request head",
"Downstream feature exception requires a human APPROVED review of the current pull request head",
);
}
return {
Expand Down Expand Up @@ -252,7 +250,6 @@ function effectiveApproval(current, repository, pullRequest) {
...resolveApproval(
paginatedJson(`repos/${repository}/pulls/${pullRequest}/reviews`),
currentHead,
current.user?.login ?? "",
),
};
}
Expand Down
47 changes: 23 additions & 24 deletions scripts/check-upstream-provenance.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -93,51 +93,50 @@ test("forbids governance and workflow paths in feature mode", () => {
);
});

test("requires a current-head approval from a non-author human", () => {
assert.deepEqual(resolveApproval([review()], currentHead, "author"), {
test("requires a current-head APPROVED human review, author or not", () => {
assert.deepEqual(resolveApproval([review()], currentHead), {
id: 1,
authorLogin: "reviewer",
authorType: "User",
state: "APPROVED",
commitOid: currentHead,
});
// A solo maintainer is their own only reviewer, so self-approval counts.
assert.deepEqual(
resolveApproval([review({ user: { login: "author", type: "User" } })], currentHead),
{
id: 1,
authorLogin: "author",
authorType: "User",
state: "APPROVED",
commitOid: currentHead,
},
);
for (const candidate of [
review({ commit_id: otherHead }),
review({ user: { login: "author", type: "User" } }),
review({ user: { login: "automation", type: "Bot" } }),
review({ state: "DISMISSED" }),
]) {
assert.throws(
() => resolveApproval([candidate], currentHead, "author"),
/current pull request head/,
);
assert.throws(() => resolveApproval([candidate], currentHead), /current pull request head/);
}
});

test("latest review state replaces an older approval", () => {
assert.throws(
() =>
resolveApproval(
[review({ id: 1 }), review({ id: 2, state: "DISMISSED" })],
currentHead,
"author",
),
() => resolveApproval([review({ id: 1 }), review({ id: 2, state: "DISMISSED" })], currentHead),
/current pull request head/,
);
});

test("preserves current-head approval across later comment-only reviews", () => {
for (const state of ["COMMENTED", "PENDING"]) {
assert.deepEqual(
resolveApproval([review({ id: 1 }), review({ id: 2, state })], currentHead, "author"),
{
id: 1,
authorLogin: "reviewer",
authorType: "User",
state: "APPROVED",
commitOid: currentHead,
},
);
assert.deepEqual(resolveApproval([review({ id: 1 }), review({ id: 2, state })], currentHead), {
id: 1,
authorLogin: "reviewer",
authorType: "User",
state: "APPROVED",
commitOid: currentHead,
});
}
});

Expand Down Expand Up @@ -182,7 +181,7 @@ test("governance evidence does not require review approval", () => {
});

test("feature evidence retains effective approval", () => {
const approval = resolveApproval([review()], currentHead, "author");
const approval = resolveApproval([review()], currentHead);
const evidence = exceptionEvidence(
"fork/project",
7,
Expand Down
2 changes: 1 addition & 1 deletion scripts/ci-workflow.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ test("feature exceptions enforce phase-specific human approval evidence", () =>
assert.match(provenance, /evidenceType: "exact-head-review"/);
assert.match(provenance, /phase: "post-merge"/);
assert.match(provenance, /evidenceType: "human-merger"/);
assert.match(provenance, /non-author human approval of the current pull request head/);
assert.match(provenance, /human APPROVED review of the current pull request head/);
});

test("governance exceptions do not require human approval", () => {
Expand Down
Loading