Skip to content

Commit 2fde12a

Browse files
committed
fix(ci): reduce contributor friction in PR gates
Change-Id: I1f4b06c06ca304d98f5a1151aad48f1bf4537cec
1 parent e205260 commit 2fde12a

7 files changed

Lines changed: 28 additions & 75 deletions

File tree

.github/pull_request_template.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
## Behavior / risk
3737

3838
<!-- Required only when changing provider contracts, config/schema parsing, release/package files, or GitHub workflows.
39-
State the user-visible behavior or compatibility/security risk. “No user-visible behavior change” is valid for an internal refactor. -->
39+
State the user-visible behavior or compatibility/security risk. “No user-visible behavior change” is valid for an internal refactor.
40+
No special commit-message marker or history rewrite is required. -->
4041

4142
## Validation
4243

.github/workflows/ci.yml

Lines changed: 1 addition & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -50,73 +50,6 @@ jobs:
5050
BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
5151
run: bun scripts/verify.ts full --step "${{ matrix.step }}"
5252

53-
policy-surface:
54-
name: Policy surface
55-
runs-on: ubuntu-latest
56-
steps:
57-
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
58-
with:
59-
fetch-depth: 0
60-
- name: Require [policy] marker when control files change
61-
env:
62-
BASE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
63-
HEAD_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
64-
run: |
65-
set -euo pipefail
66-
67-
# The control plane: deterministic gates and the rules that govern them.
68-
# Editing any of these changes what every other check enforces, so the
69-
# change must be explicit — a [policy] marker in the commit range.
70-
patterns='
71-
.dependency-cruiser.cjs
72-
sgconfig.yml
73-
biome.json
74-
tools/architecture/
75-
.githooks/
76-
.claude/hooks/
77-
.github/workflows/
78-
scripts/lint-changed.sh
79-
scripts/verify.ts
80-
scripts/verify.test.ts
81-
scripts/pr-evidence.ts
82-
scripts/pr-evidence.test.ts
83-
scripts/release/
84-
scripts/setup-githooks.sh
85-
'
86-
87-
base="$BASE_SHA"
88-
# New branch / first push: no usable base; nothing to diff against.
89-
if [ -z "$base" ] || ! git cat-file -e "$base^{commit}" 2>/dev/null; then
90-
echo "No base commit to compare against; skipping policy-surface check."
91-
exit 0
92-
fi
93-
94-
changed=$(git diff --name-only "$base" "$HEAD_SHA")
95-
touched=""
96-
for p in $patterns; do
97-
hit=$(printf '%s\n' "$changed" | grep -F "$p" || true)
98-
if [ -n "$hit" ]; then
99-
touched="$touched$hit"$'\n'
100-
fi
101-
done
102-
103-
if [ -z "$touched" ]; then
104-
echo "No control-plane files changed."
105-
exit 0
106-
fi
107-
108-
echo "Control-plane files changed:"
109-
printf '%s' "$touched" | sed 's/^/ - /'
110-
111-
if git log "$base..$HEAD_SHA" --format=%B | grep -qF '[policy]'; then
112-
echo "Found [policy] marker in commit range. OK."
113-
exit 0
114-
fi
115-
116-
echo "::error::Control-plane files changed without a [policy] marker in any commit message."
117-
echo "Add '[policy]' to a commit message to acknowledge changing the verification harness itself."
118-
exit 1
119-
12053
maintainer-evidence:
12154
name: Maintainer evidence
12255
runs-on: ubuntu-latest
@@ -157,14 +90,13 @@ jobs:
15790
gate:
15891
name: Gate
15992
runs-on: ubuntu-latest
160-
needs: [prepare-verification, verify, policy-surface, maintainer-evidence, package-compatibility]
93+
needs: [prepare-verification, verify, maintainer-evidence, package-compatibility]
16194
if: always()
16295
steps:
16396
- name: Check results
16497
run: |
16598
if [ "${{ needs.prepare-verification.result }}" != "success" ] || \
16699
[ "${{ needs.verify.result }}" != "success" ] || \
167-
[ "${{ needs.policy-surface.result }}" != "success" ] || \
168100
[ "${{ needs.maintainer-evidence.result }}" != "success" ] || \
169101
[ "${{ needs.package-compatibility.result }}" != "success" ]; then
170102
echo "One or more required checks failed."

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ Every PR must satisfy:
3939
2. At least one maintainer approval
4040
3. No unresolved review comments
4141

