docs: dual-account PR review arch, ATM interval centralization, reviewer rename (Cycle 13)

CHANGELOG.md

@@ -7,6 +7,41 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Added
 
+- **Dual-Account PR Review Architecture**: Introduced a separate Forgejo reviewer
+  account (`FORGEJO_REVIEWER_PAT` / `FORGEJO_REVIEWER_USERNAME` /
+  `FORGEJO_REVIEWER_PASSWORD`) to solve the shared-bot-account problem where a
+  single account cannot approve its own PRs. The `pr-reviewer` agent now uses
+  reviewer credentials for formal `APPROVED` reviews via curl (the primary
+  approval path). The `pr-review-pool-supervisor` and `product-builder` pass
+  reviewer credentials when dispatching reviewers. The `pr-merge-pool-supervisor`
+  recognises the reviewer account as the primary approver.
+
+- **ATM-Centralized Cycle Interval Calculation**: The
+  `automation-tracking-manager` now handles rolling-average cycle interval
+  calculation internally via the `--sleep-interval-default` parameter. All 17
+  supervisor agents have had their duplicated 7-line interval calculation blocks
+  removed; they now pass a single default and let the ATM compute the actual
+  interval. A new `CYCLE_ANNOUNCEMENT_REVIEW` operation (#11) was added to the
+  ATM operation set.
+
+- **Curl-Based Review Authentication**: Because the Forgejo MCP server
+  authenticates with a single server-level token that cannot be overridden
+  per-call, the `pr-reviewer` agent now uses `curl` with `FORGEJO_REVIEWER_PAT`
+  for all write operations (reviews, comments). MCP tools are used only for
+  read operations. The `pr-review-pool-supervisor` documents the
+  READ-via-MCP / WRITE-via-curl split.
+
+### Changed
+
+- **PR Reviewer Renamed**: `pr-self-reviewer` renamed to `pr-reviewer`
+  throughout the agent system. All references to "PR Self-Reviewer" and
+  "self-reviewer" updated to "PR Reviewer" in agent definitions, spec, and
+  coordination sections.
+
+- **Staleness Threshold Increased**: `system-watchdog-pool-supervisor` now
+  uses a 2× expected interval as the staleness threshold (previously 1.2×),
+  reducing false-positive stall detections for agents with variable cycle times.
+
 - **Git Worktree Sandbox Apply** (#4454): The `plan apply` command now merges
   LLM-generated changes via `git merge` from an isolated worktree branch
   instead of flat `shutil.copy2`.  Displays spec-aligned Apply Summary

docs/development/automation-tracking.md

@@ -90,18 +90,55 @@ The `automation-tracking-manager` subagent:
 4. Applies all required labels atomically
 5. Returns the new issue number to the calling agent
 
+### Cycle Interval Calculation (ATM-Centralized)
+
+As of the Subagent Specialization Principle refactor, cycle interval calculation
+is **centralized in the ATM**. Agents no longer compute rolling averages
+themselves. Instead they pass a `--sleep-interval-default` parameter to
+`CREATE_TRACKING_ISSUE` and the ATM injects the computed
+`**Estimated Cycle Interval**: Nmin` field into the issue body automatically.
+
+This eliminated 17 duplicated 7-line calculation blocks across all supervisor
+agents.
+
+### Supported Operations
+
+| Operation | Description |
+|-----------|-------------|
+| `READ_TRACKING_STATE` | Recover cycle number, timestamps, and interval from previous session |
+| `CREATE_TRACKING_ISSUE` | Close old issue, create new one with correct cycle number and labels |
+| `UPDATE_TRACKING_ISSUE` | Add a comment to the current tracking issue |
+| `CREATE_ANNOUNCEMENT_ISSUE` | Create a cross-agent announcement with priority |
+| `CLOSE_ANNOUNCEMENT_ISSUE` | Close an announcement when no longer relevant |
+| `LIST_TRACKING_ISSUES` | List all current tracking and announcement issues |
+| `READ_ANNOUNCEMENTS` | Read announcements from other agents (cross-agent awareness) |
+| `REVIEW_OWN_ANNOUNCEMENTS` | Review and clean up own announcements |
+| `CYCLE_ANNOUNCEMENT_REVIEW` | Periodic review of announcements during normal cycle |
+
 ### Usage
 
-Agents that use the centralized manager delegate all tracking operations to it:
+Agents that use the centralized manager delegate all tracking operations to it.
+The **critical startup order** is: READ state first, then CREATE new issue.
 
 ```bash
-# Example: implementation-pool-supervisor delegating to tracking manager
-# (Inside agent definition — pseudocode)
-TRACKING_RESULT=$(call_subagent automation-tracking-manager \
-  --prefix "AUTO-IMP-POOL" \
-  --type "Health Report" \
-  --body "...")
-NEW_ISSUE_NUMBER=$(echo "$TRACKING_RESULT" | jq -r '.issue_number')
+# STEP 1 (ON STARTUP): Read state from previous session BEFORE creating new
+recovered_state=$(task automation-tracking-manager "READ_TRACKING_STATE" \
+  --agent-prefix "AUTO-IMP-POOL" \
+  --tracking-type "Health Report" \
+  --repo-owner "cleveragents" \
+  --repo-name "cleveragents-core")
+# Returns: cycle_number, created_at, offline_duration_minutes,
+#          estimated_cycle_interval, issue_body, comments
+
+# STEP 2: Create new tracking issue (closes ALL old status issues first)
+task automation-tracking-manager "CREATE_TRACKING_ISSUE" \
+  --agent-prefix "AUTO-IMP-POOL" \
+  --tracking-type "Health Report" \
+  --body "$tracking_body" \
+  --sleep-interval-default 20 \
+  --repo-owner "cleveragents" \
+  --repo-name "cleveragents-core"
+# ATM automatically injects "**Estimated Cycle Interval**: Nmin" into the body
 ```
 
 ### Migrated Agents
@@ -210,15 +247,23 @@ When a stalled agent is detected:
 # Pseudocode for health monitoring
 for issue in automation_tracking_issues:
     if issue.title.matches("[AUTO-*] * (Cycle *)"):
-        expected_interval = get_expected_interval(issue.agent_prefix, issue.type)
+        # Interval is now parsed from the issue body's
+        # "Estimated Cycle Interval" field (ATM-injected)
+        expected_interval = parse_interval_from_body(issue.body)
         time_since_creation = now() - issue.created_at
-        staleness_threshold = expected_interval * 1.2  # 20% tolerance
+        staleness_threshold = expected_interval * 2.0  # 2× tolerance
         
         if time_since_creation > staleness_threshold:
             mark_agent_as_stalled(issue.agent_prefix)
             trigger_recovery_actions(issue.agent_prefix, issue)
 ```
 
+> **Note**: The staleness threshold was increased from 1.2× to 2× to reduce
+> false-positive stall detections for agents with variable cycle times. The
+> expected interval is now parsed from the `**Estimated Cycle Interval**` field
+> in the issue body (injected by the ATM) rather than looked up from a hardcoded
+> table.
+
 ## Standardized Tracking Issue Templates
 
 ### Common Header Format
@@ -482,6 +527,43 @@ A second migration introduced the centralized `automation-tracking-manager` suba
 to prevent cycle reuse issues where agents were commenting on old tracking issues
 instead of creating new ones.
 
+## Dual-Account PR Review Architecture
+
+The agent system uses two separate Forgejo accounts to handle the constraint that
+a single account cannot approve its own pull requests.
+
+### Accounts
+
+| Account | Purpose | Credentials |
+|---------|---------|-------------|
+| **Bot account** (`HAL9000`) | Creates PRs, reads issues, general automation | `FORGEJO_PAT` |
+| **Reviewer account** | Submits formal `APPROVED` reviews on bot PRs | `FORGEJO_REVIEWER_PAT`, `FORGEJO_REVIEWER_USERNAME`, `FORGEJO_REVIEWER_PASSWORD` |
+
+### MCP Token Limitation
+
+The Forgejo MCP server authenticates with a **single server-level token** that
+cannot be overridden per-call. This means:
+
+- **Read operations** (list PRs, get issues, etc.) → use MCP tools (bot account)
+- **Write operations** (submit reviews, post approval comments) → use `curl` with
+  `FORGEJO_REVIEWER_PAT` (reviewer account)
+
+```bash
+# Example: submit a formal APPROVED review as the reviewer account
+curl -s -X POST \
+  -H "Authorization: token ${FORGEJO_REVIEWER_PAT}" \
+  -H "Content-Type: application/json" \
+  "https://git.cleverthis.com/api/v1/repos/${OWNER}/${REPO}/pulls/${PR_NUMBER}/reviews" \
+  -d '{"event":"APPROVED","body":"LGTM — automated review"}'
+```
+
+### Credential Propagation
+
+The `product-builder` passes all three reviewer credentials when launching the
+`pr-review-pool-supervisor`, which in turn passes them to each dispatched
+`pr-reviewer` worker. The `pr-merge-pool-supervisor` recognises the reviewer
+account as the primary approver when checking merge readiness.
+
 ## Related Documentation
 
 - [System Watchdog](system-watchdog.md) - Specific documentation for watchdog agent

docs/development/review_playbook.md

@@ -294,3 +294,44 @@ A PR may only be merged if **all** of the following are true:
 5. `nox -s security_scan` reports zero high/critical findings
 6. At least one reviewer has approved
 7. All P0 and P1 findings are resolved
+
+---
+
+## Automated PR Review (Dual-Account Architecture)
+
+The `pr-reviewer` agent performs automated code reviews using a **separate
+reviewer Forgejo account** to satisfy the constraint that a single account
+cannot approve its own pull requests.
+
+### How It Works
+
+1. **Bot account** (`HAL9000`) creates the PR using `FORGEJO_PAT` / MCP tools.
+2. **Reviewer account** submits a formal `APPROVED` review using
+   `FORGEJO_REVIEWER_PAT` via `curl` (MCP cannot override the server-level token).
+
+### MCP Token Limitation
+
+The Forgejo MCP server uses a single server-level token. The `pr-reviewer`
+agent therefore splits operations:
+
+| Operation | Method | Credential |
+|-----------|--------|-----------|
+| Read PR details, list files | MCP tools | `FORGEJO_PAT` (bot) |
+| Submit `APPROVED` review | `curl` POST | `FORGEJO_REVIEWER_PAT` (reviewer) |
+| Post approval comment | `curl` POST | `FORGEJO_REVIEWER_PAT` (reviewer) |
+
+### Approval Forms
+
+The `pr-merge-pool-supervisor` recognises the following as valid approvals:
+
+- Formal `APPROVED` review from the reviewer account (primary path)
+- Comment containing `LGTM`, `✅`, `Approved`, or `"ready to merge"` from the
+  reviewer account (fallback)
+
+### Required Credentials
+
+| Variable | Description |
+|----------|-------------|
+| `FORGEJO_REVIEWER_PAT` | Personal access token for the reviewer account |
+| `FORGEJO_REVIEWER_USERNAME` | Username of the reviewer account |
+| `FORGEJO_REVIEWER_PASSWORD` | Password for the reviewer account (used by `pr-review-pool-supervisor` for stuck-PR recovery) |