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
10 changes: 8 additions & 2 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## What
## Summary

<!-- One-sentence summary of the change -->

Expand Down Expand Up @@ -33,6 +33,12 @@
- [ ] New provider follows the six-file structure, if applicable
- [ ] No new runtime dependencies added without justification

## Behavior / risk

<!-- Required only when changing provider contracts, config/schema parsing, release/package files, or GitHub workflows.
State the user-visible behavior or compatibility/security risk. “No user-visible behavior change” is valid for an internal refactor. -->

## Validation

<!-- How did you verify this change works? -->
<!-- Commands, automated test cases, or manual reproduction used to verify this change.
For high-risk changes, this must be non-empty. -->
22 changes: 21 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ jobs:
scripts/lint-changed.sh
scripts/verify.ts
scripts/verify.test.ts
scripts/pr-evidence.ts
scripts/pr-evidence.test.ts
scripts/release/
scripts/setup-githooks.sh
'
Expand Down Expand Up @@ -115,6 +117,23 @@ jobs:
echo "Add '[policy]' to a commit message to acknowledge changing the verification harness itself."
exit 1

maintainer-evidence:
name: Maintainer evidence
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
with:
fetch-depth: 0
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
with:
bun-version: "1.3.5"
- name: Require evidence only for high-risk changes
env:
BASE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
HEAD_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
PR_BODY: ${{ github.event.pull_request.body }}
run: bun scripts/pr-evidence.ts

package-compatibility:
name: Package compatibility / Node ${{ matrix.node }}
runs-on: ubuntu-latest
Expand All @@ -138,14 +157,15 @@ jobs:
gate:
name: Gate
runs-on: ubuntu-latest
needs: [prepare-verification, verify, policy-surface, package-compatibility]
needs: [prepare-verification, verify, policy-surface, maintainer-evidence, package-compatibility]
if: always()
steps:
- name: Check results
run: |
if [ "${{ needs.prepare-verification.result }}" != "success" ] || \
[ "${{ needs.verify.result }}" != "success" ] || \
[ "${{ needs.policy-surface.result }}" != "success" ] || \
[ "${{ needs.maintainer-evidence.result }}" != "success" ] || \
[ "${{ needs.package-compatibility.result }}" != "success" ]; then
echo "One or more required checks failed."
exit 1
Expand Down
4 changes: 4 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ Every PR must satisfy:
2. At least one maintainer approval
3. No unresolved review comments

We keep the contribution path open: documentation, examples, and ordinary small changes do not need a design document or a special PR label. Changes to provider contracts, configuration/schema parsing, release/package files, or GitHub workflows have a small additional requirement: fill in the **Behavior / risk** and **Validation** sections of the PR description. This lets maintainers review the contract and its evidence before reading an implementation diff.

Maintainers should enable the repository settings that make the same policy effective at merge time: require the `Gate` and `Analyze TypeScript` checks, require one approving review, dismiss stale approvals, require approval of the latest push, and require resolved conversations. CODEOWNERS review and a merge queue are intentionally not required for routine contributions at the current project stage.

## Reporting bugs and requesting features

Open an issue using the templates under [.github/ISSUE_TEMPLATE](./.github/ISSUE_TEMPLATE). For anything security-related, do **not** open a public issue — follow [SECURITY.md](./SECURITY.md) instead.
Expand Down
1 change: 1 addition & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ For external contributors.
| Doc | What's inside |
|-----|---------------|
| [Contributing overview](../CONTRIBUTING.md) | Dev setup, merge requirements, code style. |
| [Maintainer review](./contributing/maintainer-review.md) | Evidence-based PR review and merge gates. |
| [Development](./contributing/development.md) | Clone, install, run, test, package boundaries. |
| [Provider development](./contributing/provider-development.md) | The six files a new provider must implement. |
| [Release](./contributing/release.md) | npm publishing workflow and Trusted Publishing. |
Expand Down
28 changes: 28 additions & 0 deletions docs/contributing/maintainer-review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Maintainer review

OpenAgentPack welcomes exploratory and AI-assisted contributions. The merge bar is based on evidence, not on who wrote the code or how it was produced.

## Review in this order

1. Read **Summary**, then **Behavior / risk** and **Validation** when present.
2. Confirm that CI covers the claimed behavior. A passing test is useful only if it would fail when the stated contract regresses.
3. Read the diff around provider, configuration, credentials, release, and workflow boundaries. These paths have the greatest blast radius.
4. Resolve every conversation, then approve only the latest commit.

## Gate levels

| Change | Automated gate | Human review |
| --- | --- | --- |
| Documentation, examples, small isolated fixes | `Gate` | Scope and user-facing accuracy |
| CLI or SDK behavior | `Gate` | Contract plus a regression test or reproduction |
| Provider contracts, config/schema, release/package files, workflows | `Gate` plus CodeQL | Behavior/risk and validation evidence; inspect compatibility and security impact |

