diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..b7cd32a --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,28 @@ +version: 2 + +# Why this exists: the workflow's actions had drifted three majors behind - `actions/checkout` sat +# on v4 while v7 was current - and nobody noticed until GitHub started warning that the Node 20 +# runtime those versions target is being force-migrated to Node 24. Pinning without a bump +# mechanism only postpones that discovery to the day the fallback disappears. +# +# Deliberately limited to `github-actions`. The Docker ecosystem is *not* enabled: the App +# `Dockerfile` pulls its base from a private GitLab registry that Dependabot cannot authenticate +# against, so enabling it would produce recurring failures rather than updates. The base tag in +# `test/Dockerfile` is pinned on purpose (see the comment there) and is reviewed by hand. +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + day: monday + # just after the weekly fresh-resolution run at 04:17 UTC, so all CI housekeeping for the + # week surfaces in one place instead of trickling in on random days + time: '04:30' + timezone: Etc/UTC + # one pull request for all action bumps, not one per action - each PR costs a full CI run + groups: + actions: + patterns: + - '*' + commit-message: + prefix: ci diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6a29465..85c6d36 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,6 +1,24 @@ name: test -on: [push] +on: + push: + branches: [main] + pull_request: + schedule: + # Mondays, 04:17 UTC. Off the hour on purpose: GitHub's scheduler is congested at :00 and + # delays or silently drops runs queued there. + - cron: '17 4 * * 1' + # so a fresh resolution can be checked after a suspected break, instead of waiting for Monday + workflow_dispatch: + +# `true` exactly for the runs that must resolve the conda environment from scratch. +# `test/Dockerfile` resolves the environment in its own layer - correctly, it is what keeps a +# source-only change cheap - but since v2.2.0 that environment is deliberately open: only `pandas` +# carries a floor and a ceiling. So a cached layer lets this suite report green against a +# resolution nobody has re-checked, and nothing evicts it on its own, because every push's +# `cache-from` resets GitHub's seven-day idle timer. +env: + FRESH: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' }} jobs: test: @@ -8,19 +26,34 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + # GitHub's default is 360 minutes. That was harmless while every run was a cache hit and + # finished in seconds, but the weekly run now resolves conda for real - and a solver that + # hangs would otherwise burn six hours of runner time, once a week, unattended. + timeout-minutes: 30 steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Setup Docker buildx - uses: docker/setup-buildx-action@v3 + uses: docker/setup-buildx-action@v4 - name: Test via build - uses: docker/build-push-action@v6 + uses: docker/build-push-action@v7 with: context: . file: ./test/Dockerfile push: false - cache-from: type=gha - cache-to: type=gha,mode=max + # `no-cache` alone ought to outrank an imported cache, but that precedence is documented + # nowhere, and it is the one assumption this whole guard rests on - so the fresh runs + # drop `cache-from` as well and the question stops mattering. + no-cache: ${{ env.FRESH == 'true' }} + # Do NOT "simplify" this to `env.FRESH == 'true' && '' || 'type=gha'`. `''` is falsy in + # GitHub expressions, so the `||` branch would always win and both paths would quietly + # get the cache back - reinstating the exact false green this exists to prevent. + cache-from: ${{ env.FRESH != 'true' && 'type=gha' || '' }} + # `ignore-error`: a pull request from a fork runs with a read-only token and cannot write + # the Actions cache, so the export 403s. Without this the run fails at the very end, + # after the tests have already passed. Import is unaffected - a PR may read the cache of + # its own branch, the base branch and the default branch - so fork PRs stay warm. + cache-to: type=gha,mode=max,ignore-error=true diff --git a/.gitignore b/.gitignore index 63f55f5..65e1ae6 100644 --- a/.gitignore +++ b/.gitignore @@ -359,7 +359,12 @@ vignettes/*.pdf .Renviron # pkgdown site -docs/ +# `docs/*` rather than `docs/`: the rule covers *generated* pkgdown output, and this repo has no +# pkgdown site at all - it inherits the rule from the R gitignore template. Ignoring the directory +# itself would stop git descending into it, and the negation below could never take effect. +docs/* +# hand-written design docs, which are sources and belong in the repo +!docs/superpowers/ # translation temp files po/*~ diff --git a/docs/superpowers/plans/2026-08-04-ci-fresh-resolution-and-pr-trigger.md b/docs/superpowers/plans/2026-08-04-ci-fresh-resolution-and-pr-trigger.md new file mode 100644 index 0000000..8110d6f --- /dev/null +++ b/docs/superpowers/plans/2026-08-04-ci-fresh-resolution-and-pr-trigger.md @@ -0,0 +1,505 @@ +# CI: fresh resolution weekly, and test pull requests — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `.github/workflows/test.yml` resolve the conda environment from scratch on a weekly +schedule and on demand, and run on pull requests so fork contributions are tested. + +**Architecture:** One workflow-level `env.FRESH` flag, derived from `github.event_name`, drives the +cache inputs of the single existing build step. Scheduled and dispatched runs set `no-cache: true` +*and* drop `cache-from`; push and pull-request runs keep the cache and stay fast. No new jobs, no +new steps, no new files. + +**Tech Stack:** GitHub Actions, `docker/build-push-action@v6`, BuildKit GHA cache backend +(`type=gha`), conda (`condaforge/miniforge3`). Verification uses `actionlint` via Docker, `yq`, and +`gh`. + +**Spec:** [`docs/superpowers/specs/2026-08-04-ci-fresh-resolution-and-pr-trigger-design.md`](../specs/2026-08-04-ci-fresh-resolution-and-pr-trigger-design.md) +**Ticket:** [movestore-groundcontrol#7](https://gitlab.com/couchbits/movestore/movestore-groundcontrol/-/work_items/7) + +## Global Constraints + +- Branch: `claude/work-item-7-908f21`, based on `origin/main` = `b667d21` (= tag `v2.2.0`). +- The only production file this plan changes is `.github/workflows/test.yml`. Do not touch + `test/Dockerfile`, `python/environment.yml`, or the pins in either. +- `permissions:` stays `contents: read`. Do not widen it. +- The trigger is `pull_request`, **never** `pull_request_target`. +- Cron is `'17 4 * * 1'` — quoted, and deliberately not on the hour. +- `cache-from` expression must keep the non-empty string on the truthy side of the `&&`. The + inverted-looking form is correct; the "natural" form is silently broken. See Task 1. +- Commit messages: imperative subject, body explains *why*, and end with + `Co-Authored-By: Claude Opus 5 `. +- Tasks 2–4 push to the public repo `movestore/link-r-python`. Each is outward-facing and must be + confirmed with the user before running — do not chain them automatically. + +## Sequencing constraint (read before planning your order) + +GitHub only offers `workflow_dispatch` and `schedule` for a workflow file that exists **on the +default branch**. Therefore: + +- The `pull_request` trigger can be verified from the PR branch (Task 2) — `pull_request` uses the + workflow as defined in the PR. +- The `push: branches: [main]` trigger can only be verified on merge (Task 3). +- The fresh/no-cache path can only be verified **after** merge (Task 4). + +Do not try to `gh workflow run` from the feature branch; it will fail with +`Workflow does not have 'workflow_dispatch' trigger` and that is expected, not a bug in the change. + +## File Structure + +| File | Change | Responsibility | +| --- | --- | --- | +| `.github/workflows/test.yml` | Modify (whole file rewritten) | Decides *when* the suite runs and *whether* the conda layer may come from cache | +| `docs/superpowers/specs/2026-08-04-…-design.md` | Already committed (`afd7270`) | Records the reasoning | +| `docs/superpowers/plans/2026-08-04-…md` | This file | — | + +--- + +### Task 1: Rewrite the workflow and gate it locally + +**Files:** +- Modify: `.github/workflows/test.yml` (whole file — 27 lines in, 45 out) +- Test: no test file. A workflow has no unit-testable surface; the gates are `actionlint` (schema + and context correctness) and `yq` assertions (the specific expression that has a silent-failure + mode). Both are commands, given verbatim below. + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: a workflow-level env var `FRESH`, a string that is exactly `"true"` on `schedule` and + `workflow_dispatch` runs and exactly `"false"` on `push` and `pull_request` runs. Tasks 2–4 + assert against that contract and against the resulting `buildx` command line. + +- [ ] **Step 1: Establish the pre-change baseline is green** + +`actionlint` must pass *before* the edit, otherwise a failure afterwards is ambiguous. + +```bash +docker run --rm -v "$PWD":/repo -w /repo rhysd/actionlint:latest -color +``` + +Expected: no output, exit code 0. + +- [ ] **Step 2: Write the new workflow** + +Replace the entire contents of `.github/workflows/test.yml` with: + +```yaml +name: test + +on: + push: + branches: [main] + pull_request: + schedule: + # Mondays, 04:17 UTC. Off the hour on purpose: GitHub's scheduler is congested at :00 and + # delays or silently drops runs queued there. + - cron: '17 4 * * 1' + # so a fresh resolution can be checked after a suspected break, instead of waiting for Monday + workflow_dispatch: + +# `true` exactly for the runs that must resolve the conda environment from scratch. +# `test/Dockerfile` resolves the environment in its own layer - correctly, it is what keeps a +# source-only change cheap - but since v2.2.0 that environment is deliberately open: only `pandas` +# carries a floor and a ceiling. So a cached layer lets this suite report green against a +# resolution nobody has re-checked, and nothing evicts it on its own, because every push's +# `cache-from` resets GitHub's seven-day idle timer. +env: + FRESH: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' }} + +jobs: + test: + name: Unit tests + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup Docker buildx + uses: docker/setup-buildx-action@v3 + + - name: Test via build + uses: docker/build-push-action@v6 + with: + context: . + file: ./test/Dockerfile + push: false + # `no-cache` alone ought to outrank an imported cache, but that precedence is documented + # nowhere, and it is the one assumption this whole guard rests on - so the fresh runs + # drop `cache-from` as well and the question stops mattering. + no-cache: ${{ env.FRESH == 'true' }} + # Do NOT "simplify" this to `env.FRESH == 'true' && '' || 'type=gha'`. `''` is falsy in + # GitHub expressions, so the `||` branch would always win and both paths would quietly + # get the cache back - reinstating the exact false green this exists to prevent. + cache-from: ${{ env.FRESH != 'true' && 'type=gha' || '' }} + # `ignore-error`: a pull request from a fork runs with a read-only token and cannot write + # the Actions cache, so the export 403s. Without this the run fails at the very end, + # after the tests have already passed. Import is unaffected - a PR may read the cache of + # its own branch, the base branch and the default branch - so fork PRs stay warm. + cache-to: type=gha,mode=max,ignore-error=true +``` + +- [ ] **Step 3: Run actionlint and verify it still passes** + +This is the step that catches the two things most likely to be wrong: whether the `env` context is +actually allowed inside a step's `with:`, and whether every expression is well-typed. + +```bash +docker run --rm -v "$PWD":/repo -w /repo rhysd/actionlint:latest -color +``` + +Expected: no output, exit code 0. +If it reports `context "env" is not allowed here`, stop — the design's single-step approach is +unavailable and the fallback is two `if:`-guarded steps. Report back rather than improvising. + +- [ ] **Step 4: Assert the triggers parsed as intended** + +```bash +yq '.on | keys' .github/workflows/test.yml +yq '.on.push.branches' .github/workflows/test.yml +yq '.on.schedule[0].cron' .github/workflows/test.yml +``` + +Expected, in order: + +``` +- push +- pull_request +- schedule +- workflow_dispatch +``` +``` +- main +``` +``` +17 4 * * 1 +``` + +Note: `pull_request:` with an empty value parses as `null`. That is correct YAML for "all activity +types, all branches" — do not "fix" it by giving it a body. + +- [ ] **Step 5: Assert the trap expression survived verbatim** + +This is the regression guard for the one line with a silent-failure mode. + +```bash +yq -r '.jobs.test.steps[] | select(.name == "Test via build") | .with."cache-from"' .github/workflows/test.yml +``` + +Expected, character for character: + +``` +${{ env.FRESH != 'true' && 'type=gha' || '' }} +``` + +If it reads `env.FRESH == 'true' && '' || 'type=gha'`, the change is broken in the specific way the +spec warns about — both events would get `type=gha`. Rewrite it. + +- [ ] **Step 6: Confirm `pull_request_target` did not sneak in** + +```bash +grep -c 'pull_request_target' .github/workflows/test.yml || true +``` + +Expected: `0`. + +- [ ] **Step 7: Commit** + +```bash +git add .github/workflows/test.yml +git commit -F - <<'EOF' +Resolve the environment fresh weekly, and test pull requests + +`test/Dockerfile` resolves conda in its own layer, and since v2.2.0 that +environment is open by design - only `pandas` is pinned. So a cached layer let +the suite pass against a resolution nobody had re-checked, and nothing evicted +it: every push's `cache-from` reset GitHub's seven-day idle timer. Scheduled +and dispatched runs now build with `no-cache` and without `cache-from`, then +publish the fresh result, which also caps how stale an ordinary run can be. + +Pull requests were not tested at all. For a repo meant to be copied, a fork PR +is the likelier origin of a change than a branch here, so `pull_request` is now +a trigger and `cache-to` tolerates the 403 a fork's read-only token earns. + +`push` narrows to `main` so a branch with an open PR stops building twice. + +Refs couchbits/movestore/movestore-groundcontrol#7 + +Co-Authored-By: Claude Opus 5 +EOF +``` + +--- + +### Task 2: Verify the `pull_request` trigger against real GitHub + +**⚠️ Outward-facing — confirm with the user before Step 1.** This pushes a branch to the public +repo `movestore/link-r-python` and opens a pull request. + +**Files:** none changed. This task only observes. + +**Interfaces:** +- Consumes: `FRESH` from Task 1, expected `"false"` on this event. +- Produces: evidence that the PR path is green and that `cache-to` does not fail a run. + +- [ ] **Step 1: Push the branch** + +```bash +git push -u origin claude/work-item-7-908f21 +``` + +Expected: branch created. No workflow run starts — `push` is scoped to `main` now, and that +silence is itself the first confirmation that Task 1 landed correctly. + +- [ ] **Step 2: Confirm the push did NOT trigger a run** + +```bash +gh run list --branch claude/work-item-7-908f21 --limit 5 +``` + +Expected: no run with event `push`. If a push run appears, `on.push.branches` is wrong. + +- [ ] **Step 3: Open the pull request** + +```bash +gh pr create --repo movestore/link-r-python --base main \ + --title 'CI: resolve the environment fresh weekly, and test pull requests' \ + --body 'Closes the CI half of couchbits/movestore/movestore-groundcontrol#7. + +Since `v2.2.0` the conda environment is open by design - only `pandas` carries a floor and a +ceiling. `test/Dockerfile` resolves it in its own layer, so while that layer came from the Actions +cache the suite reported green against a resolution nobody had re-checked. Nothing evicted it +either: the cache drops entries idle for seven days, and every push refreshed the timer. + +Scheduled (Mondays 04:17 UTC) and manually dispatched runs now build with `no-cache` and without +`cache-from`, then publish the fresh resolution - so ordinary runs test an environment at most a +week old rather than an arbitrarily old one. + +Separately, the workflow did not trigger on `pull_request`, so fork contributions never ran. +It does now, with `ignore-error=true` on the cache export because a fork'"'"'s read-only token +cannot write the Actions cache. + +Design notes: `docs/superpowers/specs/2026-08-04-ci-fresh-resolution-and-pr-trigger-design.md` + +🤖 Generated with [Claude Code](https://claude.com/claude-code)' +``` + +- [ ] **Step 4: Watch the run and confirm it is green** + +```bash +gh run watch "$(gh run list --branch claude/work-item-7-908f21 --event pull_request --limit 1 --json databaseId --jq '.[0].databaseId')" --exit-status +``` + +Expected: exit 0. A run exists at all — that is the fix for problem 2 in the ticket. + +- [ ] **Step 5: Confirm this run used the cache (i.e. `FRESH` was false)** + +```bash +gh run view "$(gh run list --branch claude/work-item-7-908f21 --event pull_request --limit 1 --json databaseId --jq '.[0].databaseId')" --log | grep -E 'buildx build|--no-cache|--cache-from' | head -20 +``` + +Expected: the buildx invocation contains `--cache-from type=gha` and does **not** contain +`--no-cache`. This is the assertion that `FRESH` resolved to `"false"` — read from the real +expression engine, not from a local re-implementation of it. + +- [ ] **Step 6: Confirm the cache export did not fail the run** + +```bash +gh run view "$(gh run list --branch claude/work-item-7-908f21 --event pull_request --limit 1 --json databaseId --jq '.[0].databaseId')" --log | grep -iE 'cache export|ignore-error|403' | head -20 +``` + +Expected: either nothing, or a non-fatal warning. A same-repo PR *can* write the cache, so a 403 is +not expected here; this step establishes the baseline for the fork case, which cannot be tested +without an actual fork. Record what you see. + +- [ ] **Step 7: No commit.** This task changes no files. + +--- + +### Task 3: Merge, and verify the `push: branches: [main]` trigger + +**⚠️ Outward-facing — confirm with the user before Step 1.** This merges to the default branch of +a public repo. + +**Files:** none changed. + +- [ ] **Step 1: Merge the pull request** + +```bash +gh pr merge --repo movestore/link-r-python --merge --delete-branch=false +``` + +Use `--merge`, not `--squash`: every previous change landed here as a merge commit (#6, #8, #9, +#10, #12) and the plan should not silently change the repo's history style. + +- [ ] **Step 2: Confirm the merge triggered a run on main** + +```bash +gh run list --repo movestore/link-r-python --branch main --event push --limit 3 +``` + +Expected: a new run, event `push`. This confirms narrowing `push` to `main` did not disable it. + +- [ ] **Step 3: Confirm it is green** + +```bash +gh run watch "$(gh run list --repo movestore/link-r-python --branch main --event push --limit 1 --json databaseId --jq '.[0].databaseId')" --exit-status +``` + +Expected: exit 0. + +- [ ] **Step 4: No commit.** + +--- + +### Task 4: Verify the fresh path actually rebuilds the conda layer + +**⚠️ Outward-facing — confirm with the user before Step 1.** Only possible after Task 3, because +`workflow_dispatch` requires the file on the default branch. + +This is the task that proves the ticket is actually fixed. Everything before it proves the +plumbing; this proves the water runs. + +**Files:** none changed. + +**Interfaces:** +- Consumes: `FRESH` from Task 1, expected `"true"` on this event. + +- [ ] **Step 1: Confirm the workflow is now dispatchable** + +```bash +gh workflow list --repo movestore/link-r-python +``` + +Expected: `test` is listed. If it is not, the file did not reach `main`. + +- [ ] **Step 2: Dispatch a run on main** + +```bash +gh workflow run test --repo movestore/link-r-python --ref main +``` + +- [ ] **Step 3: Wait for it and capture the id** + +```bash +sleep 10 +RUN_ID="$(gh run list --repo movestore/link-r-python --event workflow_dispatch --limit 1 --json databaseId --jq '.[0].databaseId')" +echo "$RUN_ID" +gh run watch "$RUN_ID" --exit-status +``` + +Expected: exit 0. If it **fails**, that is not necessarily a defect in this change — a fresh +resolution genuinely breaking is the exact signal the ticket asked for. Read the failure before +concluding anything, and report which of the two it is. + +- [ ] **Step 4: Assert the fresh flags reached buildx** + +```bash +gh run view "$RUN_ID" --repo movestore/link-r-python --log | grep -E 'buildx build' | head -5 +``` + +Expected: the command line contains `--no-cache` and contains **no** `--cache-from`. Both halves +matter — `--no-cache` is the intent, the absent `--cache-from` is the belt-and-braces the spec +argued for. + +- [ ] **Step 5: Assert the conda layer was genuinely rebuilt** + +```bash +gh run view "$RUN_ID" --repo movestore/link-r-python --log | grep -E 'conda env create|CACHED' | head -20 +``` + +Expected: the `RUN conda env create` step appears **without** a `CACHED` marker, and takes minutes +rather than seconds. This is the end-to-end proof: a fresh resolution really happened. If it shows +`CACHED`, the change does not fix the ticket, no matter how correct the YAML looks. + +- [ ] **Step 6: Confirm the fresh result was published to the cache** + +```bash +gh run view "$RUN_ID" --repo movestore/link-r-python --log | grep -iE 'exporting cache|importing cache' | head -10 +``` + +Expected: an "exporting cache" section. This is what caps the staleness of subsequent runs at +roughly a week. + +- [ ] **Step 7: No commit.** + +--- + +### Task 5: Report back on the ticket + +**⚠️ Outward-facing — confirm with the user before running.** Posts to GitLab. + +- [ ] **Step 1: Comment on the work item with the evidence and the limitation** + +```bash +glab api --method POST \ + "projects/couchbits%2Fmovestore%2Fmovestore-groundcontrol/issues/7/notes" \ + --field 'body=Umgesetzt in `movestore/link-r-python` auf `main`. + +- Wöchentlich montags 04:17 UTC plus `workflow_dispatch`: Build mit `no-cache` **und** ohne + `cache-from`, danach wird die frische Auflösung in den Cache geschrieben - damit testet auch ein + gewöhnlicher Push eine Umgebung, die höchstens eine Woche alt ist. +- `pull_request` ist jetzt Trigger; `cache-to` hat `ignore-error=true`, weil der Read-only-Token + eines Forks den Actions-Cache nicht schreiben darf und der Export sonst am Ende 403 wirft. +- `push` ist auf `main` eingegrenzt, sonst baut ein Branch mit offenem PR jedes Mal doppelt. + +Nachweis: der dispatchte Lauf baut `conda env create` ohne `CACHED`-Marker neu. + +Bekannte Einschränkung, die die Prämisse des Tickets teilweise aushebelt: GitHub deaktiviert +`schedule:`-Trigger in Repos ohne Commits seit 60 Tagen. Der wöchentliche Kanarienvogel verstummt +dann lautlos - kein rotes Build, sondern gar kein Signal. Innerhalb von Actions gibt es dafür keine +saubere Lösung; das bräuchte einen externen Ping. Ebenso gehen Fehlermeldungen geplanter Läufe nur +an die Person, die die Cron-Zeile zuletzt geändert hat.' +``` + +- [ ] **Step 2: Ask the user whether to close the issue or leave it open** for the 60-day + limitation follow-up. Do not close it unilaterally. + +--- + +## Addendum — scope extended after Task 2 + +Task 2 came back green but the log showed two things worth acting on, so the following were folded +into the same pull request rather than deferred. The spec section *Carried in the same change* +records the reasoning; this note exists so the plan is not a misleading record of what shipped. + +- Action majors bumped: `actions/checkout` v4 → v7, `docker/setup-buildx-action` v3 → v4, + `docker/build-push-action` v6 → v7 (every run warned about the Node 20 runtime). +- `.github/dependabot.yml` added, `github-actions` only, grouped into one pull request. +- `timeout-minutes: 30` on the job. +- Spec updated with the finding that the *test* layer caches too, not only the conda layer — so a + green check on an unchanged tree executes no tests at all. + +The verification steps in Tasks 3 and 4 are unchanged and still apply. Task 4 Step 5 gains a second +meaning: it now also confirms the bumped actions work on a real build. + +## Self-Review + +**Spec coverage:** + +| Spec section | Task | +| --- | --- | +| Triggers (`push`→main, `pull_request`, `schedule`, `workflow_dispatch`) | 1 (Step 2), verified 2 (Step 2), 3 (Step 2), 4 (Step 1) | +| `env.FRESH` derived from event | 1 (Step 2), verified 2 (Step 5), 4 (Step 4) | +| `cache-from` dropped on fresh runs | 1 (Step 2), verified 4 (Step 4) | +| Inverted ternary, with the trap documented | 1 (Steps 2, 5) | +| `ignore-error=true` for fork PRs | 1 (Step 2), verified 2 (Step 6) | +| Weekly run rewrites the cache | 1 (Step 2), verified 4 (Step 6) | +| `permissions` unchanged, no `pull_request_target` | 1 (Step 2, Step 6) | +| Out of scope: no `concurrency`, no pin changes | Global Constraints | +| Known limitation: 60-day schedule disablement | 5 (Step 1) | + +No spec requirement is unimplemented. + +**Placeholder scan:** none. Every command is literal and every expected output is stated. + +**Type consistency:** `FRESH` is referenced as a string compared against `'true'` in all three +places it appears (`no-cache`, `cache-from`, and both verification tasks). The `env.FRESH != 'true'` +in `cache-from` is an intentional inversion, not an inconsistency — Task 1 Step 5 pins it. + +**Known risk the plan cannot eliminate:** the true fork-PR path (read-only token → cache-export +403) is not exercised anywhere, because that needs a PR from an actual fork. Task 2 Step 6 records +the same-repo baseline so the difference is recognisable if it ever bites. `ignore-error=true` is +the mitigation and it is documented; this is accepted, not overlooked. diff --git a/docs/superpowers/specs/2026-08-04-ci-fresh-resolution-and-pr-trigger-design.md b/docs/superpowers/specs/2026-08-04-ci-fresh-resolution-and-pr-trigger-design.md new file mode 100644 index 0000000..00160b5 --- /dev/null +++ b/docs/superpowers/specs/2026-08-04-ci-fresh-resolution-and-pr-trigger-design.md @@ -0,0 +1,213 @@ +# CI: guarantee a fresh environment resolution, and test pull requests + +Date: 2026-08-04 +Ticket: [movestore-groundcontrol#7](https://gitlab.com/couchbits/movestore/movestore-groundcontrol/-/work_items/7) +Touches: `.github/workflows/test.yml` + +## Problem + +`test/Dockerfile` resolves the conda environment in its own layer, before the sources are copied: + +```dockerfile +COPY python/environment.yml ./python/ +RUN conda env create --prefix ${ENV_PREFIX} --file python/environment.yml && \ + conda clean --all --yes +``` + +That layering is correct — it is what keeps a source-only change cheap. It is also what makes the +CI result untrustworthy, because since `v2.2.0` the environment is deliberately open: + +```yaml +- python +- pandas>=2.3.3,<3 +- geopandas +- movingpandas +``` + +Only `pandas` carries a floor and a ceiling; everything else floats. So "does this still resolve, +and does it still pass?" is a question with a moving answer, and the CI only ever asks it when the +cache misses. As long as the `conda env create` layer is served from the GitHub Actions cache, the +suite runs against a resolution that may be months old and reports green. + +Nothing evicts that layer on its own. The Actions cache drops entries untouched for seven days, but +every push reads it via `cache-from`, which resets the timer. In an active repo the stale layer +survives indefinitely — until `environment.yml` changes, which is exactly the moment nobody is +watching for an unrelated dependency to have moved underneath. + +This is not hypothetical. During rollout an App build pulled its conda layer from the kaniko cache +and completed "successfully" against the old environment. + +### It is worse than the ticket says: the test layer caches too + +Confirmed empirically on the first `pull_request` run of this change +([run 30901212941](https://github.com/movestore/link-r-python/actions/runs/30901212941), 21 seconds, +green): + +``` +#10 [4/6] RUN conda env create --prefix /opt/link-r-python/conda --file python/environment.yml +#10 CACHED +#12 [6/6] RUN conda run ... python -m unittest python.tests.test_transform_to_pickle ... +#12 CACHED +``` + +The ticket describes the *resolution* going stale. But `unittest` runs in a `RUN` layer like any +other, so on an unchanged tree it is cached as well and no test executes at all. A green check +means "nothing needed rebuilding", not "the tests passed". + +For a change that touches only `.github/` or `docs/` that is the correct outcome — there is nothing +to retest. And coverage of real changes is intact: touching `python/` invalidates the `COPY` layer, +touching `environment.yml` invalidates the conda layer, and in both cases the tests re-run. So this +does not call for a fix of its own. + +It does, however, change what the weekly run is *for*. It is not only the one run that re-resolves +the environment; it is the one run that executes the test suite at all against an unchanged tree. +That makes it the sole periodic evidence that the App's environment still works — which is a +stronger reason to have it than the ticket claimed. + +Second, separate problem: the workflow triggers on `push` only. Pull requests from forks never run. +For a template repo whose whole purpose is to be copied by external contributors, a fork PR is the +*likelier* origin of a change than a branch in this repo. + +## Design + +### Triggers + +```yaml +on: + push: + branches: [main] + pull_request: + schedule: + - cron: '17 4 * * 1' + workflow_dispatch: +``` + +`push` is narrowed to `main`. Without that, a branch in this repo with an open PR builds twice on +every push — the same Docker build, twice the minutes, twice the cache writes. Everything in this +repo lands through a PR (#6, #8, #9, #10, #12), so branch coverage is not lost, only deduplicated. + +The cron sits at `:17`, not `:00`. GitHub's scheduler is congested on the hour and delays or drops +runs queued there. + +`workflow_dispatch` exists so a fresh resolution can be checked on demand — after a suspected +upstream break, or to confirm a fix — instead of waiting up to a week for the next Monday. + +### One flag, derived from the event + +```yaml +env: + FRESH: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' }} +``` + +```yaml + no-cache: ${{ env.FRESH == 'true' }} + cache-from: ${{ env.FRESH != 'true' && 'type=gha' || '' }} + cache-to: type=gha,mode=max,ignore-error=true +``` + +Scheduled and manually dispatched runs resolve from scratch. Pushes and pull requests keep using +the cache and stay fast. + +Three decisions here are not self-evident from the diff: + +**`cache-from` is dropped on fresh runs, not merely overridden by `no-cache`.** `--no-cache` ought +to take precedence over an imported cache, but that precedence is not documented either way, and it +is the single assumption the whole ticket rests on. Removing the import removes the question. The +belt is cheap; the braces are the point. + +**The ternary is inverted deliberately.** The natural spelling is a trap: + +```yaml +cache-from: ${{ env.FRESH == 'true' && '' || 'type=gha' }} # WRONG +``` + +`''` is falsy in GitHub expressions, so `true && ''` yields `''`, the `||` branch wins, and the +result is `type=gha` on *both* paths. That failure is silent and it reintroduces precisely the +false-green this change exists to remove. Keeping the non-empty string on the truthy side is what +makes the expression work; do not "simplify" it back. + +**`ignore-error=true` on the export.** A fork PR runs with a read-only token and cannot write the +Actions cache, so `cache-to` fails with a 403. Without this attribute the build dies at the very +end, after the tests have already passed — a red check for a green suite. Import is unaffected: a +PR run may read the cache of its own branch, the base branch, and the default branch, so fork PRs +still get a warm build. The attribute is documented for the `gha` backend. + +The trade-off accepted: a genuine cache-export failure on `main` is now also swallowed. That is the +right way round, because a failed cache write says nothing about whether the code is correct. + +### Letting the weekly run write the cache + +The scheduled run keeps `cache-to`. It resolves fresh, then publishes that resolution as the new +lineage, so ordinary pushes test an environment at most seven days old rather than an arbitrarily +old one. This turns the weekly job from a pure detector into something that also bounds the +staleness of every other run. + +### Unchanged + +`permissions: contents: read` stays. Cache export authenticates through the Actions runtime token, +not the `permissions` block, so no widening is needed. + +The trigger is `pull_request`, **not** `pull_request_target`. `pull_request` runs fork code with a +read-only token and no secrets, which is the whole reason it is safe to build arbitrary contributor +Dockerfiles. `pull_request_target` would hand that same untrusted code a writable token. + +### Carried in the same change + +Three things surfaced while verifying the above and are fixed here rather than deferred, because +all three are about the same thing: whether a green check can be believed. + +**Action majors.** `actions/checkout` v4 → v7, `docker/setup-buildx-action` v3 → v4, +`docker/build-push-action` v6 → v7. Every run was warning that these target Node 20 and are being +force-migrated to Node 24. All three majors are substantively just that runtime switch; the one +real breaking change, `checkout` v7 refusing to check out fork code under `pull_request_target` and +`workflow_run`, cannot affect us because we use neither — and it hardens the same boundary this +design already chose. + +**`.github/dependabot.yml`.** The bump above is the symptom; the absence of an update mechanism is +the cause. Three majors of drift went unnoticed until GitHub started warning. Scoped to +`github-actions` only and grouped into a single pull request. The Docker ecosystem is deliberately +left off: the App `Dockerfile` pulls from a private GitLab registry Dependabot cannot authenticate +against, so enabling it would generate recurring failures rather than updates. + +**`timeout-minutes: 30`.** GitHub's default is 360. Harmless while every run was a cache hit +finishing in 21 seconds — but the weekly run now resolves conda for real, unattended, and a hung +solver would otherwise burn six hours of runner time every Monday. + +## Out of scope + +**No `concurrency` block.** With `push` limited to `main` the duplicate-run problem is already +solved, and `cancel-in-progress` on a build that writes cache risks interrupting an export and +leaving a partial lineage. + +**No change to the pins in `environment.yml`.** The `pandas<3` ceiling has its own ticket. + +**No R tests.** Worth stating plainly because it is easy to miss: there are none. `find r -iname +'*test*'` is empty, so CI covers the Python half of this translator App and nothing else. Writing +them is a project, not a rider on a CI ticket — but the gap should not be discovered by accident. + +**`environment.dev.yml` is still never built.** It is kept "in step with `environment.yml`" by hand +and can rot silently. Building it weekly too would roughly double the scheduled run's cost for a +file no App ever uses at runtime. + +**No failure-to-issue reporting for scheduled runs.** It is the only measure that would fully +deliver the ticket's premise — see Known limitations — but it needs `issues: write`, a +`github-script` stage and deduplication, which is a different kind of change from this one. + +## Known limitations + +**GitHub disables `schedule:` triggers in a repo with no commits for 60 days.** A template repo can +easily go that quiet. The canary then stops silently — the failure mode is *no signal*, not a red +build, which is the worse of the two. There is no clean fix inside Actions; it needs an external +ping or a periodic human check. Worth knowing, because the ticket's premise is "we hear it from CI +and not from an App build", and a disabled schedule quietly voids that. + +**Scheduled-run failure notifications go to whoever last edited the cron line**, not to repo +watchers. If that person leaves, the alert leaves with them. + +## Verification + +- The workflow parses against the GitHub Actions schema. +- `FRESH` evaluates `true` for `schedule` / `workflow_dispatch` and `false` for `push` / + `pull_request`, and `cache-from` is empty in exactly the `true` case — this is the expression + that has a silent-failure mode, so it is checked by evaluating it, not by reading it. +- A dispatched run rebuilds the `conda env create` layer instead of reporting `CACHED`.