From afd7270a13df4a9dec3c9996bd6bf1968453e056 Mon Sep 17 00:00:00 2001 From: Clemens Hahn Date: Tue, 4 Aug 2026 12:23:33 +0200 Subject: [PATCH 1/4] Spec the CI change: fresh resolution weekly, and test PRs The environment is open by design since v2.2.0, so a cached `conda env create` layer lets the suite report green against a resolution nobody has re-checked. The spec records why the fresh runs drop `cache-from` outright rather than trusting `no-cache` to outrank it, and why the cache-from ternary is inverted. Narrows the inherited pkgdown `docs/` rule so the spec is trackable; this repo has no pkgdown site, and the rule was only ever meant for generated output. Refs couchbits/movestore/movestore-groundcontrol#7 Co-Authored-By: Claude Opus 5 --- .gitignore | 7 +- ...-fresh-resolution-and-pr-trigger-design.md | 152 ++++++++++++++++++ 2 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 docs/superpowers/specs/2026-08-04-ci-fresh-resolution-and-pr-trigger-design.md 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/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..6e4b6fd --- /dev/null +++ b/docs/superpowers/specs/2026-08-04-ci-fresh-resolution-and-pr-trigger-design.md @@ -0,0 +1,152 @@ +# 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. + +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. + +## 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. + +## 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`. From fcc3e3942e3b864285c93378181c76436ff04658 Mon Sep 17 00:00:00 2001 From: Clemens Hahn Date: Tue, 4 Aug 2026 12:28:26 +0200 Subject: [PATCH 2/4] Plan the CI change task by task Records the sequencing constraint that shapes the order: `workflow_dispatch` and `schedule` only exist for a workflow already on the default branch, so the fresh-resolution path cannot be verified until after the merge, while `pull_request` verifies straight from the PR branch. Refs couchbits/movestore/movestore-groundcontrol#7 Co-Authored-By: Claude Opus 5 --- ...8-04-ci-fresh-resolution-and-pr-trigger.md | 489 ++++++++++++++++++ 1 file changed, 489 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-04-ci-fresh-resolution-and-pr-trigger.md 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..a7351c1 --- /dev/null +++ b/docs/superpowers/plans/2026-08-04-ci-fresh-resolution-and-pr-trigger.md @@ -0,0 +1,489 @@ +# 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. + +--- + +## 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. From 45b19bad8452499787cab5af04e222b702dda5eb Mon Sep 17 00:00:00 2001 From: Clemens Hahn Date: Tue, 4 Aug 2026 12:31:32 +0200 Subject: [PATCH 3/4] 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 --- .github/workflows/test.yml | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6a29465..8e322e8 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: @@ -22,5 +40,16 @@ jobs: 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 From a36a67b64b350dc534027a28a591c9f1b090d0b5 Mon Sep 17 00:00:00 2001 From: Clemens Hahn Date: Tue, 4 Aug 2026 13:43:44 +0200 Subject: [PATCH 4/4] Trust the green check: bump the actions, bound the run, watch the pins Verifying the trigger change surfaced three things in the same blind spot, so they land together. Every run warned that `actions/checkout@v4`, `docker/setup-buildx-action@v3` and `docker/build-push-action@v6` target Node 20 and are being force-migrated to Node 24. All three majors are substantively that runtime switch; `checkout` v7 additionally refuses to check out fork code under `pull_request_target` and `workflow_run`, neither of which this workflow uses. That drift - three majors on checkout - went unnoticed because nothing watches the pins, so add `dependabot.yml` for `github-actions`, grouped into one pull request. Docker is left off on purpose: the App `Dockerfile` pulls from a private GitLab registry Dependabot cannot authenticate against. `timeout-minutes: 30` because GitHub's default is 360. That was harmless while every run was a 21-second cache hit, but the weekly run resolves conda for real and unattended, and a hung solver would burn six hours every Monday. The spec also records what the first PR run proved: the `unittest` layer is cached like any other, so a green check on an unchanged tree runs no tests at all. That does not need fixing - a real change invalidates the layer - but it means the weekly run is the only periodic evidence the suite still passes. Refs couchbits/movestore/movestore-groundcontrol#7 Co-Authored-By: Claude Opus 5 --- .github/dependabot.yml | 28 +++++++++ .github/workflows/test.yml | 10 ++- ...8-04-ci-fresh-resolution-and-pr-trigger.md | 16 +++++ ...-fresh-resolution-and-pr-trigger-design.md | 61 +++++++++++++++++++ 4 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 .github/dependabot.yml 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 8e322e8..85c6d36 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -26,16 +26,20 @@ 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 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 index a7351c1..8110d6f 100644 --- 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 @@ -459,6 +459,22 @@ an die Person, die die Cron-Zeile zuletzt geändert hat.' --- +## 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:** 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 index 6e4b6fd..00160b5 100644 --- 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 @@ -37,6 +37,33 @@ 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. @@ -124,6 +151,28 @@ The trigger is `pull_request`, **not** `pull_request_target`. `pull_request` run 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 @@ -132,6 +181,18 @@ 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