Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
96ff368c7c | ||
|
|
fe684bad50 | ||
|
|
f433b0980a |
@@ -2,18 +2,28 @@
|
|||||||
|
|
||||||
Quick reference for devx tools when working on this repo.
|
Quick reference for devx tools when working on this repo.
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
Invoke this skill when creating PRs, checking CI status, adding
|
||||||
|
labels, rebasing branches, or performing any PR lifecycle operation.
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- `.venv` exists (run `make setup` if not)
|
||||||
|
- `.env` with `DEVELOPER_GITEA_API_TOKEN`, `VIKUNJA_TOKEN`
|
||||||
|
|
||||||
## PR Workflow (use these, not raw git/tea/MCP)
|
## PR Workflow (use these, not raw git/tea/MCP)
|
||||||
|
|
||||||
| Task | Command |
|
| Task | Command |
|
||||||
|------|---------|
|
|------|---------|
|
||||||
| Create Vikunja task | `make create-task -- --title "..." --description "..."` |
|
| Create Vikunja task | `.venv/bin/python -m devx.tools.create_task --title "..." --description "..."` (make target doesn't forward args) |
|
||||||
| Create PR | `make create-pr` |
|
| Create PR | `make create-pr` |
|
||||||
| Push + create PR | `make push-with-pr` |
|
| Push + create PR | `make push-with-pr` |
|
||||||
| Check CI status | `make devx-pr-status` or `make devx-pr-status PR=42 WAIT=1` |
|
| Check CI status | `make devx-pr-status` or `make devx-pr-status PR=42 WAIT=1` |
|
||||||
| Fetch CI failure logs | `make devx-pr-logs` or `make devx-pr-logs PR=42 JOB=quality TAIL=50` |
|
| Fetch CI failure logs | `make devx-pr-logs` or `make devx-pr-logs PR=42 JOB=quality TAIL=50` |
|
||||||
| Add ready-to-merge label | `make devx-pr-label` or `make devx-pr-label PR=42` |
|
| Add ready-to-merge label | `make devx-pr-label` or `make devx-pr-label PR=42` |
|
||||||
| Rebase current branch | `make rebase` |
|
| Rebase current branch | `make devx-rebase` |
|
||||||
| Rebase PR via API | `make pr-rebase` or `make pr-rebase PR=42` |
|
| Rebase PR via API | `make devx-pr-rebase` or `make pr-rebase PR=42` |
|
||||||
|
|
||||||
## Auto-merge Behavior
|
## Auto-merge Behavior
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,70 @@
|
|||||||
|
# molecule-testing
|
||||||
|
|
||||||
|
Authoring and debugging `gitea_runner` molecule scenarios. For running
|
||||||
|
tests use the `testing-and-debugging` make targets — this covers
|
||||||
|
writing scenarios and fixing DIND/platform issues.
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
- Adding a molecule scenario for the `gitea_runner` role
|
||||||
|
- A scenario fails on platform setup, DIND, or registration mocking
|
||||||
|
- Reviewing scenario coverage for a role change
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- Docker running locally
|
||||||
|
- `.venv` exists (`make setup`)
|
||||||
|
|
||||||
|
## Scenario Layout
|
||||||
|
|
||||||
|
`ansible/roles/gitea_runner/molecule/<scenario>/`:
|
||||||
|
|
||||||
|
Current scenarios: `default`, `template-content`, `deregister`,
|
||||||
|
`multi-instance`, `update`, `remove`, `lifecycle`.
|
||||||
|
|
||||||
|
| File | Purpose |
|
||||||
|
|------|---------|
|
||||||
|
| `molecule.yml` | driver/platforms/provisioner config |
|
||||||
|
| `converge.yml` | applies the role |
|
||||||
|
| `verify.yml` | assertions scoped to the scenario |
|
||||||
|
| `prepare.yml` | optional host prep |
|
||||||
|
|
||||||
|
Scenario registration lives in `pyproject.toml` (scenario map used by
|
||||||
|
`devx.molecule` distribution in CI) — a new scenario MUST be
|
||||||
|
registered there or CI never runs it.
|
||||||
|
|
||||||
|
## molecule.yml Conventions
|
||||||
|
|
||||||
|
- Platform name/image/command are env-overridable via
|
||||||
|
`${MOLECULE_PLATFORM_*}` so all-platforms runs work.
|
||||||
|
- `remote_tmp: /tmp` in provisioner `config_options` — default temp
|
||||||
|
dir breaks in containers.
|
||||||
|
- `ANSIBLE_ROLES_PATH` must include the repo roles root.
|
||||||
|
- Use `inventory.group_vars` to isolate the scenario: disable
|
||||||
|
unrelated features rather than editing tasks.
|
||||||
|
- Runner registration in tests is mocked/faked — scenarios must not
|
||||||
|
require a live Gitea instance; check how existing scenarios stub
|
||||||
|
the registration/token flow before adding API calls.
|
||||||
|
|
||||||
|
## Debugging
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd ansible/roles/gitea_runner
|
||||||
|
molecule test -s <scenario>
|
||||||
|
molecule converge -s <scenario>
|
||||||
|
molecule login -s <scenario>
|
||||||
|
```
|
||||||
|
|
||||||
|
- "Failed to create temporary directory" → `remote_tmp: /tmp` missing.
|
||||||
|
- Idempotence failures → find the changed task on second converge.
|
||||||
|
- Registration/API timeouts → the scenario hit a real endpoint —
|
||||||
|
stub it like the existing scenarios do.
|
||||||
|
|
||||||
|
## Common Mistakes
|
||||||
|
|
||||||
|
- Adding a scenario without registering it in `pyproject.toml` —
|
||||||
|
silently untested.
|
||||||
|
- Hardcoding the platform image — keep `${MOLECULE_PLATFORM_*}`
|
||||||
|
overrides.
|
||||||
|
- Calling the real Gitea API in converge — scenarios must be
|
||||||
|
self-contained; mock the registration path.
|
||||||
@@ -0,0 +1,73 @@
|
|||||||
|
# runner-ops
|
||||||
|
|
||||||
|
Operating the Gitea Actions runner fleet: registration lifecycle,
|
||||||
|
stale-runner cleanup, image pruning, and safe debugging. Core code:
|
||||||
|
`src/grm/runner_manager.py`, `src/grm/executor.py`,
|
||||||
|
`src/grm/registry.py`.
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
- Runners go offline, stall, or pile up stale registrations
|
||||||
|
- Runner hosts need install/update/remove/deregister operations
|
||||||
|
- Disk pressure on runner hosts (image/container accumulation)
|
||||||
|
- Working on S08 (leases, physical-host admission, disk watermarks)
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- `.env` with Gitea admin token for API operations
|
||||||
|
- SSH access to runner hosts for Ansible-driven lifecycle
|
||||||
|
- Runner registrations visible via admin API:
|
||||||
|
`GET /api/v1/admin/actions/runners`
|
||||||
|
|
||||||
|
## Architecture
|
||||||
|
|
||||||
|
- `RunnerManager` orchestrates install/update/lifecycle via
|
||||||
|
`AnsibleExecutor` against the `gitea_runner` role; `RunnerRegistry`
|
||||||
|
tracks local runner state.
|
||||||
|
- Runners execute jobs in Docker (`docker` label) — every job gets a
|
||||||
|
fresh container from `ci-base`/`ci-quality`/`ci-full` images.
|
||||||
|
- Molecule jobs nest containers (DIND) — privileged, `SYS_ADMIN`,
|
||||||
|
`/var/lib/docker` volume.
|
||||||
|
|
||||||
|
## Lifecycle Operations
|
||||||
|
|
||||||
|
| Task | Entry point |
|
||||||
|
|------|-------------|
|
||||||
|
| Install/update runners | `grm` CLI → `RunnerManager` (Ansible) |
|
||||||
|
| Stale registration cleanup | `scripts/cleanup_stale_runners.py` — deletes runners offline >1h via `DELETE /api/v1/admin/actions/runners/{id}` |
|
||||||
|
| Image pruning | `scripts/prune_runner_images.py` — reclaims disk from old CI image versions |
|
||||||
|
|
||||||
|
Stale registrations accumulate when a host is rebuilt, re-registered,
|
||||||
|
or its runner process dies unrecoverably — clean them before capacity
|
||||||
|
accounting.
|
||||||
|
|
||||||
|
## Debugging a Stuck Runner
|
||||||
|
|
||||||
|
1. Check registration state via admin API (offline vs online).
|
||||||
|
2. SSH to the host: `systemctl status` the runner service / inspect
|
||||||
|
`docker ps` for orphaned job containers.
|
||||||
|
3. Orphaned molecule containers: safe to remove ONLY when no molecule
|
||||||
|
run is active — check runner logs first (`runner-ops` counterpart
|
||||||
|
of "don't force-remove active containers", fixed in GRM-166/167).
|
||||||
|
4. Disk pressure: check `/var/lib/docker` usage, then
|
||||||
|
`prune_runner_images.py` — never blanket `docker system prune`
|
||||||
|
while jobs may be mid-flight.
|
||||||
|
|
||||||
|
## S08-Relevant Rules
|
||||||
|
|
||||||
|
- Runner admission must be per physical host — a runner that shares
|
||||||
|
hardware must declare capacity, not just labels.
|
||||||
|
- Cleanup must never remove a container a live job owns — ownership
|
||||||
|
check before any force-removal.
|
||||||
|
- Disk watermark logic belongs in the role/scripts, not ad-hoc
|
||||||
|
cron `docker prune`.
|
||||||
|
|
||||||
|
## Common Mistakes
|
||||||
|
|
||||||
|
- `docker system prune -a` on a runner host — kills in-flight job
|
||||||
|
containers and image cache mid-run.
|
||||||
|
- Deleting an offline runner registration while the host still runs
|
||||||
|
the service — it re-registers and duplicates; stop the service
|
||||||
|
first.
|
||||||
|
- Treating molecule DIND containers as junk — they belong to an
|
||||||
|
active scenario; check timestamps and runner logs.
|
||||||
@@ -1,5 +1,14 @@
|
|||||||
# Spec-Driven Development
|
# Spec-Driven Development
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
Invoke this skill when starting any change — every PR requires a spec
|
||||||
|
at `docs/specs/<TASK-ID>.md` that CI validates before merge.
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- A Vikunja task ID (`GRM-N`) — see `vikunja-tasks` skill
|
||||||
|
|
||||||
## Overview
|
## Overview
|
||||||
|
|
||||||
Every change starts with a spec. No spec, no code. No code, no PR.
|
Every change starts with a spec. No spec, no code. No code, no PR.
|
||||||
|
|||||||
@@ -3,6 +3,17 @@
|
|||||||
Make targets for testing, debugging, and CI investigation. **Use these
|
Make targets for testing, debugging, and CI investigation. **Use these
|
||||||
instead of raw `pytest`, `ruff`, or `molecule` commands.**
|
instead of raw `pytest`, `ruff`, or `molecule` commands.**
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
Invoke this skill when running tests, investigating CI failures, or
|
||||||
|
debugging molecule scenarios. Also invoke when asked to "run tests",
|
||||||
|
"check coverage", or "debug a failure".
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- `.venv` exists (run `make setup` if not)
|
||||||
|
- For molecule tests: Docker is running
|
||||||
|
|
||||||
## Why Make Targets
|
## Why Make Targets
|
||||||
|
|
||||||
Make targets encapsulate the correct venv activation, PYTHONPATH, env
|
Make targets encapsulate the correct venv activation, PYTHONPATH, env
|
||||||
@@ -31,9 +42,8 @@ produces false failures (missing dependencies, wrong Python version).
|
|||||||
|
|
||||||
| Task | Command | Notes |
|
| Task | Command | Notes |
|
||||||
|------|---------|-------|
|
|------|---------|-------|
|
||||||
| All scenarios | `make molecule` | All 6 scenarios on Ubuntu 22.04 |
|
| All scenarios | `make molecule` | All 7 scenarios on Ubuntu 22.04 |
|
||||||
| All platforms | `make molecule-all` | All 6 scenarios on all 4 OSes |
|
| All platforms | `make molecule-all` | All 7 scenarios on all 4 OSes |
|
||||||
| Parallel | `make molecule-all-parallel` | MOLECULE_JOBS=4 |
|
|
||||||
|
|
||||||
### Spec-Driven Workflow
|
### Spec-Driven Workflow
|
||||||
|
|
||||||
@@ -46,12 +56,12 @@ CI validates the spec before running expensive jobs.
|
|||||||
**Before pushing any branch:**
|
**Before pushing any branch:**
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
make pre-push
|
make lint-all && make pytest-cov
|
||||||
```
|
```
|
||||||
|
|
||||||
This runs `lint-all` + `pytest-cov`. The pre-push git hook only
|
This runs all linters + unit tests with coverage. The pre-push git
|
||||||
validates the Vikunja task exists — it does NOT run tests. You must
|
hook only validates the Vikunja task exists — it does NOT run tests.
|
||||||
run `make pre-push` manually.
|
Run the checks manually (there is no `pre-push` target here).
|
||||||
|
|
||||||
## CI Failure Investigation
|
## CI Failure Investigation
|
||||||
|
|
||||||
@@ -59,7 +69,7 @@ When investigating a CI failure:
|
|||||||
|
|
||||||
1. **Fetch logs via MCP** — use `mcp_call_tool` with gitea server,
|
1. **Fetch logs via MCP** — use `mcp_call_tool` with gitea server,
|
||||||
`actions_run_read` method, `download_job_log` tool
|
`actions_run_read` method, `download_job_log` tool
|
||||||
2. **Reproduce locally** — use `make pytest-cov` or `make lint-ci`
|
2. **Reproduce locally** — use `make pytest-cov` or `make lint-all`
|
||||||
depending on which CI job failed
|
depending on which CI job failed
|
||||||
3. **Never run raw pytest** — always use the make target
|
3. **Never run raw pytest** — always use the make target
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,74 @@
|
|||||||
|
# vikunja-tasks
|
||||||
|
|
||||||
|
Vikunja task lifecycle beyond `create`: querying status, closing, and
|
||||||
|
recovering when the tracker is unreachable.
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
- Creating, closing, or checking a Vikunja task
|
||||||
|
- A spec workflow step needs the task ID or done state
|
||||||
|
- `vikunja.oblachno.oblachno.fyi` fails to resolve / times out
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- `.env` with `VIKUNJA_TOKEN`
|
||||||
|
- Project ID comes from `[tool.devx]` in `pyproject.toml`
|
||||||
|
(`DEVX_VIKUNJA_PROJECT_ID`)
|
||||||
|
|
||||||
|
## Create
|
||||||
|
|
||||||
|
`make create-task` does **not** forward arguments — call the module:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
.venv/bin/python -m devx.tools.create_task \
|
||||||
|
--title "Task title (no GRM-N prefix)" \
|
||||||
|
--description "<h2>Context</h2><p>...</p>"
|
||||||
|
```
|
||||||
|
|
||||||
|
Prints `GRM-N` + next steps. Title must not include the task-ID
|
||||||
|
prefix (auto-merge prepends it; a manual prefix double-prefixes the
|
||||||
|
PR title and fails validation).
|
||||||
|
|
||||||
|
## Query / Close
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Task details (ID = numeric part of GRM-N)
|
||||||
|
curl -sf -H "Authorization: Bearer $VIKUNJA_TOKEN" \
|
||||||
|
"https://vikunja.oblachno.oblachno.fyi/api/v1/tasks/<N>"
|
||||||
|
|
||||||
|
# Close: mark done
|
||||||
|
curl -sf -X POST -H "Authorization: Bearer $VIKUNJA_TOKEN" \
|
||||||
|
-H "Content-Type: application/json" -d '{"done":true}' \
|
||||||
|
"https://vikunja.oblachno.oblachno.fyi/api/v1/tasks/<N>"
|
||||||
|
```
|
||||||
|
|
||||||
|
Post-merge automation marks the task done when the PR squash-merges —
|
||||||
|
manual close is only needed for abandoned/superseded tasks.
|
||||||
|
|
||||||
|
## Task-ID / Spec Collisions
|
||||||
|
|
||||||
|
Vikunja IDs can collide with historical spec files (an old task reused
|
||||||
|
the number). Convention: preserve the old file as
|
||||||
|
`docs/specs/<ID>-<topic>-historical.md`, then write the new spec at
|
||||||
|
`docs/specs/<ID>.md`. Check `git log` on the existing spec before
|
||||||
|
moving it.
|
||||||
|
|
||||||
|
## Tracker Unreachable
|
||||||
|
|
||||||
|
If the Vikunja host fails DNS/TLS:
|
||||||
|
|
||||||
|
1. Don't block the whole workflow — record the intended task title in
|
||||||
|
the spec draft and retry `create_task` before branching.
|
||||||
|
2. Never invent an ID — branch/PR titles must match a real task or
|
||||||
|
`pre_push_check` / auto-merge validation fails.
|
||||||
|
3. DNS failures observed so far were transient; retry after a few
|
||||||
|
minutes before escalating.
|
||||||
|
|
||||||
|
## Common Mistakes
|
||||||
|
|
||||||
|
- `make create-task -- --title ...` — args are dropped; use the module
|
||||||
|
call above (forwarding fix is S11 scope).
|
||||||
|
- Including `GRM-N:` in the task title — double prefix breaks
|
||||||
|
auto-merge.
|
||||||
|
- Closing a task whose PR is still open — auto-merge's post-merge
|
||||||
|
step handles the close; manual close confuses the audit trail.
|
||||||
@@ -111,7 +111,7 @@ The auto-merge workflow enforces the APPROVE review check programmatically
|
|||||||
as a defense-in-depth measure, but branch protection is the primary gate.
|
as a defense-in-depth measure, but branch protection is the primary gate.
|
||||||
|
|
||||||
### 1. Create Vikunja Task
|
### 1. Create Vikunja Task
|
||||||
Create a task in Vikunja project 6 via `make create-task -- --title "Task title" --description "<h2>...</h2>"` (requires `VIKUNJA_TOKEN` in `.env`). This prints the `GRM-N` identifier and next-step instructions.
|
Create a task in Vikunja project 6 via `.venv/bin/python -m devx.tools.create_task --title "Task title" --description "<h2>...</h2>"` (make target does not forward args) (requires `VIKUNJA_TOKEN` in `.env`). This prints the `GRM-N` identifier and next-step instructions.
|
||||||
|
|
||||||
**IMPORTANT:** The task title must NOT include the `GRM-N:` prefix.
|
**IMPORTANT:** The task title must NOT include the `GRM-N:` prefix.
|
||||||
The `make create-pr` and `check_auto_merge_ready` commands automatically
|
The `make create-pr` and `check_auto_merge_ready` commands automatically
|
||||||
|
|||||||
@@ -8,12 +8,12 @@ Each runner runs in an isolated **rootless Docker** environment under a dedicate
|
|||||||
|
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/src/branch/master/LICENSE)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/src/branch/master/LICENSE)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/wiki)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/wiki)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/releases)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/releases)
|
||||||
[](https://www.python.org/downloads/)
|
[](https://www.python.org/downloads/)
|
||||||
|
|
||||||
## Why GRM?
|
## Why GRM?
|
||||||
|
|
||||||
|
|||||||
+6
-6
@@ -8,12 +8,12 @@ Each runner runs in an isolated **rootless Docker** environment under a dedicate
|
|||||||
|
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/src/branch/master/LICENSE)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/src/branch/master/LICENSE)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/wiki)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/wiki)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/actions)
|
||||||
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/releases)
|
[](https://git.oblachno.oblachno.fyi/oblachno-oss/grm/releases)
|
||||||
[](https://www.python.org/downloads/)
|
[](https://www.python.org/downloads/)
|
||||||
|
|
||||||
## Overview
|
## Overview
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,28 @@
|
|||||||
|
# GRM-171: Use kireto token for auto-merge approval review
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
The auto-merge workflow posts approval reviews with
|
||||||
|
`REVIEWER_GITEA_API_TOKEN` (emil), but emil is also the PR creator.
|
||||||
|
Gitea ignores self-approvals, so the merge fails with HTTP 405
|
||||||
|
`Does not have enough approvals`.
|
||||||
|
|
||||||
|
## Approach
|
||||||
|
REQ-1: Change the approval review step in `.gitea/workflows/ci.yml` to use
|
||||||
|
`DEVELOPER_GITEA_API_TOKEN` (kireto) instead of
|
||||||
|
`REVIEWER_GITEA_API_TOKEN` (emil), since kireto is a different user
|
||||||
|
than the PR creator.
|
||||||
|
|
||||||
|
## Test Plan
|
||||||
|
- `make lint-all` passes (workflow-lint validates the YAML)
|
||||||
|
- Next auto-merge PR succeeds (approval posted by kireto, merge completes)
|
||||||
|
|
||||||
|
## Deploy Plan
|
||||||
|
- Merge to master
|
||||||
|
|
||||||
|
## Rollback Plan
|
||||||
|
- Revert the merge commit
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
- [x] REQ-1: Change the approval review step in `.gitea/workflows/ci.yml`
|
||||||
|
to use `DEVELOPER_GITEA_API_TOKEN` (kireto) instead of
|
||||||
|
`REVIEWER_GITEA_API_TOKEN` (emil)
|
||||||
+39
-16
@@ -1,28 +1,51 @@
|
|||||||
# GRM-171: Use kireto token for auto-merge approval review
|
# GRM-171: Add runner-ops, molecule-testing, vikunja-tasks skills, fix create-task docs
|
||||||
|
|
||||||
## Problem
|
## Problem
|
||||||
The auto-merge workflow posts approval reviews with
|
|
||||||
`REVIEWER_GITEA_API_TOKEN` (emil), but emil is also the PR creator.
|
The OBL-INFRA-548 programme audit found grm lacks skills for runner
|
||||||
Gitea ignores self-approvals, so the merge fails with HTTP 405
|
fleet operations (needed for S08: leases, admission, watermarks),
|
||||||
`Does not have enough approvals`.
|
molecule scenario authoring, and Vikunja task lifecycle.
|
||||||
|
`devx-workflow` and `AGENTS.md` document `make create-task -- --title`,
|
||||||
|
which fails because `devx-create-task` forwards no arguments.
|
||||||
|
|
||||||
## Approach
|
## Approach
|
||||||
REQ-1: Change the approval review step in `.gitea/workflows/ci.yml` to use
|
|
||||||
`DEVELOPER_GITEA_API_TOKEN` (kireto) instead of
|
REQ-1: Add `runner-ops` skill: RunnerManager/AnsibleExecutor model,
|
||||||
`REVIEWER_GITEA_API_TOKEN` (emil), since kireto is a different user
|
stale-runner cleanup, image pruning, molecule container lifecycle,
|
||||||
than the PR creator.
|
safe-debugging rules.
|
||||||
|
REQ-2: Add `molecule-testing` skill: gitea_runner scenario layout,
|
||||||
|
platform overrides, isolation flags, debugging.
|
||||||
|
REQ-3: Add `vikunja-tasks` skill: create via module call, query,
|
||||||
|
close, spec-collision convention.
|
||||||
|
REQ-4: Fix broken `make create-task -- --title` documentation in
|
||||||
|
`devx-workflow` skill and `AGENTS.md`.
|
||||||
|
REQ-5: Add skill validation tests (`tests/unit/test_skills_validation.py`)
|
||||||
|
+ fix stale make-target refs and missing sections in existing skills.
|
||||||
|
|
||||||
|
Preserve the colliding spec as
|
||||||
|
[GRM-171-kireto-token-historical](GRM-171-kireto-token-historical.md).
|
||||||
|
|
||||||
## Test Plan
|
## Test Plan
|
||||||
- `make lint-all` passes (workflow-lint validates the YAML)
|
|
||||||
- Next auto-merge PR succeeds (approval posted by kireto, merge completes)
|
- `pytest tests/unit/test_skills_validation.py` passes (12 tests).
|
||||||
|
|
||||||
## Deploy Plan
|
## Deploy Plan
|
||||||
- Merge to master
|
|
||||||
|
Documentation/skills only — auto-merge to master; no runtime deploy.
|
||||||
|
|
||||||
## Rollback Plan
|
## Rollback Plan
|
||||||
- Revert the merge commit
|
|
||||||
|
Revert the squash-merge commit; skills are inert documentation.
|
||||||
|
|
||||||
## Acceptance Criteria
|
## Acceptance Criteria
|
||||||
- [x] REQ-1: Change the approval review step in `.gitea/workflows/ci.yml`
|
|
||||||
to use `DEVELOPER_GITEA_API_TOKEN` (kireto) instead of
|
- [x] REQ-1: `runner-ops` skill exists.
|
||||||
`REVIEWER_GITEA_API_TOKEN` (emil)
|
- [x] REQ-2: `molecule-testing` skill exists.
|
||||||
|
- [x] REQ-3: `vikunja-tasks` skill exists.
|
||||||
|
- [x] REQ-4: create-task docs corrected.
|
||||||
|
- [x] REQ-5: Skill validation tests added and passing.
|
||||||
|
|
||||||
|
## Out of Scope
|
||||||
|
|
||||||
|
- Runner lease/admission implementation (S08 scope).
|
||||||
|
- Fixing `devx-create-task` argument forwarding (devx repo, S11).
|
||||||
|
|||||||
@@ -0,0 +1,120 @@
|
|||||||
|
"""Pytest tests for Devin skill validation.
|
||||||
|
|
||||||
|
Validates that all skills in .devin/skills/ are well-formed: H1 title,
|
||||||
|
"when to invoke" section, prerequisites when commands are referenced,
|
||||||
|
make-target references that exist, and file references that exist.
|
||||||
|
|
||||||
|
Run with: make pytest TEST=tests/test_skills_validation.py
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||||
|
|
||||||
|
# Sections required for every skill
|
||||||
|
REQUIRED_SECTIONS = ["when to invoke"]
|
||||||
|
|
||||||
|
# Sections required only for skills that reference commands/tools
|
||||||
|
COMMAND_REQUIRED_SECTIONS = ["prerequisites"]
|
||||||
|
|
||||||
|
# Markers indicating a skill references commands/tools
|
||||||
|
COMMAND_MARKERS = ("`make ", "```bash", "```sh", "curl ", "python ", "python3 ", "ssh ")
|
||||||
|
|
||||||
|
EXPECTED_SKILLS = [
|
||||||
|
"dependency-graph",
|
||||||
|
"deployment-coordination",
|
||||||
|
"devx-workflow",
|
||||||
|
"molecule-testing",
|
||||||
|
"pr-review",
|
||||||
|
"runner-ops",
|
||||||
|
"skill-creation",
|
||||||
|
"spec-driven-development",
|
||||||
|
"testing-and-debugging",
|
||||||
|
"vikunja-tasks",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def _find_skills() -> dict[str, Path]:
|
||||||
|
skills_dir = REPO_ROOT / ".devin" / "skills"
|
||||||
|
assert skills_dir.exists(), ".devin/skills/ directory not found"
|
||||||
|
return {d.name: d / "SKILL.md" for d in skills_dir.iterdir() if d.is_dir() and (d / "SKILL.md").exists()}
|
||||||
|
|
||||||
|
|
||||||
|
# Skills shared with other repos — file-path references are only checked
|
||||||
|
# in the owning repo (infra), where the referenced files live.
|
||||||
|
SHARED_SKILLS = {"cross-repo-sync", "branch-hygiene", "dependency-graph", "skill-creation"}
|
||||||
|
|
||||||
|
|
||||||
|
def _make_targets() -> set[str]:
|
||||||
|
"""Collect make targets from Makefile plus included devx .mak files."""
|
||||||
|
targets: set[str] = set()
|
||||||
|
makefile = REPO_ROOT / "Makefile"
|
||||||
|
if makefile.exists():
|
||||||
|
targets.update(re.findall(r"^([a-zA-Z][a-zA-Z0-9_-]*):", makefile.read_text(), re.MULTILINE))
|
||||||
|
for mak in REPO_ROOT.glob(".venv/lib/python*/site-packages/devx/make/*.mak"):
|
||||||
|
targets.update(re.findall(r"^([a-zA-Z][a-zA-Z0-9_-]*):", mak.read_text(), re.MULTILINE))
|
||||||
|
return targets
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_skill(skill_name: str, skill_path: Path, make_targets: set[str]) -> list[str]:
|
||||||
|
"""Validate a single skill file. Returns list of error messages."""
|
||||||
|
errors: list[str] = []
|
||||||
|
content = skill_path.read_text()
|
||||||
|
|
||||||
|
if not re.search(r"^# ", content, re.MULTILINE):
|
||||||
|
errors.append(f"{skill_name}: missing H1 title")
|
||||||
|
|
||||||
|
lower = content.lower()
|
||||||
|
for section in REQUIRED_SECTIONS:
|
||||||
|
if f"## {section}" not in lower:
|
||||||
|
errors.append(f"{skill_name}: missing '## {section.title()}' section")
|
||||||
|
|
||||||
|
references_commands = any(marker in content for marker in COMMAND_MARKERS)
|
||||||
|
if references_commands:
|
||||||
|
for section in COMMAND_REQUIRED_SECTIONS:
|
||||||
|
if f"## {section}" not in lower:
|
||||||
|
errors.append(
|
||||||
|
f"{skill_name}: missing '## {section.title()}' section "
|
||||||
|
"(required because skill references commands/tools)"
|
||||||
|
)
|
||||||
|
|
||||||
|
for target in re.findall(r"`make ([a-zA-Z][a-zA-Z0-9_-]*)`", content):
|
||||||
|
if target not in make_targets:
|
||||||
|
errors.append(f"{skill_name}: references `make {target}` but target does not exist")
|
||||||
|
|
||||||
|
# File-path checks: skip shared skills (checked in infra) and
|
||||||
|
# placeholder paths containing <...> templates.
|
||||||
|
if skill_name not in SHARED_SKILLS:
|
||||||
|
for match in re.findall(r"`((?:scripts|src|ansible|docs|tests|environments)/[^`\s]+)`", content):
|
||||||
|
if "<" in match:
|
||||||
|
continue
|
||||||
|
if not (REPO_ROOT / match).exists():
|
||||||
|
errors.append(f"{skill_name}: references `{match}` but file does not exist")
|
||||||
|
|
||||||
|
return errors
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("skill_name", EXPECTED_SKILLS)
|
||||||
|
def test_skill_exists(skill_name: str) -> None:
|
||||||
|
"""Each expected skill must have a SKILL.md."""
|
||||||
|
skill = REPO_ROOT / ".devin" / "skills" / skill_name / "SKILL.md"
|
||||||
|
assert skill.exists(), f"{skill_name}/SKILL.md not found"
|
||||||
|
|
||||||
|
|
||||||
|
def test_minimum_skill_count() -> None:
|
||||||
|
"""The repo should carry a working set of skills, not a stub."""
|
||||||
|
assert len(_find_skills()) >= 8, "expected >=10 skills"
|
||||||
|
|
||||||
|
|
||||||
|
def test_all_skills_validate() -> None:
|
||||||
|
"""All skills must pass structure/reference validation."""
|
||||||
|
make_targets = _make_targets()
|
||||||
|
errors: list[str] = []
|
||||||
|
for skill_name, skill_path in _find_skills().items():
|
||||||
|
errors.extend(_validate_skill(skill_name, skill_path, make_targets))
|
||||||
|
assert not errors, "Skill validation failed:\n" + "\n".join(f" - {e}" for e in errors)
|
||||||
Reference in New Issue
Block a user