GRM-33: feat: add mandatory PR review step to workflow

AGENTS.md

@@ -16,17 +16,103 @@ make test-all       # pytest-cov + molecule
 
 - **Python CLI** (`src/gitea_runner_manager/`) — Click-based CLI that delegates to Ansible
 - **Ansible Role** (`ansible/roles/gitea-runner/`) — Idempotent role for rootless Docker runner setup
-- **CI Scripts** (`scripts/`) — Automation for auto-merge, post-merge, publishing, molecule distribution
+- **CI Scripts** (`scripts/`) — Automation for auto-merge, post-merge, publishing, molecule distribution, PR reviews
+
+## PR Workflow (Mandatory)
+
+Every change to master goes through this workflow. No exceptions.
+
+### 1. Create Vikunja Task
+Create a task in Vikunja project 6 to get a `GRM-N` identifier.
+
+### 2. Create Branch
+```bash
+git checkout master && git pull
+git checkout -b GRM-N-short-description
+```
+
+### 3. Implement Changes
+- Write code following conventions below
+- Write/update tests (100% coverage required)
+- Update documentation (CHANGELOG, README, AGENTS.md as needed)
+
+### 4. Commit (Conventional Commits)
+Branch commits use conventional commit format (no `GRM-N:` prefix):
+```
+feat: add new feature
+fix: resolve bug
+docs: update README
+```
+
+### 5. Push and Create PR
+- **PR title format**: `GRM-N: <vikunja task title>` (colon-separated)
+- PR body: summary of changes, `Closes GRM-N`
+- Add `ready-to-merge` label **only after review is complete**
+
+### 6. Review the PR (Mandatory — Before Adding ready-to-merge Label)
+Review the full diff (`git diff master...HEAD`) focusing on:
+
+- **Functional completeness**: Does the code do what it claims? Are all requirements met?
+- **Edge cases**: Are boundary conditions, empty inputs, error paths handled?
+- **Technical excellence**:
+  - Architecture compliance and evolution
+  - Single Responsibility Principle (SRP)
+  - Deduplication (no copy-paste, single source of truth)
+  - Code smells detection and removal
+  - Best industry practices
+  - Industry-grade code quality
+  - Reusability
+  - Clean code
+  - Readability
+  - Maintainability
+  - Extensibility
+- **Performance**: No unnecessary allocations, O(n) vs O(n²), efficient data structures
+- **Security**: No secrets in logs/process list, input validation, no injection vectors
+- **User experience**: Clear error messages, intuitive CLI flags, helpful output
+- **Documentation**: Completeness and relevance of docs, CHANGELOG entries, AGENTS.md updates
+
+Post review comments using `scripts/review_pr.py`:
+```bash
+REPO_TOKEN=<token> python3 scripts/review_pr.py <pr_number> <owner/repo> \
+  --event REQUEST_CHANGES \
+  --body "Review summary" \
+  --comments-json comments.json
+```
+
+### 7. Address Review Comments
+Fix each comment one by one, commit, and push. Re-review until satisfied.
+
+### 8. Approve and Merge
+Once all comments are addressed:
+```bash
+REPO_TOKEN=<token> python3 scripts/review_pr.py <pr_number> <owner/repo> \
+  --event APPROVE \
+  --body "All comments addressed. LGTM."
+```
+
+Then add the `ready-to-merge` label. The auto-merge workflow will:
+1. Wait for all CI checks to pass
+2. Squash-merge with title: `GRM-N <conventional commit message>` (space-separated)
+3. The post-merge workflow marks the Vikunja task as done
+
+### Title Format Summary
+
+| What | Format | Example |
+|------|--------|---------|
+| Branch name | `GRM-N-short-description` | `GRM-33-add-pr-review-step` |
+| Branch commits | `<conventional commit>` | `feat: add review script` |
+| PR title | `GRM-N: <vikunja task title>` | `GRM-33: Add mandatory PR review step` |
+| Merge commit | `GRM-N <conventional commit>` | `GRM-33 feat: add review script` |
 
 ## Key Conventions
 
 - Python 3.12+ required (ruff/pyright target `py312`)
 - 100% test coverage required (`--cov-fail-under=100`)
 - Conventional commits on feature branches (no `GRM-N:` prefix)
-- `GRM-N:` prefix on master branch (added by auto-merge)
 - Branch names must include `GRM-N` task ID
 - Line length: 120 chars
 - Secrets are passed via temp JSON files, never on the command line (CWE-214)
+- CI triggers only on `opened` and `synchronize` PR events (not `labeled`)
 
 ## Ansible Role Structure
 
@@ -36,8 +122,8 @@ main.yml → systemd_check → user_setup → rootless_docker → install_runner
 
 - `install_runner.yml` handles: download, config, validate, register, service
 - `main.yml` handles: prune, integration_test (NOT install_runner — avoids duplicates)
-- All `systemctl --user` tasks must be guarded by `docker_rootless_setup`
-- All template creation tasks must be guarded by `docker_rootless_setup`
+- `systemctl --user` tasks must be guarded by `docker_rootless_setup`
+- Template creation tasks are NOT guarded by `docker_rootless_setup` (they just create files)
 
 ## Molecule Scenarios