Working against a stale mirror has cost us three different ways: - mailu #6 was linked as the fix for two internet-facing Roundcube CVEs while upstream had already merged AND released it (3.1.3+2024.06.57). The work was done; only our mirror was behind. Reconciling closed the PR automatically. - a stale mirror makes a survey report 'no upgrades available', so the recipe silently drops out of the weekly run. - reading the wrong branch: several coopcloud recipes keep a stale 'main' beside the real default 'master'. gitea's main is 1.24.2-rootless while master has 1.27.1-rootless and the merged PRs, so reading main manufactures a false 'three releases behind, missing two CVSS-9.8 RCEs' finding. The reconcile logic already existed inside open-recipe-pr.sh --reconcile-only and already resolves the default branch itself. What was missing was a single obvious entry point and a rule saying to run it. reconcile-upstream.sh takes recipes or --all, and is idempotent — recipe work lives in branches, never on mirror main, so force-syncing main discards nothing. /ci-test-review and /cc-ci-tests-update had NO reconcile step at all; both now require it. /cve-check, /recipe-upgrade and /upgrade-all already reconciled and now point at the shared script.
273 řádky
17 KiB
Markdown
273 řádky
17 KiB
Markdown
---
|
|
name: cc-ci-tests-update
|
|
description: Sweep all maintained recipes for stale cc-ci tests and update the stale ones so they pass again, verified via the standard PR + !testme -> CI flow. Runs the deterministic test suite across every enrolled recipe, identifies failures caused by a genuinely-stale test (the recipe legitimately changed upstream and the cc-ci test/overlay still asserts the old behavior), authors the minimal test update, opens a cc-ci test PR per stale recipe, and verifies each by running !testme on the corresponding recipe upgrade PR with the test change applied (or a direct harness run with the cc-ci branch checked out). NEVER weakens a test to force green. Opens each cc-ci test PR for visibility/historical record and merges it directly once verified green (the skill invocation IS the authorization); the end-of-run report includes every merged PR link + a summary of what changed. The fleet-wide stale-test cleanup that the weekly /upgrade-all deliberately defers (it comments-only on stale tests; this skill closes the loop). Invoke as /cc-ci-tests-update.
|
|
---
|
|
|
|
# cc-ci-tests-update
|
|
|
|
The fleet-wide **stale-test cleanup** for cc-ci. The weekly `/upgrade-all` cron deliberately does
|
|
**not** auto-edit cc-ci tests — it runs in DEFAULT mode and, where a recipe's existing test is
|
|
genuinely stale against a new upstream version, it leaves an **explanatory comment** on the recipe
|
|
PR and stops (operator decides). Over time those commented stale tests accumulate. This skill
|
|
**closes that loop**: it sweeps every maintained recipe, finds the ones whose cc-ci test is stale,
|
|
authors the minimal test update, opens a cc-ci test PR per stale recipe, and **verifies** each via
|
|
the standard `!testme` → CI flow (or a direct harness run with the test branch applied). It never
|
|
weakens a test. Verified test PRs are merged directly — the PR is the visible, reviewable record;
|
|
the invocation is the authorization — and the report lists every merged PR + what changed.
|
|
|
|
> **Relationship to the other skills.** This is the fleet-wide version of the per-recipe
|
|
> `--with-tests` path in `/recipe-upgrade` (step 5b), and the "TEST out-of-date" classification
|
|
> branch of `/ci-test-review`. It reuses both skills' discipline + helpers. The difference is
|
|
> **scope**: those skills react to a failure they happened to hit; this one **proactively sweeps
|
|
> the whole fleet** for stale tests and fixes them in one pass.
|
|
|
|
## What this skill does NOT do
|
|
|
|
- **Does NOT bump recipe image tags** — that's `/upgrade-all` / `/recipe-upgrade`. This skill fixes
|
|
the **tests**, not the recipes. (It may *discover* a stale test by re-running a recipe upgrade
|
|
PR's `!testme`, but the recipe bump itself is owned by `/upgrade-all`.)
|
|
- **Does NOT touch the cc-ci server host / flake** — that's `/cc-ci-server-update`.
|
|
- **Does NOT fix real upgrade regressions** — if a recipe's `!testme` is red because the upgrade is
|
|
broken (not because the test is stale), that's a `/recipe-upgrade` (step 5a) or `/ci-test-review`
|
|
job. This skill only touches a test when the **upgrade is correct** and the **test is wrong**.
|
|
- **Does merge its own cc-ci test PRs** (once verified green) — they're opened for visibility and
|
|
historical record; the invocation is the authorization, and the operator reviews the merged PRs
|
|
via the links in the report. It does NOT merge the paired **recipe upgrade PRs** — those stay
|
|
operator-owned (`/upgrade-all` scope).
|
|
|
|
## Preconditions / access
|
|
|
|
- `ssh cc-ci` works (root). Same access notes as `/recipe-upgrade` / `/ci-test-review`.
|
|
- `/srv/cc-ci/.testenv` has the `GITEA_*` creds (for opening cc-ci test PRs + posting `!testme`).
|
|
- The shared Swarm is **quiescent enough** for test runs — each stale-test verification deploys a
|
|
recipe, so don't run this concurrent with a `/upgrade-all` or active build loops. Serialize.
|
|
- abra over ssh needs the pseudo-TTY wrapper (see the box in `/recipe-upgrade`): every `abra` call
|
|
is `ssh cc-ci 'script -qec "abra <args> -n" /dev/null'`.
|
|
|
|
## Procedure
|
|
|
|
### 1. Build the stale-test candidate list (sweep — deterministic, no AI judgement yet)
|
|
|
|
The candidates are recipes whose cc-ci test is **potentially stale**. Two sources, merge both:
|
|
|
|
**(a) Carry-over from `/upgrade-all` — the commented stale tests.** Each weekly run's report has a
|
|
"PRs where a test looks stale" section (recipes where `!testme` went RED on a stale test and got a
|
|
PR comment). Read the latest report + a few weeks back:
|
|
```
|
|
grep -A2 "test.*stale\|stale.*test\|SUCCESS-PENDING-TESTS" /srv/cc-ci/.cc-ci-logs/upgrades/upgrade-all-*.md | tail -40
|
|
```
|
|
Each line names a recipe + the stale test + the PR URL. These are **known stale** (already
|
|
diagnosed by the upgrader) — they go straight to the fix list.
|
|
|
|
**(b) Fresh sweep — re-run the test suite across all maintained recipes.** Use the
|
|
`/ci-test-review` deterministic sweep helper to find failures that aren't already on the carry-over
|
|
list:
|
|
```
|
|
bash /srv/cc-ci-orch/.claude/skills/ci-test-review/run-all-recipes.sh
|
|
```
|
|
It runs the full harness per recipe on cc-ci and writes a JSON summary to
|
|
`/srv/cc-ci/.cc-ci-logs/ci-test-review-<runid>.json`. Parse it for `fail` verdicts.
|
|
|
|
> **Concurrency note:** a full sweep is ~20 recipes × full suite — it takes a while and uses the
|
|
> shared Swarm. Run it when the host is otherwise idle. If you only want the carry-over stale tests
|
|
> (skip the fresh sweep), pass `RECIPES="recipe1 recipe2"` to the helper to limit it.
|
|
|
|
**(c) Merge + classify.** Combine (a) and (b) into `CANDIDATES`. For each candidate, classify the
|
|
failure (AI — this is the `ci-test-review` step-3 diagnosis):
|
|
- **RECIPE bug** (the upgrade is broken) → NOT this skill. Hand off to `/recipe-upgrade` (5a) or
|
|
note it for the operator. Don't touch the test.
|
|
- **CI-SERVER bug** (harness/infra wrong) → NOT this skill (that's a `/ci-test-review` cc-ci-side
|
|
fix, not a test-update). Note it.
|
|
- **TEST out-of-date** (the recipe legitimately changed upstream and the cc-ci test/overlay asserts
|
|
old behavior) → **this skill**. Add to `STALE_TESTS` with: recipe, the failing test name, what
|
|
changed upstream, what the test currently asserts.
|
|
- **FLAKY** → re-run once or twice; if it passes, drop it (not stale, just flaky).
|
|
|
|
> ### ⚠️ RECONCILE FROM UPSTREAM FIRST — always, before any PR work or upgrade check
|
|
> ```
|
|
> cc-ci-plan/reconcile-upstream.sh <recipe>... # or --all
|
|
> ```
|
|
> Deterministic, idempotent, and safe (recipe work lives in branches, never on mirror `main`). It
|
|
> force-syncs each mirror to coopcloud's **default branch — resolved from the API, `main` OR
|
|
> `master`** — and closes any mirror PR whose changes upstream already merged. Skipping it has cost
|
|
> us three distinct ways: mailu #6 was reported as the fix for two internet-facing CVEs while
|
|
> upstream had already merged AND released it; a stale mirror makes a survey report "no upgrades
|
|
> available" so the recipe drops out of the weekly run; and reading the wrong branch on a recipe with
|
|
> a stale `main` beside a live `master` (gitea) manufactures a false "three releases behind, missing
|
|
> two CVSS-9.8 RCEs" finding.
|
|
|
|
### 2. For each stale test — author the minimal test update (AI; never weaken)
|
|
|
|
> **Read `tests/STYLE.md` in the cc-ci repo before writing the update.** It is the rulebook for test
|
|
> changes, written from failures this pipeline actually produced. Most load-bearing: set state up
|
|
> through the app's **own interface, never its database** (a plausible fixture that INSERTed rows
|
|
> passed on v2 and silently broke on v3 — 202 acks, rows in postgres, nothing ingested — and held the
|
|
> recipe RED for six weeks), **gate on version rather than supporting both** (old-version tests can be
|
|
> deleted; the older version is only exercised via the upgrade tier), and correct the fixture or the
|
|
> wait but **never the assertion**.
|
|
|
|
Work on **one recipe at a time** (serialize — each verification deploys a recipe on the shared
|
|
Swarm). For each `STALE_TESTS` entry:
|
|
|
|
**(a) Set up a dedicated cc-ci clone on a branch** (single-writer — never push `main`, never touch
|
|
the build loops' clones):
|
|
```
|
|
git clone ssh://git@git.autonomic.zone:2222/recipe-maintainers/cc-ci.git /home/loops/work/cc-ci-tests-<recipe>
|
|
cd /home/loops/work/cc-ci-tests-<recipe>
|
|
git fetch origin && git checkout main && git reset --hard origin/main
|
|
git checkout -b test/<recipe>-stale-test-$(date -u +%Y%m%d)
|
|
```
|
|
(Or reuse `/home/loops/work/cc-ci-fix` on a new branch if it's clean — the rule is a dedicated
|
|
branch off `main`, in a clone you're not racing the loops on.)
|
|
|
|
**(b) Read the failing test + the upstream change.** The test lives in
|
|
`tests/<recipe>/` (custom tests, overlays, fixtures). Read:
|
|
- the failing test (the sweep / carry-over names it),
|
|
- the recipe's current `compose.yml` + upstream changelog (use the per-recipe registry
|
|
`cc-ci-plan/upstream/<recipe>.md` for the release-notes URLs — the `/upgrade-all` diagnosis
|
|
already identified what changed),
|
|
- what the test currently asserts vs what the new version does.
|
|
|
|
**(c) Author the minimal update.** Change the test/overlay to assert the **new, correct** behavior
|
|
— not to weaken it. Examples of a legitimate stale-test fix:
|
|
- a config key was renamed upstream → update the overlay to the new key.
|
|
- a default flipped (e.g. `allow_uncategorized_topics` false→true in Discourse 3.5) → update the
|
|
assertion to the new default, OR make the test explicitly set the old value (preferred — the
|
|
test then asserts the behavior it cares about, not the default).
|
|
- an auth method was removed (e.g. Bearer-auth in impress v5.4.0) → drop that assertion, or
|
|
replace it with the new auth method's assertion (preferred if the new method is the successor).
|
|
- a readiness endpoint changed → update the readiness gate to the new endpoint.
|
|
|
|
**What is NOT allowed:** lowering an assertion's strictness to make a red go green, deleting a test
|
|
that still applies, skipping a tier that still applies, or changing a `pass` threshold. The fix
|
|
must make the test **correct for the new version**, not lenient. If you can't fix it without
|
|
weakening, stop and report `FAILED — test cannot be updated without weakening; needs operator
|
|
decision` (leave the recipe's stale-test PR comment as-is).
|
|
|
|
**(d) Lint the test change** (cc-ci has a lint gate):
|
|
```
|
|
ruff check tests/<recipe>/ && ruff format --check tests/<recipe>/
|
|
```
|
|
Fix any lint errors you introduce (don't commit red lint).
|
|
|
|
### 3. Open the cc-ci test PR (merged in step 4 once verified)
|
|
|
|
```
|
|
git add tests/<recipe>/
|
|
git commit -m "test(<recipe>): update stale test for <upstream-change>
|
|
|
|
<test-name> asserted <old-behavior>; <recipe> <new-version> now <new-behavior>.
|
|
Updated to assert the new behavior. No assertion weakened.
|
|
|
|
Stale-test fix for recipe PR <url> (carry-over from /upgrade-all <date>)."
|
|
git push -u origin HEAD
|
|
```
|
|
Open the PR via the helper:
|
|
```
|
|
set -a; . /srv/cc-ci/.testenv; set +a
|
|
TITLE="test(<recipe>): update stale test for <upstream-change>" \
|
|
BODY_FILE=/tmp/cc-ci-tests-<recipe>-pr-body.md \
|
|
bash /srv/cc-ci-orch/.claude/skills/ci-test-review/open-cc-ci-pr.sh
|
|
```
|
|
The PR body must: name the stale test, explain what changed upstream, state that no assertion was
|
|
weakened, and link the **dependent recipe upgrade PR** (the test change only goes green once the
|
|
recipe upgrade is also applied — they're a pair). The PR is opened for visibility and historical
|
|
record; it gets merged directly once verification is green (step 4).
|
|
|
|
Capture the `TEST_PR_URL`.
|
|
|
|
### 4. Verify the updated test passes (deterministic; the standard PR + !testme flow)
|
|
|
|
A test-update PR is only "working" once cc-ci verifies it green with the new test applied. There
|
|
are two verification paths — use whichever applies:
|
|
|
|
**(a) If the stale test was carry-over from an open recipe upgrade PR** — verify via the paired
|
|
`!testme` flow (the `/recipe-upgrade --with-tests` step-5b path). The recipe upgrade PR's `!testme`
|
|
uses the **deployed/main** cc-ci tests, so it can't see your unmerged test branch — you need to
|
|
run the harness with your cc-ci test branch checked out on cc-ci:
|
|
```
|
|
# 1. On cc-ci, check out your test branch in a deploy clone (NOT /root/builder-clone — use a
|
|
# separate checkout so the running server is unaffected):
|
|
ssh cc-ci 'rm -rf /root/cc-ci-test-verify && git clone --branch test/<recipe>-stale-test-<date> \
|
|
ssh://git@git.autonomic.zone:2222/recipe-maintainers/cc-ci.git /root/cc-ci-test-verify && \
|
|
cd /root/cc-ci-test-verify && git submodule update --init secrets 2>/dev/null || true'
|
|
# 2. Run the harness against the recipe upgrade PR head, with your test branch as the cc-ci checkout:
|
|
RECIPE=<recipe> REF=<recipe-upgrade-pr-head-branch> REMOTE_ROOT=/root/cc-ci-test-verify \
|
|
bash /srv/cc-ci-orch/.claude/skills/ci-test-review/verify-pr.sh
|
|
```
|
|
Green ⇔ the recipe upgrade passes under your corrected test. This is the **paired** verification
|
|
(the test change + the recipe upgrade together go green).
|
|
|
|
**(b) If the stale test is against a recipe already at its current upstream** (no upgrade PR — the
|
|
test is just stale against current main) — verify the test branch directly:
|
|
```
|
|
RECIPE=<recipe> REMOTE_ROOT=/root/cc-ci-test-verify \
|
|
bash /srv/cc-ci-orch/.claude/skills/ci-test-review/verify-pr.sh
|
|
```
|
|
(No `REF` — runs against the current recipe main, with your updated tests.)
|
|
|
|
**(c) Cleanup the verify clone on cc-ci** (mandatory — shared host):
|
|
```
|
|
ssh cc-ci 'rm -rf /root/cc-ci-test-verify'
|
|
```
|
|
|
|
**If verification is RED:** iterate the test fix on the same branch (**bounded — ≤3 attempts**) and
|
|
re-verify. If still red after 3, leave the test PR open and report `FAILED — test update not green
|
|
after 3 verify runs` with the failing evidence. Don't weaken the test to force green.
|
|
|
|
> **Re-verify with a real `!testme` on the recipe PR too?** The harness-branch verification in (a)
|
|
> is the authoritative pre-merge proof. After it's green, merge the test PR; if the paired recipe
|
|
> upgrade PR is still open, optionally post a fresh `!testme` on it (now running with the merged
|
|
> tests) as a belt-and-braces confirmation, and leave a comment noting the test PR is merged so
|
|
> the operator knows merging the recipe PR → green.
|
|
|
|
### 5. Tear down any dev deploys (defence-in-depth)
|
|
|
|
If your verification path (or diagnosis) deployed a `dev-<recipe>` stack on cc-ci, tear it down:
|
|
```
|
|
ssh cc-ci 'D=dev-<recipe>.ci.commoninternet.net; script -qec "abra app undeploy $D -n" /dev/null 2>&1 | tail -2; \
|
|
script -qec "abra app rm $D -n --no-input" /dev/null 2>&1 | tail -2'
|
|
ssh cc-ci 'docker stack ls --format "{{.Name}}" | grep -c "^dev-<recipe>"' # should be 0
|
|
```
|
|
(The `/upgrade-all` sweep would reap it next run anyway, but clean up yourself.)
|
|
|
|
### 6. Report — one line per recipe + summary
|
|
|
|
Write `/srv/cc-ci/.cc-ci-logs/tests-update-<YYYY-MM-DD>.md` and print, leading with the PR list:
|
|
```markdown
|
|
# cc-ci Stale-Test Update — <YYYY-MM-DD>
|
|
## Summary
|
|
- Swept: N recipes · Stale tests found: N · Test PRs opened + verified: N · Failed: N · Not-stale (dropped): N
|
|
## Test PRs (merged — links for after-the-fact review)
|
|
| recipe | stale test | upstream change | test PR (merged) | verified |
|
|
|--------|-----------|-----------------|------------------|----------|
|
|
| <recipe> | <test-name> | <what changed> | <url> | GREEN (paired with recipe PR <url>) |
|
|
## Failed (needs work)
|
|
- <recipe> — <test-name>: <reason> (test PR <url> left open, red)
|
|
## Not-stale (dropped — was flaky / real recipe bug / CI-server bug)
|
|
- <recipe> — <test-name>: <classification> (handed off / noted)
|
|
```
|
|
End with the report path and **links to every merged test PR + a summary of what changed in each**
|
|
(so the operator can review after the fact). Failed/red test PRs are left open, not merged; the
|
|
paired recipe upgrade PRs remain operator-owned and unmerged.
|
|
|
|
## Guardrails
|
|
|
|
- **Never weaken a test** to turn a red green. The fix makes the test correct for the new upstream
|
|
behavior; if that's not possible without weakening, stop and report.
|
|
- **Create for visibility, verify, then merge directly.** Each test PR is merged only after its
|
|
verification is green (the invocation is the authorization); red/unverified PRs stay open. The
|
|
paired recipe upgrade PRs are NOT yours to merge. Always report merged-PR links + summaries.
|
|
- **Single-writer:** dedicated branch in a separate clone; never push `main`; never touch the
|
|
build loops' `/cc-ci` `/cc-ci-adv` clones or `/root/builder-clone`. Use `/root/cc-ci-test-verify`
|
|
on the host and `rm -rf` it after.
|
|
- **Stale ≠ broken.** Only update a test when the **upgrade/recipe is correct** and the **test is
|
|
wrong** (asserts old behavior). A real upgrade regression is a `/recipe-upgrade` (5a) job; a
|
|
CI-server bug is a `/ci-test-review` job. This skill fixes tests, not recipes or harness.
|
|
- **Bounded fixes** — minimal update to the failing assertion/overlay, not a test rewrite.
|
|
- **Bounded retries** — ≤3 verify runs per test PR; then leave it open + red with evidence.
|
|
- **Flake ≠ stale** — re-run before classifying as stale; don't author a fix for a flake.
|
|
- **Serialize on the shared Swarm** — one recipe's verify deploy at a time; tear down before the
|
|
next. Don't run concurrent with `/upgrade-all` or active build loops.
|
|
- **Real abra path** throughout (no docker-level bypass); pseudo-TTY wrap every abra call.
|