33f1978bd0
Root cause: actions/checkout@v4 was not configured with explicit write credentials (token + persist-credentials), and no git user identity (user.name/user.email) was set. Both are required for any git push operation in Forgejo Actions. Changes: - release.yml create-release job: add token: secrets.FORGEJO_TOKEN and fetch-depth: 0 to checkout; add 'Configure git identity for push operations' step using HTTPS credential store; add 'Smoke-test push access' step that validates write permission via Forgejo API before any push attempt - ci.yml: add push-validation job that validates push credentials on every CI run using FORGEJO_TOKEN, including credential helper verification and API-based write permission check; add push-validation to status-check needs and result reporting - docs/development/ci-cd.md: add FORGEJO_TOKEN, FORGEJO_URL, and CONTAINER_REGISTRY* secrets to the secrets table; add 'Repository Push Authentication' section documenting root cause, fix pattern, smoke-test step, setup instructions, and security notes; add push-validation to CI job dependency graph and quality gates table Design decisions: - HTTPS token authentication (not SSH deploy keys) -- simpler to manage - ~/.git-credentials with chmod 600 for ephemeral, secure storage - Smoke-test validates write permission via API before push attempts - push-validation job is independent (no needs) -- runs in parallel - No hardcoded credentials -- all secrets via Forgejo Secrets ISSUES CLOSED: #1541
402 lines
16 KiB
Markdown
402 lines
16 KiB
Markdown
# CI/CD Pipeline and Branch Protection
|
|
|
|
This document describes the CI/CD pipeline, branch protection rules, and merge
|
|
requirements for the CleverAgents project. For quality tooling details (pre-commit
|
|
hooks, nox sessions, security scanning), see
|
|
[Quality Automation Guide](quality-automation.md).
|
|
|
|
## Branch Protection Rules
|
|
|
|
### Protected Branch: `master`
|
|
|
|
The `master` branch is the primary integration branch. All changes must go through
|
|
pull requests with the following protections enforced.
|
|
|
|
#### Required Status Checks
|
|
|
|
Every pull request targeting `master` must pass **all** of the following CI jobs
|
|
before merging is allowed:
|
|
|
|
| CI Job | What It Checks | Failure Means |
|
|
|--------|---------------|---------------|
|
|
| `lint` | Ruff format and lint (nox) | Code style violations or lint errors |
|
|
| `typecheck` | Pyright strict type checking (nox) | Type errors in `src/` |
|
|
| `security` | Security scan + dead code (nox) | Security vulnerabilities or dead code |
|
|
| `quality` | Radon complexity (nox) | Extremely complex methods (31+ cyclomatic complexity) |
|
|
| `unit_tests` | BDD unit tests via `nox -s unit_tests` | Failing unit test scenarios |
|
|
| `integration_tests` | Robot integration tests via `nox -s integration_tests` | Failing integration tests |
|
|
| `coverage` | Test coverage measurement (nox) | Coverage dropped below 97% |
|
|
| `build` | Wheel build | Build failure |
|
|
|
|
**Deployment-gating jobs** (run after core checks pass):
|
|
|
|
| CI Job | Depends On | What It Checks |
|
|
|--------|-----------|----------------|
|
|
| `docker` | `lint`, `typecheck`, `unit_tests`, `security` | Docker image builds and runs |
|
|
|
|
#### Required Reviews
|
|
|
|
- **Minimum 1 approving review** required before merge.
|
|
- Reviews are selective (see [Review Priority Matrix](#review-priority-matrix) below).
|
|
- Stale reviews are dismissed when new commits are pushed.
|
|
- Code owners can be configured per directory if needed.
|
|
|
|
#### Additional Protections
|
|
|
|
- **No direct pushes** to `master`. All changes must come via pull request.
|
|
The `no-commit-to-branch` pre-commit hook enforces this locally.
|
|
- **No force pushes** to `master`. History must be preserved.
|
|
- **No deletions** of the `master` branch.
|
|
- **Require branches to be up-to-date** before merging (prevents merge skew).
|
|
|
|
### Setting Up Branch Protection in Forgejo
|
|
|
|
To configure these rules in Forgejo:
|
|
|
|
1. Navigate to **Repository Settings** > **Branches** > **Branch Protection**.
|
|
2. Add a rule for branch name pattern: `master`.
|
|
3. Enable the following settings:
|
|
|
|
```
|
|
[x] Enable branch protection
|
|
[x] Block push (no direct pushes)
|
|
[x] Block force push
|
|
[x] Block branch deletion
|
|
[x] Require pull request reviews before merging
|
|
- Required approvals: 1
|
|
- Dismiss stale reviews: Yes
|
|
[x] Require status checks to pass before merging
|
|
- Required checks:
|
|
- lint
|
|
- typecheck
|
|
- security
|
|
- quality
|
|
- behave
|
|
- coverage
|
|
- build
|
|
[x] Require branches to be up-to-date before merging
|
|
```
|
|
|
|
4. Save the branch protection rule.
|
|
|
|
## Review Process
|
|
|
|
### Review Priority Matrix
|
|
|
|
Not all code changes require the same level of review attention. Use this matrix
|
|
to determine review depth:
|
|
|
|
| Priority | Category | Review Depth | Examples |
|
|
|----------|----------|-------------|----------|
|
|
| **P0** | Architecture and Security | Deep review required | Service layer design, DB schema, API contracts, sandbox boundaries, authentication |
|
|
| **P1** | Complex Business Logic | Careful review | Decision tree traversal, merge conflict resolution, dependency closure, state machines |
|
|
| **P2** | Normal Features | Standard review | CLI commands, model definitions, straightforward service methods |
|
|
| **P3** | Tests, Docs, Refactoring | Trust automation | Behave scenarios, documentation updates, import reorganization, formatting |
|
|
|
|
### What Reviewers Should Focus On
|
|
|
|
**Always review** (P0 items):
|
|
|
|
- Service layer design choices and dependency injection patterns
|
|
- Database schema decisions and Alembic migrations
|
|
- API contract definitions and interface boundaries
|
|
- Sandbox security boundaries and isolation guarantees
|
|
- Input validation and sanitization logic
|
|
- Any code touching `eval()`, `exec()`, `compile()`, or `subprocess`
|
|
|
|
**Review carefully** (P1 items):
|
|
|
|
- Algorithm implementations (decision tree, merge, closure computation)
|
|
- State machine transitions and lifecycle management
|
|
- Error propagation across actor and plan boundaries
|
|
- Concurrent access patterns and thread safety
|
|
|
|
**Standard review** (P2 items):
|
|
|
|
- Verify tests exist for new functionality
|
|
- Check that argument validation follows `CONTRIBUTING.md` guidelines
|
|
- Confirm type annotations are complete (no bare `Any`)
|
|
|
|
**Trust automation** (P3 items):
|
|
|
|
- If `nox` passes (lint, typecheck, tests, coverage, security), formatting and
|
|
style are covered.
|
|
- Behave test scenarios and Robot integration tests follow established patterns.
|
|
- Documentation changes are verified by `nox -s docs` (MkDocs build).
|
|
|
|
### Review Checklist
|
|
|
|
Every PR should pass this checklist (also available in the
|
|
PR template at `.forgejo/pull_request_template.md` in the repository root):
|
|
|
|
- [ ] Code follows `CONTRIBUTING.md` coding standards
|
|
- [ ] All public/protected methods have argument validation
|
|
- [ ] Static typing is complete (no bare `Any` unless justified)
|
|
- [ ] `nox -s typecheck` passes
|
|
- [ ] `nox -s lint` passes
|
|
- [ ] Unit tests written/updated (Behave scenarios in `features/`)
|
|
- [ ] Integration tests written/updated (Robot suites in `robot/`) if applicable
|
|
- [ ] Coverage remains above 97%
|
|
- [ ] No security issues introduced
|
|
- [ ] No dead code introduced
|
|
- [ ] Documentation updated if behavior changed
|
|
|
|
## CI Pipeline Reference
|
|
|
|
### Workflow Files
|
|
|
|
| Workflow | File | Trigger | Purpose |
|
|
|----------|------|---------|---------|
|
|
| CI | `.forgejo/workflows/ci.yml` | Push to `master`/`develop`, PRs to `master` | Full nox-based validation pipeline |
|
|
| Nightly Quality | `.forgejo/workflows/nightly-quality.yml` | Midnight UTC (cron), manual | Comprehensive quality monitoring |
|
|
|
|
### CI Job Dependency Graph
|
|
|
|
```
|
|
lint ──────────────────┐
|
|
typecheck ─────────────┤
|
|
├── coverage (needs lint + typecheck)
|
|
security ──────────────┤
|
|
├── docker (needs lint + typecheck + unit_tests + security)
|
|
unit_tests ────────────┘
|
|
integration_tests ──────── (independent)
|
|
quality ────────────────── (independent)
|
|
build ──────────────────── (independent)
|
|
push-validation ────────── (independent — validates CI push credentials)
|
|
```
|
|
|
|
### Nox-Based CI
|
|
|
|
All CI jobs now run their checks through nox sessions rather than invoking
|
|
tools directly. This ensures that the CI environment matches local development
|
|
exactly. Each job runs `pip install uv nox` and then delegates to the
|
|
appropriate nox session.
|
|
|
|
### Quality Gates Summary
|
|
|
|
All gates must pass for a PR to be mergeable:
|
|
|
|
| Gate | Threshold | Enforced By |
|
|
|------|-----------|-------------|
|
|
| Formatting | Zero violations | `lint` job (ruff format) |
|
|
| Linting | Zero violations | `lint` job (ruff check) |
|
|
| Type Safety | Zero errors | `typecheck` job (pyright strict) |
|
|
| Security | Zero HIGH severity | `security` job (bandit) |
|
|
| Dead Code | Zero findings | `security` job (vulture) |
|
|
| Complexity | No grade-F methods | `quality` job (radon) |
|
|
| Unit Tests | All pass (3.11-3.13) | `behave` job |
|
|
| Coverage | >= 97% | `coverage` job (nox) |
|
|
| Build | Wheel builds | `build` job |
|
|
| Push Access | Credentials valid | `push-validation` job |
|
|
|
|
### Nightly Quality Monitoring
|
|
|
|
The nightly workflow provides trend data beyond PR-level checks:
|
|
|
|
- Full lint, type check, and security scan (all severities, not just HIGH)
|
|
- Complete Behave test suite with coverage measurement
|
|
- Complexity and maintainability index analysis
|
|
- Quality trend JSON with timestamps (90-day artifact retention)
|
|
|
|
Reports are uploaded as artifacts under `nightly-quality-reports` and can be
|
|
downloaded from the Forgejo Actions UI.
|
|
|
|
## Local Development Workflow
|
|
|
|
### Before Pushing a PR
|
|
|
|
```bash
|
|
# Quick validation (runs default nox sessions: lint, format, typecheck,
|
|
# unit_tests, integration_tests, docs, build, benchmark, coverage_report)
|
|
nox
|
|
|
|
# Or run individual checks matching CI jobs
|
|
nox -s lint # Formatting and linting (CI: lint job)
|
|
nox -s format -- --check # Format check only (CI: lint job)
|
|
nox -s typecheck # Type checking (CI: typecheck job)
|
|
nox -s unit_tests # Behave tests (CI: unit_tests job)
|
|
nox -s integration_tests # Robot tests (CI: integration_tests job)
|
|
nox -s coverage_report # Coverage (must be >=97%) (CI: coverage job)
|
|
nox -s security_scan # Bandit security scanning (CI: security job)
|
|
nox -s dead_code # Vulture dead code detection (CI: security job)
|
|
nox -s complexity # Radon complexity check (CI: quality job)
|
|
nox -s build # Wheel build (CI: build job)
|
|
```
|
|
|
|
### Local Reproduction of CI Failures
|
|
|
|
To reproduce a CI failure locally, run the exact nox session that failed:
|
|
|
|
```bash
|
|
# Example: coverage job failed
|
|
nox -s coverage_report
|
|
|
|
# Example: typecheck job failed
|
|
nox -s typecheck
|
|
|
|
# Example: security job failed
|
|
nox -s security_scan
|
|
nox -s dead_code
|
|
```
|
|
|
|
### Caching Notes
|
|
|
|
CI jobs install `uv` (for fast dependency resolution) and `nox`. The nox
|
|
sessions use `uv` as the venv backend (`venv_backend="uv"`) and reuse
|
|
existing virtualenvs (`reuse_venv=True`) for speed. On CI, each job starts
|
|
fresh but `uv` provides fast installs via its built-in caching.
|
|
|
|
### CI Secrets
|
|
|
|
Some CI jobs require secrets configured in the Forgejo UI. Secrets are managed
|
|
under **Repository Settings > Actions > Secrets** and are automatically masked
|
|
in job logs.
|
|
|
|
| Secret Name | Required By | Purpose |
|
|
|-------------|-------------|---------|
|
|
| `ANTHROPIC_API_KEY` | `integration_tests` | Anthropic API key for Robot Framework integration tests |
|
|
| `OPENAI_API_KEY` | `integration_tests` | OpenAI API key for Robot Framework integration tests |
|
|
| `AWS_ACCESS_KEY_ID` | `benchmark-regression`, `benchmark-publish` | AWS credentials for ASV benchmark S3 storage |
|
|
| `AWS_SECRET_ACCESS_KEY` | `benchmark-regression`, `benchmark-publish` | AWS credentials for ASV benchmark S3 storage |
|
|
| `AWS_DEFAULT_REGION` | `benchmark-regression`, `benchmark-publish` | AWS region for ASV benchmark S3 storage |
|
|
| `ASV_S3_BUCKET` | `benchmark-regression`, `benchmark-publish` | S3 bucket name for ASV benchmark storage |
|
|
| `FORGEJO_TOKEN` | `release` (`create-release` job) | Forgejo API token with **repository write** scope — used to authenticate `git push` and create releases |
|
|
| `FORGEJO_URL` | `release` (`create-release` job) | Base URL of the Forgejo instance (e.g., `https://git.cleverthis.com`) |
|
|
| `CONTAINER_REGISTRY` | `release` (`build-docker` job) | Container registry URL for Docker image pushes |
|
|
| `CONTAINER_REGISTRY_USER` | `release` (`build-docker` job) | Username for container registry authentication |
|
|
| `CONTAINER_REGISTRY_PASSWORD` | `release` (`build-docker` job) | Password/token for container registry authentication |
|
|
|
|
**Unit tests vs. integration tests:**
|
|
|
|
- **Unit tests** (Behave BDD, `nox -s unit_tests`) do **not** require any API
|
|
keys. They test internal logic without calling external services.
|
|
- **Integration tests** (Robot Framework, `nox -s integration_tests`) **do**
|
|
require real LLM API keys (`ANTHROPIC_API_KEY`, `OPENAI_API_KEY`). These
|
|
tests exercise end-to-end workflows that interact with live LLM providers.
|
|
|
|
If the LLM secrets are not configured, integration tests will fail with
|
|
authentication errors at runtime.
|
|
|
|
### Repository Push Authentication
|
|
|
|
Any CI workflow step that writes back to the repository (e.g., pushing tags,
|
|
committing auto-generated files, or creating changelog commits) **must** use
|
|
explicit token authentication. The `push-validation` job in `ci.yml` validates
|
|
that push credentials are correctly configured on every CI run.
|
|
|
|
**Fix applied (issue #1541):** The `actions/checkout@v4` action was not
|
|
configured with `token: ${{ forgejo.token }}` and `persist-credentials: true`,
|
|
and no git user config (`user.name` / `user.email`) was set. Both are required
|
|
for push operations. The `push-validation` job now validates these on every run.
|
|
|
|
#### Root Cause of "Unable to Push" Failures
|
|
|
|
The most common cause of CI push failures is missing or misconfigured
|
|
credentials. Symptoms include:
|
|
|
|
```
|
|
remote: Permission denied
|
|
fatal: unable to access 'https://...': The requested URL returned error: 403
|
|
```
|
|
|
|
or:
|
|
|
|
```
|
|
ERROR: Repository not found.
|
|
fatal: Could not read from remote repository.
|
|
```
|
|
|
|
#### Fix: HTTPS Token Authentication
|
|
|
|
The canonical fix is to configure git to use the `FORGEJO_TOKEN` secret via
|
|
HTTPS credential store. This is done in the `create-release` job of
|
|
`release.yml`:
|
|
|
|
```yaml
|
|
- uses: actions/checkout@v4
|
|
with:
|
|
fetch-depth: 0
|
|
token: ${{ secrets.FORGEJO_TOKEN }} # Use write-scoped token
|
|
|
|
- name: Configure git identity for push operations
|
|
env:
|
|
FORGEJO_URL: ${{ secrets.FORGEJO_URL }}
|
|
FORGEJO_TOKEN: ${{ secrets.FORGEJO_TOKEN }}
|
|
run: |
|
|
git config user.name "CleverAgents CI"
|
|
git config user.email "ci@cleverthis.com"
|
|
FORGEJO_HOST=$(echo "${FORGEJO_URL}" | sed 's|https\?://||' | cut -d/ -f1)
|
|
git config credential.helper store
|
|
echo "https://ci:${FORGEJO_TOKEN}@${FORGEJO_HOST}" > ~/.git-credentials
|
|
chmod 600 ~/.git-credentials
|
|
```
|
|
|
|
#### Smoke-Test Step
|
|
|
|
The `create-release` job includes a smoke-test step that validates push access
|
|
**before** attempting the real push. This catches credential issues early with
|
|
a clear error message:
|
|
|
|
```yaml
|
|
- name: Smoke-test push access
|
|
env:
|
|
FORGEJO_URL: ${{ secrets.FORGEJO_URL }}
|
|
FORGEJO_TOKEN: ${{ secrets.FORGEJO_TOKEN }}
|
|
run: |
|
|
REPO="${{ forgejo.repository }}"
|
|
API_URL="${FORGEJO_URL}/api/v1/repos/${REPO}"
|
|
HTTP_STATUS=$(curl -s -o /dev/null -w "%{http_code}" \
|
|
-H "Authorization: token ${FORGEJO_TOKEN}" \
|
|
"${API_URL}")
|
|
if [ "${HTTP_STATUS}" != "200" ]; then
|
|
echo "ERROR: FORGEJO_TOKEN cannot access repository API (HTTP ${HTTP_STATUS})."
|
|
exit 1
|
|
fi
|
|
PUSH_ALLOWED=$(curl -s \
|
|
-H "Authorization: token ${FORGEJO_TOKEN}" \
|
|
"${API_URL}" | python3 -c "import sys,json; d=json.load(sys.stdin); print(str(d.get('permissions',{}).get('push',False)).lower())")
|
|
if [ "${PUSH_ALLOWED}" != "true" ]; then
|
|
echo "ERROR: FORGEJO_TOKEN does not have push (write) permission."
|
|
exit 1
|
|
fi
|
|
echo "Push access verified."
|
|
```
|
|
|
|
#### Setting Up the FORGEJO_TOKEN Secret
|
|
|
|
1. Create a Forgejo personal access token (PAT) with **repository write** scope:
|
|
- Navigate to **User Settings** > **Applications** > **Access Tokens**
|
|
- Create a token with `repository` scope (read + write)
|
|
- Copy the token value (shown only once)
|
|
|
|
2. Add the token as a repository secret:
|
|
- Navigate to **Repository Settings** > **Actions** > **Secrets**
|
|
- Add secret `FORGEJO_TOKEN` with the PAT value
|
|
- Add secret `FORGEJO_URL` with the Forgejo base URL (e.g., `https://git.cleverthis.com`)
|
|
|
|
3. **Never hardcode tokens** in workflow files. Always use `${{ secrets.SECRET_NAME }}`.
|
|
|
|
#### Security Notes
|
|
|
|
- The `FORGEJO_TOKEN` secret is automatically masked in job logs by Forgejo Actions.
|
|
- The `~/.git-credentials` file is created with `chmod 600` (owner-read-only).
|
|
- The credential file is ephemeral — it exists only for the duration of the job
|
|
in the container's filesystem and is destroyed when the container exits.
|
|
- No SSH deploy keys are required; HTTPS token authentication is sufficient and
|
|
simpler to manage.
|
|
|
|
### Pre-commit Hooks
|
|
|
|
Pre-commit hooks run automatically on `git commit` and catch most issues before
|
|
they reach CI. See [Quality Automation Guide](quality-automation.md) for the
|
|
full list of hooks.
|
|
|
|
If hooks are not installed:
|
|
|
|
```bash
|
|
bash scripts/setup-dev.sh
|
|
# Or manually:
|
|
pre-commit install
|
|
pre-commit install --hook-type commit-msg
|
|
```
|