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 lines
17 KiB
Markdown
273 lines
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.
|