fix(agents): add FORGEJO_REVIEWER_PASSWORD and purge stale self-approval language
- shared/credential_security.md: added FORGEJO_REVIEWER_PASSWORD, fixed comment - shared/merge_safety.md: updated approval docstring + comment for dual-account - implementation-worker.md: updated 2 approval function docstrings - pr-merge-pool-supervisor.md: removed 2 stale self-approval references - pr-reviewer.md: removed stale self-approval error handling text - project-bootstrapper.md: updated branch protection notes for dual-account
This commit is contained in:
@@ -532,14 +532,13 @@ elif work_type == "ready-to-merge":
|
||||
"""
|
||||
Check if PR has required approvals.
|
||||
All PRs need exactly 1 approval per CONTRIBUTING.md.
|
||||
Self-approval is permitted, including for automated bot PRs.
|
||||
The system uses dual bot accounts: PRs are created by the primary bot
|
||||
(FORGEJO_USERNAME) and reviewed by a separate reviewer bot
|
||||
(FORGEJO_REVIEWER_USERNAME). Formal APPROVED reviews are the primary
|
||||
approval path. Review bodies and issue comments are checked as fallback.
|
||||
|
||||
IMPORTANT: Forgejo blocks formal APPROVED reviews when the reviewer
|
||||
is the same user who created the PR ("approve your own pull is not
|
||||
allowed"). Since all agents share a single bot account, formal
|
||||
APPROVED reviews are often impossible. Therefore this function
|
||||
checks THREE sources for approval signals:
|
||||
1. Formal reviews with APPROVED state
|
||||
This function checks THREE sources for approval signals:
|
||||
1. Formal reviews with APPROVED state (primary — from reviewer account)
|
||||
2. Review bodies (COMMENT/PENDING state) containing approval keywords
|
||||
3. Issue comments containing approval keywords
|
||||
Any one of these is sufficient.
|
||||
@@ -2153,13 +2152,10 @@ def merge_pr():
|
||||
"""
|
||||
Check if PR has required approvals.
|
||||
All PRs need exactly 1 approval per CONTRIBUTING.md.
|
||||
Self-approval is permitted, including for automated bot PRs.
|
||||
|
||||
IMPORTANT: Forgejo blocks formal APPROVED reviews when the reviewer
|
||||
is the same user who created the PR. Since all agents share a single
|
||||
bot account, formal APPROVED reviews are often impossible. This
|
||||
function checks three sources: formal reviews, review bodies, and
|
||||
issue comments for approval keywords.
|
||||
The system uses dual bot accounts: PRs are created by the primary bot
|
||||
and reviewed by a separate reviewer bot. Formal APPROVED reviews are
|
||||
the primary approval path. This function also checks review bodies and
|
||||
issue comments as fallback approval signals.
|
||||
"""
|
||||
reviews = forgejo_list_pull_reviews(owner, repo, pr_number)
|
||||
comments = forgejo_list_issue_comments(owner, repo, pr_number)
|
||||
|
||||
@@ -122,7 +122,7 @@ A PR is ready to merge when ALL of the following are true:
|
||||
|
||||
1. **Has approving review** - Either:
|
||||
- Formal review with "APPROVED" status
|
||||
- Comment containing explicit approval keywords (including self-approval)
|
||||
- Comment containing explicit approval keywords
|
||||
- Self-approval via comment from PR author
|
||||
|
||||
2. **All CI checks pass** - No failing or pending checks
|
||||
@@ -719,7 +719,7 @@ Will check again in 5 minutes for newly ready PRs.
|
||||
|
||||
```markdown
|
||||
✅ **Automatically merged** (verified) - All requirements satisfied:
|
||||
- ✓ Approval from {approver} ({formal review|comment approval|self-approval})
|
||||
- ✓ Approval from {approver} ({formal review|comment approval})
|
||||
- ✓ All CI checks passing
|
||||
- ✓ No merge conflicts
|
||||
- ✓ No blocking labels
|
||||
|
||||
@@ -463,7 +463,7 @@ Supervisor: PR Review | Agent: pr-reviewer
|
||||
```
|
||||
|
||||
**Step 2 — Formal review** (`forgejo_create_pull_review` with `state: "APPROVED"`):
|
||||
Post the same body text. If Forgejo returns an error about self-approval, log it
|
||||
Post the same body text. Since you authenticate as the reviewer account, this should succeed
|
||||
and move on — the issue comment from step 1 is the authoritative record.
|
||||
|
||||
#### REQUEST CHANGES
|
||||
|
||||
@@ -185,14 +185,14 @@ checks must pass and at least 1 approval is required before merge.
|
||||
- **Required status check context** — `["status-check"]` (the consolidation
|
||||
job that verifies ALL CI jobs passed)
|
||||
- **Require at least 1 review approval** — `required_approvals: 1` (All PRs
|
||||
require 1 approval; self-approval is permitted including for bot PRs)
|
||||
require 1 formal review approval)
|
||||
- **Dismiss stale reviews** — `dismiss_stale_approvals: true`
|
||||
- **Block on rejected reviews** — `block_on_rejected_reviews: true`
|
||||
- **Disallow direct pushes** — `enable_push: false` (all changes via PR)
|
||||
|
||||
**Note on approval requirements:** All PRs require exactly 1 approval before
|
||||
merge. This can be a formal review with "APPROVED" status, an approval comment,
|
||||
or self-approval. HAL9000 and other bots are permitted to approve their own PRs.
|
||||
merge. The system uses dual bot accounts — PRs are created by the primary bot
|
||||
and reviewed by a separate reviewer bot, so formal APPROVED reviews work natively.
|
||||
|
||||
**CRITICAL:** The `force_merge` API flag can bypass these rules. The agents
|
||||
are instructed to NEVER use it, but branch protection is the server-side
|
||||
|
||||
@@ -41,7 +41,7 @@ REQUIRED_CREDENTIALS = {
|
||||
'usage': 'Web UI access for CI logs when API unavailable'
|
||||
},
|
||||
|
||||
# === Reviewer Bot Account (reviews only — separate identity for self-approval) ===
|
||||
# === Reviewer Bot Account (reviews only — separate Forgejo user) ===
|
||||
'FORGEJO_REVIEWER_PAT': {
|
||||
'description': 'Forgejo Personal Access Token (reviewer bot account)',
|
||||
'format': r'^[a-f0-9]{40}$', # 40 hex characters
|
||||
@@ -52,11 +52,17 @@ REQUIRED_CREDENTIALS = {
|
||||
'format': r'^[a-zA-Z0-9_-]+$', # Alphanumeric with _ and -
|
||||
'usage': 'Reviewer identity — MUST be a different user than FORGEJO_USERNAME to allow formal APPROVED reviews on bot-created PRs'
|
||||
},
|
||||
'FORGEJO_REVIEWER_PASSWORD': {
|
||||
'description': 'Forgejo password (reviewer bot account)',
|
||||
'format': r'^.{8,}$', # At least 8 characters
|
||||
'usage': 'Web UI access for CI logs via ci-log-fetcher when invoked by the PR Reviewer'
|
||||
},
|
||||
}
|
||||
|
||||
# NOTE: The dual-account architecture solves the "shared bot account problem" (spec Section 6.17).
|
||||
# The primary account creates PRs. The reviewer account reviews them. Since they are different
|
||||
# Forgejo users, formal APPROVED reviews work natively without workarounds.
|
||||
# NOTE: The dual-account architecture (spec Section 6.17) uses two Forgejo accounts:
|
||||
# - Primary account (FORGEJO_*): creates PRs, issues, pushes code, merges
|
||||
# - Reviewer account (FORGEJO_REVIEWER_*): reviews PRs, posts review comments
|
||||
# Since they are different Forgejo users, formal APPROVED reviews work natively.
|
||||
|
||||
OPTIONAL_CREDENTIALS = {
|
||||
'CA_MAX_PARALLEL_WORKERS': {
|
||||
|
||||
@@ -80,14 +80,13 @@ def check_flexible_approval(owner, repo, pr_number):
|
||||
Flexible approval detection that checks formal reviews, review bodies,
|
||||
and issue comments for approval signals.
|
||||
|
||||
IMPORTANT: Forgejo may block formal APPROVED reviews when the reviewer
|
||||
is the same user who created the PR ("approve your own pull is not
|
||||
allowed"). Because all automated agents share a single bot account,
|
||||
formal APPROVED reviews will often be impossible. Therefore this
|
||||
function MUST also accept:
|
||||
The system uses dual bot accounts: PRs are created by the primary bot
|
||||
(FORGEJO_USERNAME) and reviewed by a separate reviewer bot
|
||||
(FORGEJO_REVIEWER_USERNAME). Formal APPROVED reviews are the primary
|
||||
approval path since the accounts are different users. This function
|
||||
also checks fallback signals:
|
||||
- Review bodies (state=COMMENT) that contain approval keywords
|
||||
- Issue comments that contain approval keywords
|
||||
These count as valid approvals even when posted by the PR author.
|
||||
|
||||
Returns approval status with details about what was found.
|
||||
"""
|
||||
@@ -127,9 +126,8 @@ def check_flexible_approval(owner, repo, pr_number):
|
||||
return approval_info
|
||||
|
||||
# 2. Check review BODIES for approval keywords.
|
||||
# When Forgejo rejects a formal APPROVED review (e.g. self-approval
|
||||
# blocked), the pr-reviewer agent falls back to posting a COMMENT
|
||||
# review whose body contains approval language. We must detect this.
|
||||
# Fallback path: if a review was posted as COMMENT state (e.g. from
|
||||
# a human or edge case), check the body for approval language.
|
||||
review_body_approvals = []
|
||||
for review in reviews:
|
||||
if review.get('state') in ('COMMENT', 'PENDING') and review.get('body'):
|
||||
|
||||
Reference in New Issue
Block a user