diff --git a/.claude/skills/kagent-dev/references/database-migrations.md b/.claude/skills/kagent-dev/references/database-migrations.md index d63aa498bd..99853c8949 100644 --- a/.claude/skills/kagent-dev/references/database-migrations.md +++ b/.claude/skills/kagent-dev/references/database-migrations.md @@ -205,11 +205,11 @@ These tests catch policy violations at PR time without needing a running databas ## Upgrade and rollback testing -Static analysis covers file *content*; round-trip tests cover *behavior* against a real Postgres. Beyond `runner_test.go` (rollback and concurrency), two release-to-release tests make the rollback promise real. Both are *Target — not yet enforced*. +Static analysis covers file *content*; round-trip tests cover *behavior* against a real Postgres. Beyond `runner_test.go` (rollback and concurrency), release-to-release coverage makes the rollback promise real. -**Previous-minor round-trip.** Seed a database at the previous minor's latest release with representative data, apply migrations up to `HEAD`, and assert the schema matches a clean `HEAD` install and the data survives; then reverse to the previous minor and assert the schema matches a clean previous-minor install and the data survives. This exercises every changed down file rather than only reviewing it. +**Previous-minor round-trip** (*Target — not yet enforced*). Seed a database at the previous minor's latest release with representative data, apply migrations up to `HEAD`, and assert the schema matches a clean `HEAD` install and the data survives; then reverse to the previous minor and assert the schema matches a clean previous-minor install and the data survives. This exercises every changed down file rather than only reviewing it. -**Query-level backward compatibility.** Run the previous minor's database test suite against a `HEAD`-migrated schema, proving old code's queries run against the newer schema — the exact property [ahead-schema tolerance](#rollback-and-ahead-schema-tolerance) relies on. +**Query-level backward compatibility.** A static check — `scripts/check-query-contraction.sh`, run by the `query-contraction-check` CI job — compiles a previous release's sqlc queries against the `HEAD` schema and fails if a migration dropped, renamed, or retyped a column or table an older query still reads. It catches column/table/type-shape contraction with no database, against two prior versions: the latest release reachable from `HEAD` and the previous stable line's latest patch (the `release/vX.Y.x` tip, via `scripts/prev-stable-version.sh`). The fuller property — running the previous minor's whole database *test suite* against a `HEAD`-migrated schema, which also covers semantic breaks a query still compiles against — remains a *Target — not yet enforced*. ## Downstream Extension Model diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b8e3768d9a..f3d66cfe65 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -160,6 +160,25 @@ jobs: echo "::error::Kubectl logs -n kagent deployment/kagent-controller" kubectl logs -n kagent deployment/kagent-controller + query-contraction-check: + name: Query Contraction Check + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + # Full history + tags so the previous release's queries can be read. + fetch-depth: 0 + fetch-tags: true + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version-file: go/go.mod + cache: true + cache-dependency-path: go/go.sum + - name: Previous-release queries vs current schema + run: make -C go check-query-contraction + go-unit-tests: runs-on: ubuntu-latest steps: @@ -169,7 +188,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v6 with: - go-version: "1.26" + go-version-file: go/go.mod cache: true cache-dependency-path: go/go.sum @@ -302,7 +321,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v6 with: - go-version: "1.26" + go-version-file: go/go.mod cache: true cache-dependency-path: go/go.sum - name: golangci-lint @@ -375,7 +394,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v6 with: - go-version: "1.26" + go-version-file: go/go.mod cache: true cache-dependency-path: go/go.sum diff --git a/go/Makefile b/go/Makefile index b2dc7c35b9..d948dfabcd 100644 --- a/go/Makefile +++ b/go/Makefile @@ -33,6 +33,14 @@ generate: controller-gen sqlc-generate ## Generate DeepCopy methods and sqlc que sqlc-generate: sqlc ## Generate type-safe Go code from SQL queries. cd core/internal/database && $(SQLC) generate +# Compile the previous release's sqlc queries against the current schema (the +# migration files) and fail if a migration removed/renamed/retyped a column or +# table an old query still uses. Static (no database/cluster). Needs git tags +# fetched. TARGET_VERSIONS (space-separated) overrides the auto-derived release(s). +.PHONY: check-query-contraction +check-query-contraction: sqlc ## Verify the previous release's queries still type-check against the current schema + SQLC=$(SQLC) ../scripts/check-query-contraction.sh + ##@ Development .PHONY: fmt diff --git a/go/core/internal/database/sqlc.yaml b/go/core/internal/database/sqlc.yaml index e9f1fe3cb8..44c25c9eb4 100644 --- a/go/core/internal/database/sqlc.yaml +++ b/go/core/internal/database/sqlc.yaml @@ -1,3 +1,11 @@ +# `schema` points at the migration files, so `sqlc generate` validates every +# query against the schema those migrations produce. This is a load-bearing +# migration-safety gate: a migration that drops/renames/retypes a column +# a current query uses makes codegen fail, and the manifests-check CI job catches +# the resulting drift. Do not "fix" such a failure by deleting the query — it is +# reporting a real schema/query incompatibility. The previous-release direction +# (old queries vs the new schema = the windowed-contraction invariant) is checked +# by scripts/check-query-contraction.sh. version: "2" sql: - schema: ["../../pkg/migrations/core", "../../pkg/migrations/vector"] diff --git a/scripts/check-query-contraction.sh b/scripts/check-query-contraction.sh new file mode 100755 index 0000000000..599e17e255 --- /dev/null +++ b/scripts/check-query-contraction.sh @@ -0,0 +1,125 @@ +#!/usr/bin/env bash +# check-query-contraction.sh — query-contraction check. +# +# Compiles a PREVIOUS release's sqlc queries against the CURRENT schema (the +# migration files under go/core/pkg/migrations) and fails if a migration on this +# branch removed, renamed, or retyped a column or table that an older query still +# references — a change that would break that release's code against the new +# schema (the windowed-contraction invariant). +# +# It checks two targets, deduplicated: +# A) the latest released tag reachable from HEAD — the in-line previous release; +# catches a contraction introduced during the current line's development. +# B) the previous stable line's latest patch (release/vX.Y.x tip, via +# prev-stable-version.sh) — the supported rollback-window floor. +# Today these usually resolve to the same tag (one compile); they diverge once a +# new minor releases or the stable line gets a backport patch. +# +# Static: no database and no cluster. sqlc derives the schema from the migration +# files (see go/core/internal/database/sqlc.yaml), so "does every old query still +# type-check against the new schema" is answerable offline. It catches +# column/table/type-shape contraction; semantic breaks (a new NOT NULL, a +# tightened constraint, an index/ordering change) are out of scope for a static +# check and belong to a runtime regression suite. +# +# Inputs (env): +# TARGET_VERSIONS space-separated versions without leading 'v' to check +# instead of the auto-derived A/B (for local runs). +# SQLC sqlc binary to use (default: sqlc on PATH). +# REMOTE git remote for target B (default: origin). +set -euo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +repo_root="$(git -C "$here" rev-parse --show-toplevel)" +sqlc_bin="${SQLC:-sqlc}" +queries_path="go/core/internal/database/queries" +core_migrations="$repo_root/go/core/pkg/migrations/core" +vector_migrations="$repo_root/go/core/pkg/migrations/vector" + +# Resolve the target versions. +targets=() +if [ -n "${TARGET_VERSIONS:-}" ]; then + read -ra targets <<<"${TARGET_VERSIONS}" +else + a="$(git -C "$repo_root" describe --tags --abbrev=0 2>/dev/null | sed 's/^v//' || true)" + b="$("$here/prev-stable-version.sh" 2>/dev/null || true)" + [ -n "${a}" ] && targets+=("${a}") + [ -n "${b}" ] && targets+=("${b}") +fi +if [ "${#targets[@]}" -eq 0 ]; then + echo "ERROR: no contraction target versions resolved; ensure tags are fetched and a release branch exists, or set TARGET_VERSIONS." >&2 + exit 1 +fi +# Deduplicate, preserving order (version strings are space- and glob-free). +targets=($(printf '%s\n' "${targets[@]}" | awk 'NF && !seen[$0]++')) + +workroot="$(mktemp -d)" +trap 'rm -rf "$workroot"' EXIT + +# A target resolved (non-empty version) but whose tag is absent locally means +# the checkout didn't fetch tags — a misconfiguration that would otherwise let +# the whole check pass having compiled nothing. Track the two outcomes so the +# post-loop guard can fail on that case while still allowing a legitimately +# empty run (every resolved target predates the sqlc query set). +compiled=0 +missing_tag=0 + +check_target() { + local prev="$1" + local prev_tag="v${prev}" + + if ! git -C "$repo_root" rev-parse -q --verify "refs/tags/${prev_tag}" >/dev/null; then + echo "NOTE: tag ${prev_tag} not present locally; skipping (fetch tags to include it)." + missing_tag=$((missing_tag + 1)) + return 0 + fi + if [ -z "$(git -C "$repo_root" ls-tree "$prev_tag" -- "$queries_path" 2>/dev/null)" ]; then + echo "NOTE: ${prev_tag} has no ${queries_path}; skipping (predates the sqlc query set)." + return 0 + fi + + # Self-contained sqlc project: sqlc resolves schema/queries relative to the + # config file, so stage everything under a per-target dir. Current migrations + # supply the schema; the previous release supplies the queries. + local wd="$workroot/$prev" + mkdir -p "$wd/schema/core" "$wd/schema/vector" "$wd/queries" "$wd/gen" "$wd/prev" + cp "$core_migrations"/*.sql "$wd/schema/core/" + cp "$vector_migrations"/*.sql "$wd/schema/vector/" + git -C "$repo_root" archive "$prev_tag" "$queries_path" | tar -x -C "$wd/prev" + cp "$wd/prev/$queries_path"/*.sql "$wd/queries/" + + # Minimal config: the go_type overrides in the real sqlc.yaml only affect the + # generated Go types, not whether a query type-checks against the schema. + cat >"$wd/sqlc.yaml" <<'EOF' +version: "2" +sql: + - engine: "postgresql" + schema: ["schema/core", "schema/vector"] + queries: "queries" + gen: + go: + package: "dbgen" + out: "gen" +EOF + + echo "=== Contraction check: queries@${prev_tag} vs current schema ===" + ( cd "$wd" && "$sqlc_bin" compile -f sqlc.yaml ) + echo "OK: ${prev_tag} queries still type-check against the current schema." + compiled=$((compiled + 1)) +} + +for t in "${targets[@]}"; do + check_target "$t" +done + +# Guard against a vacuous green: if nothing compiled because resolved targets had +# no local tag, the checkout almost certainly didn't fetch tags. Fail loudly +# rather than report success on an empty run. An all-predate run (no missing +# tags) is legitimately empty and stays green. +if [ "$compiled" -eq 0 ]; then + if [ "$missing_tag" -gt 0 ]; then + echo "ERROR: no targets compiled — ${missing_tag} resolved version(s) had no local tag; fetch tags (fetch-depth: 0, fetch-tags: true) so the contraction check actually runs." >&2 + exit 1 + fi + echo "NOTE: no targets compiled; all resolved versions predate the sqlc query set." +fi diff --git a/scripts/prev-stable-version.sh b/scripts/prev-stable-version.sh new file mode 100755 index 0000000000..07ba1e2c44 --- /dev/null +++ b/scripts/prev-stable-version.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# prev-stable-version.sh — prints the latest released patch of the stable line +# immediately BELOW the line currently being built: the highest +# vMAJOR.MINOR.PATCH tag on the newest release/vMAJOR.MINOR.x branch whose +# MAJOR.MINOR is strictly less than the current line. This is the rollback-window +# floor a contraction must stay compatible with. +# +# The current line comes from the base/target branch: +# - release/vX.Y.x -> current line is X.Y, so this resolves to the newest +# release line below X.Y (release/v0.9.x -> 0.8.x). +# - main (or any non-release branch) -> the unreleased next minor, which sorts +# above every release line, so this resolves to the newest +# release line overall (-> 0.9.x). +# Source order for the current ref: CURRENT_REF override, GITHUB_BASE_REF (PR +# target), GITHUB_REF_NAME (push), then the checked-out branch. +# +# Prints nothing and exits 0 when no stable line exists below the current one +# (e.g. building the oldest release line), so the caller can skip that target. +# Uses `git ls-remote`, so it needs network to the remote but not the branch +# checked out locally. Output has no leading 'v'. Override the remote with REMOTE. +set -euo pipefail + +remote="${REMOTE:-origin}" + +# Current line MAJOR.MINOR, or empty for main / any non-release line. +current_ref="${CURRENT_REF:-${GITHUB_BASE_REF:-${GITHUB_REF_NAME:-$(git rev-parse --abbrev-ref HEAD 2>/dev/null || true)}}}" +current_minor="" +if [[ "${current_ref}" =~ ^release/v([0-9]+\.[0-9]+)\.x$ ]]; then + current_minor="${BASH_REMATCH[1]}" +fi + +# All release lines on the remote, ascending by version (MAJOR.MINOR only). +lines="$(git ls-remote --heads "$remote" 'refs/heads/release/v*' 2>/dev/null \ + | sed -nE 's#.*refs/heads/release/v([0-9]+\.[0-9]+)\.x$#\1#p' \ + | sort -V)" +if [ -z "${lines}" ]; then + echo "ERROR: no release/vMAJOR.MINOR.x branch found on ${remote}" >&2 + exit 1 +fi + +# ver_lt A B -> success when A < B by version sort. +ver_lt() { [ "$1" != "$2" ] && [ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | head -1)" = "$1" ]; } + +# Highest line strictly below the current line. With no current_minor (main / +# next), every line qualifies, so this lands on the newest line overall. +prev_minor="" +for l in ${lines}; do + if [ -z "${current_minor}" ] || ver_lt "$l" "${current_minor}"; then + prev_minor="$l" + fi +done +if [ -z "${prev_minor}" ]; then + # No stable line below the current one; let the caller skip that target. + exit 0 +fi + +esc="${prev_minor//./\\.}" +latest="$(git ls-remote --tags "$remote" 2>/dev/null \ + | grep -oE "refs/tags/v${esc}\.[0-9]+$" \ + | sed 's#refs/tags/v##' | sort -V | tail -1)" +if [ -z "${latest}" ]; then + echo "ERROR: no v${prev_minor}.PATCH release tags found on ${remote} (fetch tags?)" >&2 + exit 1 +fi + +echo "${latest}"