42-
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.
42+
We keep the contribution path open: documentation, examples, and ordinary small changes do not need a design document, a special PR label, or a commit-message marker. Changes to provider contracts, configuration/schema parsing, release/package files, GitHub workflows, or verification policy 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 without asking contributors to rewrite commit history. A generated `bun.lock` change by itself is handled by audit and compatibility checks.
4343

4444
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.
4545

apps/server/tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"extends": "../../tsconfig.base.json",
33
"compilerOptions": {
44
"lib": ["ESNext", "DOM", "DOM.Iterable"],
5-
"types": ["node"],
5+
"types": ["node", "bun"],
66
"resolveJsonModule": true,
77
"paths": {
88
"@/*": ["./src/*"],

docs/contributing/maintainer-review.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ OpenAgentPack welcomes exploratory and AI-assisted contributions. The merge bar
1717
| CLI or SDK behavior | `Gate` | Contract plus a regression test or reproduction |
1818
| Provider contracts, config/schema, release/package files, workflows | `Gate` plus CodeQL | Behavior/risk and validation evidence; inspect compatibility and security impact |
1919

20-
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.
20+
The PR evidence check deliberately applies only to the third row. It does not require a design document, a linked Issue, an AI-use declaration, a special commit-message marker, or a history rewrite. A generated `bun.lock` change by itself is covered by audit and compatibility checks rather than the evidence gate.
2121

2222
## GitHub settings
2323

scripts/pr-evidence.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,21 @@ import { requiresMaintainerEvidence, validateMaintainerEvidence } from "./pr-evi
33

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

10+
test("uses PR evidence instead of commit-message markers for control-plane changes", () => {
11+
const files = [".github/workflows/ci.yml", "scripts/verify.ts"];
12+
expect(requiresMaintainerEvidence(files)).toBe(true);
13+
expect(validateMaintainerEvidence("## Summary\nAdjust CI", files)).toContain("## Behavior / risk");
14+
});
15+
16+
test("treats package manifests, but not the generated lockfile alone, as high risk", () => {
17+
expect(requiresMaintainerEvidence(["packages/sdk/package.json"])).toBe(true);
18+
expect(requiresMaintainerEvidence(["bun.lock"])).toBe(false);
19+
});
20+
1021
test("asks for behavior and validation evidence on high-risk paths", () => {
1122
const files = ["packages/sdk/src/internal/providers/interface.ts"];
1223
expect(requiresMaintainerEvidence(files)).toBe(true);

scripts/pr-evidence.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
import { existsSync } from "node:fs";
22

33
const highRiskPathPatterns = [
4+
// Verification and automation control plane. These changes require explicit
5+
// PR evidence, but never a special commit message or history rewrite.
6+
/^\.dependency-cruiser\.cjs$/,
7+
/^sgconfig\.yml$/,
8+
/^biome\.json$/,
9+
/^tools\/architecture\//,
10+
/^\.githooks\//,
11+
/^\.claude\/hooks\//,
412
/^\.github\/workflows\//,
13+
/^scripts\/(?:lint-changed\.sh|verify(?:\.test)?\.ts|pr-evidence(?:\.test)?\.ts|setup-githooks\.sh)$/,
514
/^scripts\/release\//,
6-
/^(?:package\.json|bun\.lock)$/,
15+
/(?:^|\/)package\.json$/,
716
/^packages\/sdk\/src\/internal\/(?:parser\/|types\/(?:config|session)\.ts|providers\/(?:interface|capabilities|registry|base-client)\.ts)/,
817
];
918

0 commit comments

Comments
 (0)