The PR evidence check deliberately applies only to the third row. It does not require a design document, a linked Issue, or an AI-use declaration.

## GitHub settings

Configure `main` with these required checks: `Gate` and `Analyze TypeScript`. Require one approving review, dismiss stale approvals, require approval of the most recent push, require conversations to be resolved, and require branches to be up to date. Keep linear history enabled. Do not require CODEOWNERS review or a merge queue until review volume makes them worthwhile.

## Fast disposition

If a PR does not yet provide enough evidence, ask for one focused thing: a failing regression test, a runnable reproduction, or a clear statement of the intended behavior. If the author cannot provide it, keep the PR open as a Draft or close it with an invitation to reopen when the evidence is available.
38 changes: 38 additions & 0 deletions scripts/pr-evidence.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { describe, expect, test } from "bun:test";
import { requiresMaintainerEvidence, validateMaintainerEvidence } from "./pr-evidence.ts";

describe("pull-request maintainer evidence", () => {
test("does not add a merge barrier for ordinary contribution paths", () => {
expect(requiresMaintainerEvidence(["README.md", "packages/cli/src/program.ts"])).toBe(false);
expect(validateMaintainerEvidence("", ["README.md"])).toBeUndefined();
});

test("asks for behavior and validation evidence on high-risk paths", () => {
const files = ["packages/sdk/src/internal/providers/interface.ts"];
expect(requiresMaintainerEvidence(files)).toBe(true);
expect(validateMaintainerEvidence("## Summary\nA change", files)).toContain("## Behavior / risk");
});

test("accepts concrete evidence for high-risk changes", () => {
const body = `## Summary
Add a provider capability.

## Behavior / risk

Existing provider configuration remains unchanged; the new capability is opt-in.

## Validation

\`bun test packages/sdk/tests/unit/provider-conformance.test.ts\``;
expect(validateMaintainerEvidence(body, [".github/workflows/release.yml"])).toBeUndefined();
});

test("does not treat HTML comments as evidence", () => {
const body = `## Behavior / risk
<!--

## Validation
<!--`;
expect(validateMaintainerEvidence(body, [".github/workflows/release.yml"])).toContain("## Behavior / risk");
});
});
68 changes: 68 additions & 0 deletions scripts/pr-evidence.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { existsSync } from "node:fs";

const highRiskPathPatterns = [
/^\.github\/workflows\//,
/^scripts\/release\//,
/^(?:package\.json|bun\.lock)$/,
/^packages\/sdk\/src\/internal\/(?:parser\/|types\/(?:config|session)\.ts|providers\/(?:interface|capabilities|registry|base-client)\.ts)/,
];

export function requiresMaintainerEvidence(files: readonly string[]): boolean {
return files.some((file) => highRiskPathPatterns.some((pattern) => pattern.test(file)));
}

function sectionHasContent(body: string, heading: string): boolean {
const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const match = body.match(new RegExp(`^## ${escapedHeading}[ \\t]*\\r?\\n([\\s\\S]*?)(?=^## |(?![\\s\\S]))`, "mi"));
if (!match) return false;
return (
match[1]
.replace(/<!--[\s\S]*?(?:-->|$)/g, "")
.replace(/- \[ \] .*/g, "")
.trim().length > 0
);
}

export function validateMaintainerEvidence(body: string, files: readonly string[]): string | undefined {
if (!requiresMaintainerEvidence(files)) return undefined;
const missing = ["Behavior / risk", "Validation"].filter((heading) => !sectionHasContent(body, heading));
if (missing.length === 0) return undefined;
return `High-risk changes require a non-empty ${missing.map((heading) => `## ${heading}`).join(" and ")} section.`;
}

function changedFiles(base: string, head: string): string[] {
const result = Bun.spawnSync(["git", "diff", "--name-only", "--diff-filter=ACMR", base, head], {
stdout: "pipe",
stderr: "pipe",
});
if (result.exitCode !== 0) throw new Error(result.stderr.toString().trim() || "Could not read changed files.");
return result.stdout.toString().split("\n").filter(Boolean);
}

function main(): number {
if (process.env.GITHUB_EVENT_NAME !== "pull_request") {
console.log("Not a pull request; maintainer-evidence check skipped.");
return 0;
}
const base = process.env.BASE_SHA;
const head = process.env.HEAD_SHA;
if (!base || !head || !existsSync(".git"))
throw new Error("BASE_SHA and HEAD_SHA are required for pull-request evidence checks.");
const files = changedFiles(base, head);
const error = validateMaintainerEvidence(process.env.PR_BODY ?? "", files);
if (!error) {
console.log(
requiresMaintainerEvidence(files)
? "Maintainer evidence supplied."
: "Low-risk change; maintainer evidence is optional.",
);
return 0;
}
console.error(`::error::${error}`);
console.error(
"Describe the user-visible behavior or compatibility risk, then give the test command, test case, or manual reproduction used to validate it.",
);
return 1;
}

if (import.meta.main) process.exit(main());