GRM-165: feat: adopt spec-driven CI gates, create_dependency_pr, and pr-review skill
Co-authored-by: emil User <emil.simeonov@tutanota.com>
This commit was merged in pull request #256.
This commit is contained in:
@@ -61,8 +61,37 @@ CI also runs a best-effort `make workflow-dryrun` step (skipped if act_runner is
|
||||
- **devx package** (installed from git) — Reusable CI/CD tools: auto-merge, post-merge, release, publishing, molecule distribution, PR reviews, failure notifications
|
||||
- **Versioning** (`cliff.toml`) — git-cliff configuration for automated semver versioning from conventional commits
|
||||
|
||||
|
||||
## Spec-Driven Development
|
||||
|
||||
Every change starts with a spec. No spec, no code.
|
||||
|
||||
**Workflow:**
|
||||
1. Create Vikunja task → get `<PREFIX>-N` task ID
|
||||
2. Write spec at `docs/specs/<TASK-ID>.md` (see template in `.devin/skills/spec-driven-development/SKILL.md`)
|
||||
3. Create branch, implement with `# Implements: REQ-N` comments
|
||||
4. Tick all acceptance criteria checkboxes in spec
|
||||
5. Push and create PR — CI validates spec before expensive jobs
|
||||
|
||||
**CI gates (pre-merge):**
|
||||
- `devx.ci.validate_spec` — checks spec exists, has required sections, REQ-IDs, all ACs checked
|
||||
- `devx.ci.check_pr_size` — max 500 lines / 10 files (excludes CHANGELOG, badges, locks)
|
||||
- `devx.ci.fast_molecule` — converge+verify only for changed roles, single platform
|
||||
|
||||
**Nightly (infra only):**
|
||||
- Full molecule suite (all scenarios, all platforms) + staging deploy + integration tests
|
||||
- On failure: sets `NIGHTLY_STATUS=failed`, blocks staging deploys
|
||||
- Post-merge auto-deploy to staging checks this gate before deploying
|
||||
|
||||
**Post-merge:**
|
||||
- Infra: auto-deploys to staging (if nightly gate is green)
|
||||
- GRM/sso-bridge: auto-publishes package, auto-creates infra dependency PR to bump pinned version
|
||||
|
||||
**Skill:** `.devin/skills/spec-driven-development/SKILL.md` — full template and workflow details.
|
||||
|
||||
## PR Workflow (Mandatory)
|
||||
|
||||
|
||||
Every change to master goes through this workflow. No exceptions.
|
||||
|
||||
### Branch Protection (Required Gitea Settings)
|
||||
@@ -118,67 +147,40 @@ docs: update README
|
||||
|
||||
### 6. Review the PR (Mandatory — Before Adding ready-to-merge Label)
|
||||
|
||||
**Review checklist:** Every PR is reviewed against 13 categories covering
|
||||
architecture, code quality, security, i18n, testing, performance,
|
||||
UX, documentation, workflow compliance, maintainability, resource
|
||||
management, backwards compatibility, and logging.
|
||||
**Review checklist:** Every PR is reviewed against 8 categories covering
|
||||
functional correctness, completeness, architecture, reliability,
|
||||
robustness, security, technical excellence, and test quality.
|
||||
|
||||
**Automated review (CI `validate` job):** Every PR triggers an automated
|
||||
review via `python -m devx.ci.pr_review` as a step in the `validate` job.
|
||||
This posts a review with
|
||||
`COMMENT` (no issues) or `REQUEST_CHANGES` (issues found) based on
|
||||
the **[auto]** items in the checklist:
|
||||
**Deep review (agent-invoked `pr-review` skill):** The agent invokes
|
||||
the `pr-review` skill to perform a deep, critical review of the PR.
|
||||
The skill posts inline comments for each issue found via the Gitea MCP,
|
||||
auto-fixes them, pushes fixes to the PR branch, resolves discussion
|
||||
threads, and posts a brief summary. When no blocking issues remain,
|
||||
the PR is marked `ready-to-merge`.
|
||||
|
||||
- Architecture compliance (no subprocess in CLI, no hardcoded URLs)
|
||||
- Best practices (no `print()`, no bare `except`, no `TODO`/`FIXME`,
|
||||
no functions > 50 lines)
|
||||
- Security (no hardcoded secrets, no `shell=True`, no `eval`/`exec`)
|
||||
- i18n (no raw strings in `click.echo()` without `_()` wrapper)
|
||||
- Resource management (no `open()` without `with`, no `Popen()` without cleanup)
|
||||
- Documentation (source changes must include doc updates)
|
||||
- Test coverage (source changes must include test updates)
|
||||
- Commit conventions (conventional commit format on PR commits)
|
||||
|
||||
The automated review posts inline comments on specific lines and
|
||||
includes a summary of the checklist categories. The agent **must** address all
|
||||
`REQUEST_CHANGES` issues before proceeding.
|
||||
|
||||
**Manual review (agent):** After the automated review passes, the agent
|
||||
must go through **every category** listed above and verify
|
||||
the **[manual]** items by reviewing the full diff
|
||||
(`git diff master...HEAD`).
|
||||
|
||||
Post review comments using `devx.ci.pr_review` (run as `python -m devx.ci.pr_review`):
|
||||
```bash
|
||||
CI_GITEA_TOKEN=<token> python -m devx.ci.pr_review <pr_number> <owner/repo> \
|
||||
--event REQUEST_CHANGES \
|
||||
--body "Review summary"
|
||||
```
|
||||
See `.devin/skills/pr-review/SKILL.md` for the full review procedure,
|
||||
categories, and MCP tool reference.
|
||||
|
||||
### 7. Address Review Comments
|
||||
Fix each comment one by one, commit, and push. Re-review until satisfied.
|
||||
|
||||
### 8. Approve and Merge
|
||||
Once all checklist items are verified and comments are addressed, post
|
||||
an approval review with `--checklist-confirmed` and `--checklist-categories`:
|
||||
### 8. Mark Ready to Merge
|
||||
Once all issues are addressed, add the `ready-to-merge` label:
|
||||
```bash
|
||||
CI_GITEA_TOKEN=<token> python -m devx.ci.pr_review <pr_number> <owner/repo> \
|
||||
--event APPROVE --checklist-confirmed \
|
||||
--checklist-categories 1,2,3,4,5,6,7,8,9,10,11,12,13 \
|
||||
--body "All 13 checklist categories verified. Architecture: <summary>. Security: <summary>. Tests: <summary>. Docs: <summary>."
|
||||
make devx-pr-label
|
||||
```
|
||||
The auto-merge workflow posts an APPROVE review via the Gitea API
|
||||
and squash-merges with title `GRM-N: <conventional commit message>`.
|
||||
|
||||
The `--checklist-confirmed` flag is **required** for APPROVE events —
|
||||
it attests that the reviewer has gone through every checklist category.
|
||||
The `--checklist-categories` flag is also **required** — it must list at
|
||||
least 8 of the 13 category numbers, ensuring the reviewer actually
|
||||
checked each category rather than rubber-stamping. The review body must
|
||||
be substantive (> 50 characters) — perfunctory approvals like "LGTM" are
|
||||
rejected.
|
||||
> **IMPORTANT**: Never manually merge PRs via the API. Always use the auto-merge
|
||||
> workflow by adding the `ready-to-merge` label. Manual merges bypass the
|
||||
> `GRM-N: <conventional>` format enforcement, producing incorrectly named commits.
|
||||
> The auto-merge script validates the PR title matches the Vikunja task ID
|
||||
> and conventional commit format before merging.
|
||||
|
||||
Then add the `ready-to-merge` label. The auto-merge workflow will:
|
||||
The auto-merge workflow will:
|
||||
1. **Validate** PR title format (`GRM-N: <vikunja task title>`) and match against Vikunja task title
|
||||
2. **Check** that at least one substantive APPROVE review exists (body > 20 chars or has inline comments)
|
||||
2. **Post** an APPROVE review via the Gitea API (to satisfy branch protection)
|
||||
3. Wait for all CI checks to pass (including the `validate` job)
|
||||
4. Squash-merge with title: `GRM-N: <conventional commit message>`
|
||||
5. The post-merge workflow marks the Vikunja task as done
|
||||
@@ -190,12 +192,6 @@ a new CI run. The next auto-merge attempt will merge successfully.
|
||||
No manual rebase needed. To rebase manually: `make rebase` (local) or
|
||||
`make pr-rebase` (server-side via API).
|
||||
|
||||
> **IMPORTANT**: Never manually merge PRs via the API. Always use the auto-merge
|
||||
> workflow by adding the `ready-to-merge` label. Manual merges bypass the
|
||||
> `GRM-N: <conventional>` format enforcement, producing incorrectly named commits.
|
||||
> The auto-merge script validates the PR title matches the Vikunja task ID
|
||||
> and conventional commit format before merging.
|
||||
|
||||
### CI Path Filtering
|
||||
|
||||
The CI workflow's `validate` job includes a pre-merge validation step
|
||||
@@ -278,7 +274,7 @@ via `[tool.devx.classify]` in `pyproject.toml`.
|
||||
- Any new file type not in the allowlist
|
||||
|
||||
**devx module structure** (installed from git, not in this repo):
|
||||
- `devx.ci.*` — CI/CD automation (run by workflows): release, publish, auto_merge, classify_changes, detect_release_commit, push_badges, doc_coverage, sync_wiki, distribute_molecule, discover_runners, notify_failure, post_merge, pr_review, validate_commit_msg
|
||||
- `devx.ci.*` — CI/CD automation (run by workflows): release, publish, auto_merge, classify_changes, detect_release_commit, push_badges, doc_coverage, sync_wiki, distribute_molecule, discover_runners, notify_failure, post_merge, validate_commit_msg
|
||||
- `devx.tools.*` — Dev tools (run locally): check_test_speed, configure_repo, install_checkmake, install_tools, setup, generate_badges, create_task, create_pr, pr_status, pr_logs, pr_label, rebase, pr_rebase
|
||||
- `devx.molecule.*` — Molecule helpers: molecule_all, platforms, discover_runners, distribute_molecule
|
||||
- `devx.gitea_cli` — Tea CLI wrapper
|
||||
@@ -334,12 +330,10 @@ The `tea` Gitea CLI tool is used for Gitea API interactions in devx. It is insta
|
||||
- `devx.tools.configure_repo` — Creates labels via `tea labels create` (falls back to `GiteaClient` if tea fails; branch protection still uses `GiteaClient` since tea only supports basic protect/unprotect)
|
||||
|
||||
**Operations still using `GiteaClient` (not supported by tea):**
|
||||
- PR reviews (`devx.ci.pr_review`) — tea v0.14.1 only supports interactive reviews
|
||||
- Wiki page management (`devx.ci.sync_wiki`)
|
||||
- Commit status checks (`devx.ci.auto_merge`)
|
||||
- Runner discovery (`devx.molecule.discover_runners`)
|
||||
- Branch protection with detailed config (`devx.tools.configure_repo`)
|
||||
- PR file/commit listing (`devx.ci.pr_review`)
|
||||
|
||||
### PYTHONPATH Configuration
|
||||
|
||||
@@ -347,7 +341,7 @@ Since devx is installed as a package (via `pip install` from git), it is importa
|
||||
|
||||
| PYTHONPATH | When to use | Example modules |
|
||||
|------------|-------------|-----------------|
|
||||
| `src` | Module imports from `grm` | `devx.ci.auto_merge`, `devx.ci.pr_review`, `devx.ci.pr_review`, `devx.ci.sync_wiki`, `devx.ci.post_merge`, `devx.ci.classify_changes`, `devx.molecule.discover_runners`, `devx.ci.doc_coverage` |
|
||||
| `src` | Module imports from `grm` | `devx.ci.auto_merge`, `devx.ci.sync_wiki`, `devx.ci.post_merge`, `devx.ci.classify_changes`, `devx.molecule.discover_runners`, `devx.ci.doc_coverage` |
|
||||
| (none) | Module has no GRM imports | `devx.ci.detect_release_commit`, `devx.molecule.distribute_molecule`, `devx.ci.push_badges`, `devx.ci.validate_commit_msg` |
|
||||
|
||||
**In workflows**, always use `env:` blocks (not inline `PYTHONPATH=value`):
|
||||
|
||||
Reference in New Issue
Block a user