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
2 changes: 1 addition & 1 deletion .claude/skills/ship/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Run this flow only for a pull request that targets `internal/main`.
4. Post the review to GitHub. Include the reviewed head SHA, verdict, high-risk findings, and advisory findings.
5. Stop if the review finds a high-risk issue. Request changes with path and line evidence.
6. For a clean direct import, dispatch **Upstream import merge** after the review posts.
7. For a reconciled import, review only the conflict-resolution diff. Submit the required human approval, then dispatch **Upstream import merge**.
7. For a reconciled import, review only the conflict-resolution diff, then dispatch **Upstream import merge**.
8. Confirm the pull request merged with a merge commit.

Do not merge a pull request with failed CI, failed provenance, a stale upstream head, or an unresolved high-risk finding.
16 changes: 6 additions & 10 deletions docs/fork-governance.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,13 @@ This repository is the public `ZGEnergy/paseo` fork. Keep the fork's `main` bran

Internal pull requests targeting `internal/main` use one of three mutually exclusive modes:

- **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 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.
- **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, and compares every upstream and fork pull-request patch ID in order (direct imports), or validates the `[upstreamHead, forkMain]` reconciliation merge commit (reconciled imports, with `Fork main:` and `Reconciliation merge:` metadata).
- **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.
- **Downstream feature**: uses exactly `Downstream feature: true` plus one non-empty `Downstream rationale:` line. It is fork-only 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, 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.

### Approval phases

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.
None of the three modes requires a pull-request review. This is a single-maintainer fork, and GitHub refuses to let an author approve their own pull request, so any review requirement here would be permanently unsatisfiable. Provenance is established mechanically — from the labeled metadata fields and, for upstream imports, patch-ID or reconciliation-merge verification — not from a second human's sign-off.

If an upstream import is stale, the final provenance run fails and the bot does not merge it. Update its metadata or reconcile against the new upstream head, then rerun checks. Never merge an import whose upstream head changed after review.

Expand Down Expand Up @@ -63,7 +59,7 @@ This exception does not provide a general provenance bypass: a non-governance pa

## Local ship gate

Internal teammates run `/ship` locally for a pull request that targets `internal/main`. The operator uses their own review agent, posts its review to GitHub, and stops on a high-risk finding. The skill requires CI and provenance for the current head before review. A clean direct import may dispatch **Upstream import merge** after the review posts. A reconciled import requires the focused conflict-resolution review and the existing human approval before that dispatch. See `.claude/skills/ship/SKILL.md` for the procedure.
Internal teammates run `/ship` locally for a pull request that targets `internal/main`. The operator uses their own review agent, posts its review to GitHub, and stops on a high-risk finding. The skill requires CI and provenance for the current head before review. A clean direct import may dispatch **Upstream import merge** after the review posts. A reconciled import requires the focused conflict-resolution review before that dispatch. See `.claude/skills/ship/SKILL.md` for the procedure.

## CI and permissions

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

function resolveApproval(reviews, currentHead) {
const latestByReviewer = new Map();
for (const review of reviews) {
const login = review?.user?.login;
if (!login) continue;
const state = review.state?.toUpperCase();
if (state === "COMMENTED" || state === "PENDING") continue;
const key = login.toLowerCase();
const previous = latestByReviewer.get(key);
if (!previous || Number(review.id ?? 0) >= Number(previous.id ?? 0)) {
latestByReviewer.set(key, review);
}
}
const approval = [...latestByReviewer.values()].find(
(review) =>
review.state?.toUpperCase() === "APPROVED" &&
review.user?.type === "User" &&
typeof review.commit_id === "string" &&
review.commit_id.toLowerCase() === currentHead,
);
if (!approval) {
throw new Error(
"Downstream feature exception requires a human APPROVED review of the current pull request head",
);
}
return {
id: approval.id,
authorLogin: approval.user.login,
authorType: approval.user.type,
state: approval.state,
commitOid: approval.commit_id.toLowerCase(),
};
}

