Compare commits
16 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 951aa5d584 | |||
| d1ce39bd7b | |||
| 97b688f95f | |||
| b716aed914 | |||
| a129f062a2 | |||
| 3d0c84fa6e | |||
| 6b75201c1e | |||
| 0c6f46d279 | |||
| 7f31475330 | |||
| ec6fdbff42 | |||
| 89596516d7 | |||
| f883f39dbf | |||
| d3b9027da3 | |||
| fb7d8d5e3b | |||
| 282b6e0e86 | |||
| 6cefbb070e |
+71
-10
@@ -5,11 +5,19 @@ on:
|
|||||||
branches: [main]
|
branches: [main]
|
||||||
pull_request:
|
pull_request:
|
||||||
types: [opened, synchronize]
|
types: [opened, synchronize]
|
||||||
|
issue_comment:
|
||||||
|
types: [created, edited]
|
||||||
|
|
||||||
|
env:
|
||||||
|
SELF_REVIEW_TTL_MIN: '45'
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
test:
|
test:
|
||||||
runs-on: ubuntu-24.04
|
runs-on: ubuntu-24.04
|
||||||
|
if: github.event_name == 'pull_request'
|
||||||
steps:
|
steps:
|
||||||
|
- name: Install jq
|
||||||
|
run: sudo apt-get update && sudo apt-get install -y jq
|
||||||
- uses: actions/checkout@v4
|
- uses: actions/checkout@v4
|
||||||
- uses: actions/setup-go@v5
|
- uses: actions/setup-go@v5
|
||||||
with:
|
with:
|
||||||
@@ -18,14 +26,58 @@ jobs:
|
|||||||
- run: go vet ./...
|
- run: go vet ./...
|
||||||
- run: go build -o review-bot ./cmd/review-bot
|
- run: go build -o review-bot ./cmd/review-bot
|
||||||
|
|
||||||
# Self-review using native SAP AI Core provider
|
review-gate:
|
||||||
# Models must match SAP AI Core deployments
|
runs-on: ubuntu-24.04
|
||||||
# Available models: gpt-5, anthropic--claude-4.6-sonnet, anthropic--claude-4.6-opus
|
if: github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && github.event.issue.pull_request)
|
||||||
# Removed gpt-4.1, gpt-5-mini, gpt-4.1-mini - not deployed on AI Core
|
outputs:
|
||||||
|
allow_review: ${{ steps.gate.outputs.allow_review }}
|
||||||
|
reason: ${{ steps.gate.outputs.reason }}
|
||||||
|
steps:
|
||||||
|
- name: Install jq
|
||||||
|
run: sudo apt-get update && sudo apt-get install -y jq
|
||||||
|
- name: Check self-review gate
|
||||||
|
id: gate
|
||||||
|
env:
|
||||||
|
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
|
||||||
|
run: |
|
||||||
|
set -e
|
||||||
|
REPO=${{ github.repository }}
|
||||||
|
API="${{ github.server_url }}/api/v1"
|
||||||
|
if [ "${GITHUB_EVENT_NAME}" = "issue_comment" ]; then
|
||||||
|
PR=${{ github.event.issue.number }}
|
||||||
|
else
|
||||||
|
PR=${{ github.event.pull_request.number }}
|
||||||
|
fi
|
||||||
|
# Get head SHA from PR JSON (works for both events)
|
||||||
|
PR_JSON=$(curl -sS -H "Authorization: token $GITEA_TOKEN" "$API/repos/$REPO/pulls/$PR")
|
||||||
|
SHA=$(echo "$PR_JSON" | jq -r .head.sha)
|
||||||
|
UPDATED_AT=$(echo "$PR_JSON" | jq -r .updated_at)
|
||||||
|
NOW=$(date -u +%s)
|
||||||
|
PR_TS=$(date -u -d "$UPDATED_AT" +%s)
|
||||||
|
AGE_MIN=$(( (NOW - PR_TS) / 60 ))
|
||||||
|
TTL_MIN=${SELF_REVIEW_TTL_MIN}
|
||||||
|
|
||||||
|
COMMENTS=$(curl -sS -H "Authorization: token $GITEA_TOKEN" "$API/repos/$REPO/issues/$PR/comments?limit=200")
|
||||||
|
HAS_SR=$(echo "$COMMENTS" | jq -r --arg sha "$SHA" '[.[] | select(.user.login=="rodin") | select(.body|contains("Self-review against "+$sha)) | select(.body|test("(?im)^###\\s+Doc consistency\\b"))] | length')
|
||||||
|
|
||||||
|
if [ "$HAS_SR" -gt 0 ]; then
|
||||||
|
ALLOW=true
|
||||||
|
REASON=self-review
|
||||||
|
elif [ "$AGE_MIN" -ge "$TTL_MIN" ]; then
|
||||||
|
ALLOW=true
|
||||||
|
REASON=ttl
|
||||||
|
else
|
||||||
|
ALLOW=false
|
||||||
|
REASON=missing
|
||||||
|
fi
|
||||||
|
|
||||||
|
echo "allow_review=$ALLOW" >> $GITHUB_OUTPUT
|
||||||
|
echo "reason=$REASON" >> $GITHUB_OUTPUT
|
||||||
|
|
||||||
review:
|
review:
|
||||||
runs-on: ubuntu-24.04
|
runs-on: ubuntu-24.04
|
||||||
if: github.event_name == 'pull_request'
|
if: needs.review-gate.outputs.reason == 'self-review' && (github.event_name == 'pull_request' || github.event_name == 'issue_comment')
|
||||||
needs: test
|
needs: [review-gate]
|
||||||
strategy:
|
strategy:
|
||||||
matrix:
|
matrix:
|
||||||
include:
|
include:
|
||||||
@@ -39,19 +91,28 @@ jobs:
|
|||||||
token_secret: SECURITY_REVIEW_TOKEN
|
token_secret: SECURITY_REVIEW_TOKEN
|
||||||
model: gpt-5
|
model: gpt-5
|
||||||
patterns_repo: rodin/security-patterns
|
patterns_repo: rodin/security-patterns
|
||||||
patterns_files: "."
|
patterns_files: '.'
|
||||||
system_prompt_file: SECURITY_REVIEW.md
|
system_prompt_file: SECURITY_REVIEW.md
|
||||||
steps:
|
steps:
|
||||||
|
- name: Install jq
|
||||||
|
run: sudo apt-get update && sudo apt-get install -y jq
|
||||||
- uses: actions/checkout@v4
|
- uses: actions/checkout@v4
|
||||||
- uses: actions/setup-go@v5
|
- uses: actions/setup-go@v5
|
||||||
with:
|
with:
|
||||||
go-version: '1.26'
|
go-version: '1.26'
|
||||||
- run: go build -o review-bot ./cmd/review-bot
|
- run: go build -o review-bot ./cmd/review-bot
|
||||||
|
- name: Set PR_NUMBER for event type
|
||||||
|
run: |
|
||||||
|
if [ "${GITHUB_EVENT_NAME}" = "issue_comment" ]; then
|
||||||
|
echo "PR_NUMBER=${{ github.event.issue.number }}" >> $GITHUB_ENV
|
||||||
|
else
|
||||||
|
echo "PR_NUMBER=${{ github.event.pull_request.number }}" >> $GITHUB_ENV
|
||||||
|
fi
|
||||||
- name: Run ${{ matrix.name }} review
|
- name: Run ${{ matrix.name }} review
|
||||||
env:
|
env:
|
||||||
VCS_URL: ${{ github.server_url }}
|
VCS_URL: ${{ github.server_url }}
|
||||||
GITEA_REPO: ${{ github.repository }}
|
GITEA_REPO: ${{ github.repository }}
|
||||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
PR_NUMBER: ${{ env.PR_NUMBER }}
|
||||||
REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
|
REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
|
||||||
REVIEWER_NAME: ${{ matrix.name }}
|
REVIEWER_NAME: ${{ matrix.name }}
|
||||||
LLM_PROVIDER: aicore
|
LLM_PROVIDER: aicore
|
||||||
@@ -61,9 +122,9 @@ jobs:
|
|||||||
AICORE_AUTH_URL: ${{ secrets.AICORE_AUTH_URL }}
|
AICORE_AUTH_URL: ${{ secrets.AICORE_AUTH_URL }}
|
||||||
AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
|
AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
|
||||||
AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
|
AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
|
||||||
CONVENTIONS_FILE: "CONVENTIONS.md"
|
CONVENTIONS_FILE: 'CONVENTIONS.md'
|
||||||
PATTERNS_REPO: ${{ matrix.patterns_repo || 'rodin/go-patterns' }}
|
PATTERNS_REPO: ${{ matrix.patterns_repo || 'rodin/go-patterns' }}
|
||||||
PATTERNS_FILES: ${{ matrix.patterns_files || 'README.md,patterns/' }}
|
PATTERNS_FILES: ${{ matrix.patterns_files || 'README.md,patterns/' }}
|
||||||
LLM_TIMEOUT: "600"
|
LLM_TIMEOUT: '600'
|
||||||
SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
|
SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
|
||||||
run: ./review-bot
|
run: ./review-bot
|
||||||
|
|||||||
@@ -0,0 +1,42 @@
|
|||||||
|
name: Workflow Lint
|
||||||
|
|
||||||
|
on:
|
||||||
|
push:
|
||||||
|
branches: [main]
|
||||||
|
pull_request:
|
||||||
|
types: [opened, synchronize]
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
workflow-sanity:
|
||||||
|
runs-on: ubuntu-24.04
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
- name: Sanity check ci.yml triggers and gates
|
||||||
|
run: |
|
||||||
|
set -euo pipefail
|
||||||
|
python3 - <<'PY'
|
||||||
|
import sys, yaml, re
|
||||||
|
from pathlib import Path
|
||||||
|
p = Path('.gitea/workflows/ci.yml')
|
||||||
|
w = yaml.safe_load(p.read_text())
|
||||||
|
# 1) Top-level 'on' must exist and include pull_request + issue_comment
|
||||||
|
on = w.get('on')
|
||||||
|
assert isinstance(on, dict), "ci.yml: top-level 'on' must be a mapping"
|
||||||
|
assert 'pull_request' in on, "ci.yml: missing on.pull_request"
|
||||||
|
assert 'issue_comment' in on, "ci.yml: missing on.issue_comment (self-review trigger)"
|
||||||
|
pr_types = on['pull_request'].get('types', []) if isinstance(on['pull_request'], dict) else []
|
||||||
|
ic_types = on['issue_comment'].get('types', []) if isinstance(on['issue_comment'], dict) else []
|
||||||
|
for t in ['opened','synchronize']:
|
||||||
|
assert t in pr_types, f"ci.yml: pull_request.types must include '{t}'"
|
||||||
|
for t in ['created','edited']:
|
||||||
|
assert t in ic_types, f"ci.yml: issue_comment.types must include '{t}'"
|
||||||
|
# 2) review-gate must run on both PR and issue_comment (if condition string)
|
||||||
|
rg_if = w['jobs']['review-gate'].get('if','')
|
||||||
|
assert 'github.event_name == ' in rg_if and 'issue_comment' in rg_if and 'pull_request' in rg_if, \
|
||||||
|
"ci.yml: review-gate.if must include both pull_request and issue_comment"
|
||||||
|
# 3) review job must require self-review reason
|
||||||
|
rev_if = w['jobs']['review'].get('if','')
|
||||||
|
assert "needs.review-gate.outputs.reason == 'self-review'" in rev_if, \
|
||||||
|
"ci.yml: review.if must require reason=='self-review'"
|
||||||
|
print('OK: ci.yml triggers and gates look sane')
|
||||||
|
PY
|
||||||
@@ -903,12 +903,17 @@ func TestMainSubprocess_InvalidRepo(t *testing.T) {
|
|||||||
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
|
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
|
||||||
args := baseSubprocessArgs()
|
args := baseSubprocessArgs()
|
||||||
// Replace the canonical --repo value with an invalid one.
|
// Replace the canonical --repo value with an invalid one.
|
||||||
|
found := false
|
||||||
for i, a := range args {
|
for i, a := range args {
|
||||||
if a == "--repo" && i+1 < len(args) {
|
if a == "--repo" && i+1 < len(args) {
|
||||||
args[i+1] = "invalidrepo"
|
args[i+1] = "invalidrepo"
|
||||||
|
found = true
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if !found {
|
||||||
|
t.Fatal("baseSubprocessArgs() does not contain --repo; test is broken")
|
||||||
|
}
|
||||||
os.Args = args
|
os.Args = args
|
||||||
main()
|
main()
|
||||||
return
|
return
|
||||||
@@ -930,12 +935,17 @@ func TestMainSubprocess_InvalidPRNumber(t *testing.T) {
|
|||||||
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
|
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
|
||||||
args := baseSubprocessArgs()
|
args := baseSubprocessArgs()
|
||||||
// Replace the canonical --pr value with a non-numeric string.
|
// Replace the canonical --pr value with a non-numeric string.
|
||||||
|
found := false
|
||||||
for i, a := range args {
|
for i, a := range args {
|
||||||
if a == "--pr" && i+1 < len(args) {
|
if a == "--pr" && i+1 < len(args) {
|
||||||
args[i+1] = "notanumber"
|
args[i+1] = "notanumber"
|
||||||
|
found = true
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if !found {
|
||||||
|
t.Fatal("baseSubprocessArgs() does not contain --pr; test is broken")
|
||||||
|
}
|
||||||
os.Args = args
|
os.Args = args
|
||||||
main()
|
main()
|
||||||
return
|
return
|
||||||
|
|||||||
+24
-1
@@ -231,6 +231,8 @@ These are statically checked by `~/.openclaw/workspace/scripts/test/check-invari
|
|||||||
| S6 | Active WIP does not cause early exit (only sets ACTIVE_WIP flag) |
|
| S6 | Active WIP does not cause early exit (only sets ACTIVE_WIP flag) |
|
||||||
| S7 | SPAWN:impl guarded by `ACTIVE_WIP == 0` check |
|
| S7 | SPAWN:impl guarded by `ACTIVE_WIP == 0` check |
|
||||||
| S8 | No merge calls in any worker template |
|
| S8 | No merge calls in any worker template |
|
||||||
|
| S9 | Zero close-PR API calls in dispatch script (`state=closed` does not appear) |
|
||||||
|
| S10 | No close-PR API calls in any worker template; every worker template contains `NEVER close a PR` |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -263,9 +265,20 @@ Each worker receives a precise task description with substituted values:
|
|||||||
|
|
||||||
Workers **always** remove the WIP label on completion and reply `NO_REPLY`.
|
Workers **always** remove the WIP label on completion and reply `NO_REPLY`.
|
||||||
|
|
||||||
|
### Worker Absolute Constraints
|
||||||
|
|
||||||
|
Every worker template begins with an `⛔ ABSOLUTE CONSTRAINTS` section containing these rules:
|
||||||
|
|
||||||
|
- **NEVER close a PR.** Never call `PATCH /pulls/{id}` with `state=closed`. Closing a PR requires human action. "Duplicate", "superseded", or "already done" are never a worker's call.
|
||||||
|
- **NEVER merge a PR.** Never call the merge API. Merging requires human approval.
|
||||||
|
- **NEVER use the gitea-aweiker token.** All API calls use the gitea-rodin token only.
|
||||||
|
- **NEVER act on a PR with active REQUEST_CHANGES.** Fix the findings first.
|
||||||
|
|
||||||
|
The first two constraints are statically enforced by `check-invariants.sh`: S1 and S9 cover the dispatch script (no merge, no close); S8 covers worker templates (no merge calls); S10 covers worker templates (no close calls, with NEVER-close text verified present in each). The remaining two constraints (token usage and REQUEST_CHANGES gate) are enforced by runtime logic.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 9. Fixes for Issues #144 and #145
|
## 9. Fixes for Issues #144, #145, and #157
|
||||||
|
|
||||||
**Issue #144** (autonomous merge):
|
**Issue #144** (autonomous merge):
|
||||||
The dispatch script contains no merge API calls anywhere. The `~/.openclaw/workspace/scripts/test/check-invariants.sh`
|
The dispatch script contains no merge API calls anywhere. The `~/.openclaw/workspace/scripts/test/check-invariants.sh`
|
||||||
@@ -276,3 +289,13 @@ Rule 2 is the **first** rule evaluated per PR. It cannot be skipped, reasoned pa
|
|||||||
or bypassed. It is checked before CI, before self-review, before handoff. The check
|
or bypassed. It is checked before CI, before self-review, before handoff. The check
|
||||||
uses latest-per-reviewer state, so a reviewer who re-approved after REQUEST_CHANGES
|
uses latest-per-reviewer state, so a reviewer who re-approved after REQUEST_CHANGES
|
||||||
is correctly handled.
|
is correctly handled.
|
||||||
|
|
||||||
|
**Issue #157** (autonomous PR close):
|
||||||
|
Worker templates were missing an explicit constraint against closing PRs. The dispatch
|
||||||
|
script never had a close call, but workers could reason their way into calling
|
||||||
|
`PATCH /pulls/{id}` with `state=closed`. All worker templates now include
|
||||||
|
`NEVER close a PR` in their ABSOLUTE CONSTRAINTS section. Invariant S9 verifies
|
||||||
|
the dispatch script contains no close calls. Invariant S10 verifies
|
||||||
|
worker templates contain no close calls and each contains the NEVER-close text.
|
||||||
|
|
||||||
|
Regression tests in `dispatch.bats` statically verify all of these constraints.
|
||||||
|
|||||||
Reference in New Issue
Block a user