function mergedApproval(current) {
const state = current.state?.toLowerCase();
const isClosed = state === "closed";
const isMerged = current.merged === true;
if (isClosed && !isMerged) {
throw new Error(
"Downstream exception requires a closed and merged pull request for post-merge evidence",
);
}
if (isMerged && !isClosed) {
throw new Error(
"Downstream exception requires a closed and merged pull request for post-merge evidence",
);
}
if (!isClosed) return undefined;

const merger = current.merged_by;
if (!merger?.login || merger.type !== "User") {
throw new Error("Merged downstream exception requires a recorded human merged_by actor");
}
return {
phase: "post-merge",
evidenceType: "human-merger",
authorLogin: merger.login,
authorType: merger.type,
};
}

function effectiveApproval(current, repository, pullRequest) {
const postMergeApproval = mergedApproval(current);
if (postMergeApproval) return postMergeApproval;
const currentHead = shaFrom(current.head?.sha ?? "", "Pull request head");
return {
phase: "pre-merge",
evidenceType: "exact-head-review",
...resolveApproval(
paginatedJson(`repos/${repository}/pulls/${pullRequest}/reviews`),
currentHead,
),
};
}

function exceptionEvidence(repository, pullRequest, mode, rationale, scope, approval) {
function exceptionEvidence(repository, pullRequest, mode, rationale, scope) {
return {
repository,
pullRequest,
Expand All @@ -268,16 +192,13 @@ function exceptionEvidence(repository, pullRequest, mode, rationale, scope, appr
changedFiles: scope.changedFiles,
},
currentHead: scope.currentHead,
...(mode === "downstream-feature" ? { approval } : {}),
result: mode === "downstream-governance" ? "governance-exception" : "feature-exception",
};
}

function downstreamExceptionEvidence(current, repository, pullRequest, mode, rationale) {
const scope = changedFiles(current, repository, pullRequest, mode);
const approval =
mode === "downstream-feature" ? effectiveApproval(current, repository, pullRequest) : undefined;
return exceptionEvidence(repository, pullRequest, mode, rationale, scope, approval);
return exceptionEvidence(repository, pullRequest, mode, rationale, scope);
}

function metadataValue(body, label) {
Expand Down Expand Up @@ -400,23 +321,6 @@ function patchIds(patch, label) {
return ids;
}

function approvalSummary(approval) {
const lines = [
`- Approval phase: \`${approval.phase}\``,
`- Approval evidence: \`${approval.evidenceType}\``,
];
if (approval.phase === "post-merge") {
lines.push(`- Human merger: \`${approval.authorLogin}\` (${approval.authorType})`);
} else {
lines.push(
`- Effective approval: review #${approval.id} by \`${approval.authorLogin}\` (${approval.authorType})`,
`- Review state: \`${approval.state}\``,
`- Review commit: \`${approval.commitOid}\` (matches current head)`,
);
}
return lines;
}

function writeEvidence(path, evidence) {
if (path) {
mkdirSync(dirname(path), { recursive: true });
Expand All @@ -430,8 +334,7 @@ function writeEvidence(path, evidence) {
`- Rationale: ${evidence.rationale}`,
`- Changed files: \`${evidence.scope.changedFiles.join(", ")}\``,
`- Current pull request head: \`${evidence.currentHead}\``,
...approvalSummary(evidence.approval),
"- Result: **downstream feature exception accepted; no upstream patch equivalence asserted**",
"- Result: **downstream feature exception accepted; no upstream patch equivalence asserted, no review required**",
"",
];
appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${lines.join("\n")}\n`);
Expand Down Expand Up @@ -739,10 +642,8 @@ export {
DOWNSTREAM_GOVERNANCE_PATHS,
downstreamFeatureMarker,
downstreamGovernanceMarker,
effectiveApproval,
exceptionEvidence,
exceptionMode,
resolveApproval,
validateChangedFileCount,
validateChangedPaths,
};
108 changes: 7 additions & 101 deletions scripts/check-upstream-provenance.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,13 @@ import test from "node:test";

import {
downstreamFeatureMarker,
effectiveApproval,
exceptionEvidence,
exceptionMode,
resolveApproval,
validateChangedFileCount,
validateChangedPaths,
} from "./check-upstream-provenance.mjs";

const currentHead = "a".repeat(40);
const otherHead = "b".repeat(40);

function review(overrides = {}) {
return {
id: 1,
state: "APPROVED",
commit_id: currentHead,
user: { login: "reviewer", type: "User" },
...overrides,
};
}

test("accepts exact downstream-feature marker and rationale", () => {
assert.deepEqual(
Expand Down Expand Up @@ -93,83 +80,6 @@ test("forbids governance and workflow paths in feature mode", () => {
);
});

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: "automation", type: "Bot" } }),
review({ state: "DISMISSED" }),
]) {
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),
/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), {
id: 1,
authorLogin: "reviewer",
authorType: "User",
state: "APPROVED",
commitOid: currentHead,
});
}
});

test("accepts human merger evidence only for a closed-and-merged pull request", () => {
assert.deepEqual(
effectiveApproval(
{
state: "closed",
merged: true,
merged_by: { login: "merger", type: "User" },
},
"fork/project",
7,
),
{
phase: "post-merge",
evidenceType: "human-merger",
authorLogin: "merger",
authorType: "User",
},
);
for (const current of [
{ state: "closed", merged: false, merged_by: { login: "merger", type: "User" } },
{ state: "closed", merged: true },
{ state: "closed", merged: true, merged_by: { login: "automation", type: "Bot" } },
]) {
assert.throws(
() => effectiveApproval(current, "fork/project", 7),
/closed and merged|human merged_by/,
);
}
});

test("governance evidence does not require review approval", () => {
const evidence = exceptionEvidence("fork/project", 7, "downstream-governance", undefined, {
currentHead,
Expand All @@ -180,16 +90,12 @@ test("governance evidence does not require review approval", () => {
assert.equal(evidence.currentHead, currentHead);
});

test("feature evidence retains effective approval", () => {
const approval = resolveApproval([review()], currentHead);
const evidence = exceptionEvidence(
"fork/project",
7,
"downstream-feature",
"reason",
{ currentHead, changedFiles: ["packages/example.ts"] },
approval,
);
test("feature evidence does not require or store review approval", () => {
const evidence = exceptionEvidence("fork/project", 7, "downstream-feature", "reason", {
currentHead,
changedFiles: ["packages/example.ts"],
});
assert.equal(evidence.result, "feature-exception");
assert.deepEqual(evidence.approval, approval);
assert.equal("approval" in evidence, false);
assert.equal(evidence.currentHead, currentHead);
});
20 changes: 4 additions & 16 deletions scripts/ci-workflow.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -125,25 +125,13 @@ test("fork governance workflows retain their enforcement boundaries", () => {
assert.match(relayDeploy, /if: \$\{\{ github\.repository == 'ZGEnergy\/paseo' \}\}/);
assert.match(relayDeploy, /npx wrangler deploy/);
});
test("feature exceptions enforce phase-specific human approval evidence", () => {
const provenance = readFileSync(provenanceScriptPath, "utf8");

assert.match(provenance, /pulls\/\$\{pullRequest\}\/reviews/);
assert.match(provenance, /phase: "pre-merge"/);
assert.match(provenance, /evidenceType: "exact-head-review"/);
assert.match(provenance, /phase: "post-merge"/);
assert.match(provenance, /evidenceType: "human-merger"/);
assert.match(provenance, /human APPROVED review of the current pull request head/);
});

test("governance exceptions do not require human approval", () => {
test("no exception mode fetches or requires a human review approval", () => {
const provenance = readFileSync(provenanceScriptPath, "utf8");

assert.match(provenance, /mode === "downstream-feature"/);
assert.doesNotMatch(
provenance,
/Downstream exception requires a non-author human approval of the current pull request head/,
);
assert.doesNotMatch(provenance, /pulls\/\$\{pullRequest\}\/reviews/);
assert.doesNotMatch(provenance, /APPROVED/);
assert.doesNotMatch(provenance, /merged_by/);
});

test("provenance closed events require a merge and CI covers internal release pushes", () => {
Expand Down
Loading