diff --git a/.claude/skills/kagent-dev/SKILL.md b/.claude/skills/kagent-dev/SKILL.md index 5eadc4bf8..667891d3b 100644 --- a/.claude/skills/kagent-dev/SKILL.md +++ b/.claude/skills/kagent-dev/SKILL.md @@ -79,10 +79,31 @@ After SQL changes, run `sqlc generate` in `go/core/internal/database` and commit ## Database changes -- Add paired migrations and sqlc queries. -- Preserve transaction boundaries for idempotency, ownership, lifecycle fencing, and task ordering. -- Use PostgreSQL constraints for invariants that can be enforced atomically. -- Keep migrations schema-agnostic and safe for multiple controller replicas. +- PostgreSQL migrations use Goose with embedded SQL files. +- Add each migration as `NNNNNN_description.sql`. +- Do not add legacy split files ending in `.up.sql` or `.down.sql`. +- Include one `-- +goose Up` section and one `-- +goose Down` section. +- Goose makes the Down section optional, but Kagent requires it for the database CLI. +- Do not use `-- +goose NO TRANSACTION`. +- Goose must commit each schema change and its migration record in one transaction. +- Never change, rename, or delete a migration after it merges. +- Fix an accepted migration with a new migration. +- Keep migration SQL schema-agnostic. +- Change only objects that the migration source owns. +- Do not use existence guards for source-owned objects; a migration ledger mismatch must fail. +- Use existence guards only for shared bootstrap resources such as PostgreSQL extensions. +- Each migration source must use its own migration table and advisory lock. +- Register dependent sources after the sources that they need. +- Do not add automatic down migrations when a later source fails. +- A restart must continue from each source's last committed version. +- Allow non-destructive startup when the database is ahead of the binary for rolling compatibility. +- The Goose cutover requires a fresh PostgreSQL database. +- Do not add a golang-migrate bridge for the cutover. +- Keep `schema_migrations` for the core source. +- Keep `vector_schema_migrations` for the vector source. +- Run `make -C go sqlc-generate` after a migration change. +- Test the Up and Down sections against PostgreSQL. +- Use PostgreSQL constraints for invariants that the database can enforce atomically. ## Testing and CI diff --git a/.github/actions/upgrade-test-setup/action.yaml b/.github/actions/upgrade-test-setup/action.yaml index fb74525a6..4eb4c7f93 100644 --- a/.github/actions/upgrade-test-setup/action.yaml +++ b/.github/actions/upgrade-test-setup/action.yaml @@ -2,8 +2,8 @@ name: Upgrade Test Setup description: >- Shared prelude for the upgrade-tests and rolling-upgrade-tests jobs: resolve - the upgrade-from version (with the prev-stable == adjacent skip) and bring up - the build/cluster toolchain. Exposes the resolved version and skip flag so the + the upgrade-from version, skip redundant and pre-Goose legs, and bring up the + build/cluster toolchain. Exposes the resolved version and skip flag so the caller can gate its test step. The caller MUST run actions/checkout (with fetch-depth: 0 + fetch-tags) before this action — a local action is loaded from the checked-out workspace and the version resolvers need full history. @@ -15,7 +15,7 @@ inputs: outputs: skip: - description: '"true" when this leg is redundant (prev-stable == adjacent) and the caller should skip its test step.' + description: '"true" when this leg is redundant or predates Goose and the caller should skip its test step.' value: ${{ steps.resolve.outputs.skip }} version: description: The resolved upgrade-from version (empty when skip is true). @@ -66,6 +66,11 @@ runs: fi V="$(./scripts/upgrade-from-version.sh)" fi + if [[ "$V" == 0.10.* ]]; then + echo "previous release v$V predates Goose; skipping upgrade tests." + echo "skip=true" >> "$GITHUB_OUTPUT" + exit 0 + fi echo "version=$V" >> "$GITHUB_OUTPUT" echo "=== Upgrade test (${{ inputs.upgrade-from }} leg): will upgrade FROM v$V TO the current build — building images next ===" echo "::notice title=Upgrade from::v$V (${{ inputs.upgrade-from }} leg) -> current build" diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 12cc800ae..c2c69a0e7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -207,9 +207,11 @@ jobs: strategy: fail-fast: false matrix: + # prev-stable: the previous minor line's latest published version, + # including a prerelease before GA — runs on every base. # adjacent: the same-line latest published version, including prereleases # — only meaningful on a release branch and skipped on main. - upgrade-from: [adjacent] + upgrade-from: [adjacent, prev-stable] steps: - name: Checkout repository uses: actions/checkout@v6 @@ -253,9 +255,11 @@ jobs: strategy: fail-fast: false matrix: + # prev-stable: the previous minor line's latest published version, + # including a prerelease before GA — runs on every base. # adjacent: the same-line latest published version, including prereleases # — only meaningful on a release branch and skipped on main. - upgrade-from: [adjacent] + upgrade-from: [adjacent, prev-stable] steps: - name: Checkout repository uses: actions/checkout@v6 diff --git a/.github/workflows/migration-immutability.yaml b/.github/workflows/migration-immutability.yaml index dfb5d23cb..f18526baf 100644 --- a/.github/workflows/migration-immutability.yaml +++ b/.github/workflows/migration-immutability.yaml @@ -16,20 +16,40 @@ jobs: - name: Fail if any existing migration file was modified run: | - # List files under go/core/pkg/migrations/ that were changed relative - # to the merge base of this PR. We only care about modifications (M) - # and renames (R); additions (A) are fine. BASE=$(git merge-base HEAD origin/${{ github.base_ref }}) - MODIFIED=$(git diff --name-only --diff-filter=MR "$BASE" HEAD \ - -- 'go/core/pkg/migrations/**/*.sql') + CHANGED=$(git diff --no-renames --name-status --diff-filter=MDR "$BASE" HEAD \ + -- 'go/core/pkg/migrations/**/*.sql' \ + | awk '$1 != "D" || $2 !~ /\.(up|down)\.sql$/ { print $2 }') - if [ -n "$MODIFIED" ]; then - echo "ERROR: The following migration files were modified." + if [ -n "$CHANGED" ]; then + echo "ERROR: These migration files changed or were removed." echo "Migration files are immutable once merged." echo "Fix bugs with a new migration instead." echo "" - echo "$MODIFIED" + echo "$CHANGED" exit 1 fi echo "OK: no existing migration files were modified." + + - name: Reject non-transactional migrations + run: | + SPLIT=$(find go/core/pkg/migrations -type f \ + \( -name '*.up.sql' -o -name '*.down.sql' \)) + + if [ -n "$SPLIT" ]; then + echo "ERROR: Legacy split migration files are not supported." + echo "$SPLIT" + exit 1 + fi + + FORBIDDEN=$(find go/core/pkg/migrations -type f -name '*.sql' \ + -exec grep -Eil -- '^--[[:space:]]*\+goose[[:space:]]*NO[[:space:]]+TRANSACTION[[:space:]]*$' {} + || true) + + if [ -n "$FORBIDDEN" ]; then + echo "ERROR: These migrations disable transactions." + echo "$FORBIDDEN" + exit 1 + fi + + echo "OK: all active migrations use transactions." diff --git a/Makefile b/Makefile index 3e7a51077..17812d37b 100644 --- a/Makefile +++ b/Makefile @@ -439,17 +439,10 @@ helm-uninstall: ## Uninstall kagent and kagent-crds Helm releases from the kind helm uninstall kagent --namespace kagent --kube-context kind-$(KIND_CLUSTER_NAME) --wait helm uninstall kagent-crds --namespace kagent --kube-context kind-$(KIND_CLUSTER_NAME) --wait -# Upgrade test targets install the previous released kagent chart from the public -# OCI registry, build the current images, then run the assertions in -# go/core/test/upgrade. These tests are deliberately kept out of test/e2e: they -# mutate the cluster (upgrade then reverse-migrate it) and so cannot share the -# e2e suite's cluster. The Go test performs the actual upgrade to the current -# build by invoking `make helm-install-provider`. UPGRADE_FROM_VERSION defaults to -# the latest version reachable from HEAD (scripts/upgrade-from-version.sh); CI runs -# this against two targets via a matrix — that adjacent version and the previous -# release line's latest published version (scripts/prev-stable-version.sh) — and -# you can pin either locally, e.g. -# `UPGRADE_FROM_VERSION=$$(./scripts/prev-stable-version.sh)`. +# Upgrade tests install a previous Kagent chart and upgrade it to the current build. +# The tests use a separate cluster because they change the database and deployment. +# The tests skip releases that do not use Goose. +# UPGRADE_FROM_VERSION selects the previous release. # The previous install pins the bundled Postgres image to whatever the # upgrade-from release's own install target shipped (resolved inside # install-previous-release), so the baseline matches how that release actually @@ -504,37 +497,29 @@ install-previous-release: ## Install the previous released kagent + kagent-crds --set providers.openAI.apiKey="$${OPENAI_API_KEY:-test}" \ $$db_flags $(UPGRADE_PREV_EXTRA_ARGS) -# run-upgrade-tests installs the previous release, builds the current images, and -# runs the DB-layer upgrade scenario in TestUpgrade: seed -> upgrade -> controller -# rollout (no crash) -> data survival -> schema-equivalence (upgraded == clean -# install) -> reverse schema to target (down files) + data survival. At each -# state it also runs a version-matched invoke e2e slice (TestE2EInvokeInlineAgent) -# so the serving controller's real query paths are exercised, not just psql: the -# HEAD tree post-upgrade, and the previous release's own tree (a git worktree at -# its tag, in .upgrade-prev) for the old-code-against-new-schema and post-rollback -# states. KAGENT_LOCAL_HOST (kind gateway IP) lets the agent reach the in-process -# mock LLM; without it the invoke slices self-skip and only the DB round-trip runs. +# run-upgrade-tests installs the previous release and upgrades it to the current build. +# The test skips releases that do not use Goose. +# Later Goose releases test previous-release behavior after the target migrations, +# data survival, schema equality, previous/current controller startup, and a +# complete application and schema rollback to the previous release. +# KAGENT_LOCAL_HOST lets the agent reach the local mock LLM. # Prerequisite (provided by CI as a separate step; run it locally first): a kind # cluster (make create-kind-cluster). The controller tolerates the missing # agent-sandbox CRD (the owned-resource watch is skipped), and these tests create # no SandboxAgents, so agent-sandbox is not required. .PHONY: announce-upgrade-from announce-upgrade-from: ## Print the upgrade-from -> to versions (runs before the build so it is clear up front) - @echo "=== Upgrade test: FROM $(UPGRADE_FROM_VERSION) TO $(VERSION) — building current images next ===" + @echo "=== Upgrade test: FROM $(UPGRADE_FROM_VERSION) TO $(VERSION). Building current images next. ===" .PHONY: run-upgrade-tests -run-upgrade-tests: announce-upgrade-from build install-previous-release ## Install the previous release, build current images, and run the upgrade + version-matched invoke tests +run-upgrade-tests: announce-upgrade-from build install-previous-release ## Test an upgrade between Goose releases @echo "=== Upgrade test: $(UPGRADE_FROM_VERSION) -> $(VERSION) (registry=$(DOCKER_REGISTRY)) ===" @set -e; \ - git worktree remove --force "$(CURDIR)/.upgrade-prev" 2>/dev/null || true; \ - git worktree add --detach "$(CURDIR)/.upgrade-prev" "v$(UPGRADE_FROM_VERSION)"; \ - trap 'git worktree remove --force "$(CURDIR)/.upgrade-prev" 2>/dev/null || true' EXIT; \ kind_gw="$$($(CONTAINER_RUNTIME) network inspect kind -f '{{range .IPAM.Config}}{{if .Gateway}}{{.Gateway}}{{"\n"}}{{end}}{{end}}' | grep -E '^[0-9]+\.' | head -1)"; \ echo "kind gateway (KAGENT_LOCAL_HOST): $$kind_gw"; \ cd go && \ RUN_UPGRADE_TESTS=true \ REPO_ROOT=$(CURDIR) \ - PREV_E2E_DIR=$(CURDIR)/.upgrade-prev \ KAGENT_LOCAL_HOST="$$kind_gw" \ UPGRADE_FROM_VERSION=$(UPGRADE_FROM_VERSION) \ VERSION=$(VERSION) \ diff --git a/go/core/pkg/app/app.go b/go/core/pkg/app/app.go index 9f5f682f2..d41ba5b99 100644 --- a/go/core/pkg/app/app.go +++ b/go/core/pkg/app/app.go @@ -166,13 +166,18 @@ func Run(ctx context.Context, opts Options) error { if err != nil { return err } + vectorEnabled := envBool("DATABASE_VECTOR_ENABLED") // Appended, not merged: the built-in tracks must reach their final version // before a library consumer's tables, which may reference them. - sources := append(migrations.BuiltinSources(false), opts.ExtraMigrations...) - if err := migrations.RunUp(ctx, dbURL, sources); err != nil { + sources := append(migrations.BuiltinSources(vectorEnabled), opts.ExtraMigrations...) + if envBool("SKIP_MIGRATIONS") { + if err := migrations.VerifyMigrated(ctx, dbURL, sources); err != nil { + return fmt.Errorf("verify database migrations: %w", err) + } + } else if err := migrations.RunUp(ctx, dbURL, sources); err != nil { return fmt.Errorf("run database migrations: %w", err) } - db, err := database.Connect(ctx, &database.PostgresConfig{URL: dbURL}) + db, err := database.Connect(ctx, &database.PostgresConfig{URL: dbURL, VectorEnabled: vectorEnabled}) if err != nil { return err } diff --git a/go/core/pkg/cli/db/migrate/migrate.go b/go/core/pkg/cli/db/migrate/migrate.go index b556a41a9..7a9171878 100644 --- a/go/core/pkg/cli/db/migrate/migrate.go +++ b/go/core/pkg/cli/db/migrate/migrate.go @@ -1,16 +1,4 @@ -// Package migrate exposes the `kagent db migrate` subcommand: out-of-band -// application, rollback, and inspection of the database migration tracks. -// -// The command operates on the same migrations.Source values app.Start -// registers and opens migrators through migrations.WithMigrator, so every -// operation shares the orchestrator's schema handling, tracking tables, and -// advisory-lock identity — a CLI invocation racing a booting server -// serializes instead of corrupting state. -// -// Unlike the in-app startup path, the CLI never auto-recovers a dirty -// tracking table: up, down, and goto refuse a dirty source and report the -// `force` invocation that clears it. Deliberate recovery is the operator's -// call. +// Package migrate provides the database migration commands. package migrate import ( @@ -26,32 +14,23 @@ import ( "strconv" "strings" - "github.com/golang-migrate/migrate/v4" - "github.com/golang-migrate/migrate/v4/source" + "github.com/pressly/goose/v3" "github.com/spf13/cobra" "github.com/kagent-dev/kagent/go/core/pkg/migrations" ) const ( - // dbURLEnv is the controller's env var for --postgres-database-url - // (see app.LoadFromEnv); the CLI falls back to it so an operator with - // the controller's environment needs no extra flags. dbURLEnv = "POSTGRES_DATABASE_URL" sourceFlag = "source" ) -// sourceNameRE constrains Source.Name to lowercase identifiers with -// underscores or hyphens (e.g. "core", "vector", and hyphenated names from -// downstream-registered sources). Name flows into `--source ` handling -// and operator-facing messages; the regex keeps those strings predictable. -var sourceNameRE = regexp.MustCompile(`^[a-z][a-z0-9_-]*$`) +var ( + sourceNameRE = regexp.MustCompile(`^[a-z][a-z0-9_-]*$`) + migrationFileRE = regexp.MustCompile(`^([0-9]+)_.+\.sql$`) +) -// SourcesFunc resolves the migration sources for a command invocation. It is -// called when a subcommand actually executes — never at command construction — -// so implementations may do real work (consult the environment, query a live -// cluster) without taxing unrelated CLI commands. It is called at most once -// per command execution; the result is memoized. +// SourcesFunc gets the migration sources for one command. type SourcesFunc func(ctx context.Context) ([]migrations.Source, error) type commandState struct { @@ -59,50 +38,44 @@ type commandState struct { source string resolveFn SourcesFunc - // memoized getSources result resolved bool sources []migrations.Source sourcesErr error } -// getSources resolves and validates the source list on first use, memoizing -// the result for the rest of the command execution. func (s *commandState) getSources(ctx context.Context) ([]migrations.Source, error) { if s.resolved { return s.sources, s.sourcesErr } s.resolved = true - srcs, err := s.resolveFn(ctx) + sources, err := s.resolveFn(ctx) if err != nil { s.sourcesErr = fmt.Errorf("resolve migration sources: %w", err) return nil, s.sourcesErr } - if err := validateSourceNames(srcs); err != nil { + if err := validateSourceNames(sources); err != nil { s.sourcesErr = err - return nil, s.sourcesErr + return nil, err } - s.sources = append([]migrations.Source(nil), srcs...) + s.sources = append([]migrations.Source(nil), sources...) return s.sources, nil } func validateSourceNames(sources []migrations.Source) error { - seen := map[string]bool{} + seen := make(map[string]bool, len(sources)) for _, source := range sources { if !sourceNameRE.MatchString(source.Name) { - return fmt.Errorf("migration source Name=%q must match %s", source.Name, sourceNameRE.String()) + return fmt.Errorf("migration source name %q must match %s", source.Name, sourceNameRE.String()) } if seen[source.Name] { - return fmt.Errorf("migration source %q configured twice; each source must have a unique Name", source.Name) + return fmt.Errorf("migration source %q appears twice", source.Name) } seen[source.Name] = true } return nil } -// NewCommand returns the `migrate` parent command with all subcommands -// attached, operating on the given sources in orchestrator registration -// order. Panics on an invalid or duplicate source Name so misconfiguration -// fails at wiring time, not mid-operation. +// NewCommand returns the migrate command. func NewCommand(sources ...migrations.Source) *cobra.Command { if err := validateSourceNames(sources); err != nil { panic("migrate.NewCommand: " + err.Error()) @@ -113,33 +86,23 @@ func NewCommand(sources ...migrations.Source) *cobra.Command { }) } -// NewCommandFromFunc is NewCommand with deferred source resolution: fn runs -// when a subcommand executes, and an invalid source set surfaces as a command -// error instead of a wiring-time panic. Use this when the source list depends -// on state that shouldn't be consulted while merely constructing the command -// tree (environment variables, a live cluster). +// NewCommandFromFunc returns a command with deferred source resolution. func NewCommandFromFunc(fn SourcesFunc) *cobra.Command { state := &commandState{resolveFn: fn} - - cmd := &cobra.Command{ + command := &cobra.Command{ Use: "migrate", Short: "Apply, roll back, and inspect database migrations", - Long: `Apply, roll back, and inspect database migrations independently -of server startup. Reads ` + dbURLEnv + ` from the environment when ---db-url is omitted.`, - } - cmd.PersistentFlags().StringVar(&state.dbURL, "db-url", "", - "PostgreSQL connection URL (defaults to value of "+dbURLEnv+" env var)") - cmd.PersistentFlags().StringVar(&state.source, sourceFlag, "", - "Migration source name for per-source ops (down/goto/force/version); inferred when only one source is registered. Not applicable to up or status — those aggregate across every registered source.") - - cmd.AddCommand(newUpCmd(state)) - cmd.AddCommand(newDownCmd(state)) - cmd.AddCommand(newStatusCmd(state)) - cmd.AddCommand(newVersionCmd(state)) - cmd.AddCommand(newGotoCmd(state)) - cmd.AddCommand(newForceCmd(state)) - return cmd + Long: `Apply, roll back, and inspect database migrations. +The command reads POSTGRES_DATABASE_URL when --db-url is empty.`, + } + command.PersistentFlags().StringVar(&state.dbURL, "db-url", "", "PostgreSQL connection URL") + command.PersistentFlags().StringVar(&state.source, sourceFlag, "", "Migration source for down, goto, or version") + command.AddCommand(newUpCmd(state)) + command.AddCommand(newDownCmd(state)) + command.AddCommand(newStatusCmd(state)) + command.AddCommand(newVersionCmd(state)) + command.AddCommand(newGotoCmd(state)) + return command } func (s *commandState) resolveDSN() (string, error) { @@ -148,394 +111,278 @@ func (s *commandState) resolveDSN() (string, error) { dsn = os.Getenv(dbURLEnv) } if dsn == "" { - return "", fmt.Errorf("database URL not set; pass --db-url or set %s", dbURLEnv) + return "", fmt.Errorf("set the database URL with --db-url or %s", dbURLEnv) } return dsn, nil } -// resolveSource picks the source for a per-source operation. With one source -// registered it's returned directly; with more than one the operator must -// pass --source and we report the registered set when they don't. func (s *commandState) resolveSource(ctx context.Context) (migrations.Source, error) { - srcs, err := s.getSources(ctx) + sources, err := s.getSources(ctx) if err != nil { return migrations.Source{}, err } - if len(srcs) == 0 { - return migrations.Source{}, errors.New("no migration sources registered") + if len(sources) == 0 { + return migrations.Source{}, errors.New("no migration sources are registered") } - if len(srcs) == 1 { - if s.source != "" && s.source != srcs[0].Name { - return migrations.Source{}, fmt.Errorf("--source %q not registered; registered source: %s", s.source, srcs[0].Name) + if len(sources) == 1 { + if s.source != "" && s.source != sources[0].Name { + return migrations.Source{}, fmt.Errorf("source %q is not registered", s.source) } - return srcs[0], nil + return sources[0], nil } if s.source == "" { - return migrations.Source{}, fmt.Errorf("registered sources: %s; pass --source", sourceNames(srcs)) + return migrations.Source{}, fmt.Errorf("registered sources: %s. Pass --source", sourceNames(sources)) } - for _, src := range srcs { - if src.Name == s.source { - return src, nil + for _, source := range sources { + if source.Name == s.source { + return source, nil } } - return migrations.Source{}, fmt.Errorf("--source %q not registered; registered sources: %s", s.source, sourceNames(srcs)) + return migrations.Source{}, fmt.Errorf("source %q is not registered", s.source) } -func sourceNames(srcs []migrations.Source) string { - names := make([]string, len(srcs)) - for i, s := range srcs { - names[i] = s.Name +func sourceNames(sources []migrations.Source) string { + names := make([]string, len(sources)) + for i, source := range sources { + names[i] = source.Name } return strings.Join(names, ", ") } -// readVersion returns mg's highest applied version and whether the tracking -// row is dirty (mid-failed-migration). ErrNilVersion (nothing applied) is -// normalized to (0, false, nil). -func readVersion(mg *migrate.Migrate) (uint, bool, error) { - v, dirty, err := mg.Version() - if err != nil { - if errors.Is(err, migrate.ErrNilVersion) { - return 0, false, nil - } - return 0, false, fmt.Errorf("read version: %w", err) - } - return v, dirty, nil -} - -// ensureClean returns an actionable error when src's tracking table is dirty. -// The CLI refuses to operate on a dirty source rather than auto-recovering -// (the in-app startup path is the automatic tier; the CLI is the manual one). -func ensureClean(src migrations.Source, mg *migrate.Migrate) error { - v, dirty, err := readVersion(mg) - if err != nil { - return err - } - if dirty { - return fmt.Errorf("source %q is dirty at version %d (a previous migration attempt failed and must be resolved manually); inspect the schema, then clear the flag with \"kagent db migrate force --source %s\" where is the version the schema actually reflects", - src.Name, v, src.Name) - } - return nil -} - -// sourceFileVersions returns the (ascending-sorted) versions parsed from -// every up migration file in src.FS/src.Dir, using golang-migrate's own -// filename parser so a file it would silently skip is never counted here. -// -// The set isn't required to be contiguous — gaps (e.g. a deleted 005) are -// real and the missing numbers are treated as not-applicable-to-this-binary. -// status/desync math goes via this list so the count of files and the highest -// version stay distinct. -func sourceFileVersions(src migrations.Source) ([]int, error) { - entries, err := fs.ReadDir(src.FS, src.Dir) +func sourceFileVersions(source migrations.Source) ([]int64, error) { + entries, err := fs.ReadDir(source.FS, source.Dir) if err != nil { - return nil, fmt.Errorf("read migration dir %s: %w", src.Dir, err) + return nil, fmt.Errorf("read migration directory %s: %w", source.Dir, err) } - var versions []int - for _, e := range entries { - if e.IsDir() { + versions := make([]int64, 0, len(entries)) + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".sql") { continue } - m, err := source.DefaultParse(e.Name()) - if err != nil || m.Direction != source.Up { - continue + if strings.HasSuffix(entry.Name(), ".up.sql") || strings.HasSuffix(entry.Name(), ".down.sql") { + return nil, fmt.Errorf("invalid migration file %s", entry.Name()) + } + match := migrationFileRE.FindStringSubmatch(entry.Name()) + if match == nil { + return nil, fmt.Errorf("invalid migration file %s", entry.Name()) + } + version, err := strconv.ParseInt(match[1], 10, 64) + if err != nil || version < 1 { + return nil, fmt.Errorf("invalid migration version in %s", entry.Name()) } - versions = append(versions, int(m.Version)) + versions = append(versions, version) } slices.Sort(versions) return versions, nil } -// lineRow carries per-source status data through the status command's text -// and JSON output paths. -type lineRow struct { - src migrations.Source - applied int - pending int - dbVersion int // raw DB version for desync reporting - downgraded bool // dbVersion > highest shipped version - dirty bool // mid-failed-migration; surfaced as a (dirty) annotation +func readVersion(ctx context.Context, provider *goose.Provider) (int64, error) { + version, err := provider.GetDBVersion(ctx) + if err != nil { + return 0, fmt.Errorf("read migration version: %w", err) + } + return version, nil } func newUpCmd(state *commandState) *cobra.Command { return &cobra.Command{ Use: "up", - Short: "Apply all pending migrations across every registered source", - Long: `Applies pending migrations for every registered source in -registration order, through the same orchestrator the server runs at -startup: per-source advisory locking, pre-run version snapshots, and -compensating rollback of earlier sources when a later one fails. - -Refuses to run while any source's tracking table is dirty; clear it -with 'force' first. - -The --source flag is intentionally not applicable to up; pass it only -on the per-source subcommands (down/goto/force).`, - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { + Short: "Apply all pending migrations", + Args: cobra.NoArgs, + RunE: func(command *cobra.Command, _ []string) error { if state.source != "" { - return errors.New("up aggregates across all registered sources; --source is not applicable") + return errors.New("up applies all sources. --source is not applicable") } dsn, err := state.resolveDSN() if err != nil { return err } - ctx := cmd.Context() - srcs, err := state.getSources(ctx) + sources, err := state.getSources(command.Context()) if err != nil { return err } - if len(srcs) == 0 { - return errors.New("no migration sources registered") + if len(sources) == 0 { + return errors.New("no migration sources are registered") } - - // One pre-pass over the sources: refuse dirty state, and snapshot - // pending counts so we can report "applied N migration(s)" after - // the orchestrator succeeds. - prePending := 0 - for _, src := range srcs { - p, err := pendingCount(ctx, src, dsn) - if err != nil { - return err - } - prePending += p - } - - if err := migrations.RunUp(ctx, dsn, srcs); err != nil { + if err := migrations.RunUp(command.Context(), dsn, sources); err != nil { return err } - - if prePending == 0 { - fmt.Fprintln(cmd.OutOrStdout(), "no pending migrations; schema is up to date") - return nil - } - fmt.Fprintf(cmd.OutOrStdout(), "applied %d migration(s); schema is up to date\n", prePending) + fmt.Fprintln(command.OutOrStdout(), "schema is up to date") return nil }, } } -// pendingCount counts NNN_*.up.sql files whose version is greater than the -// source's current applied version, refusing a dirty source. Uses the same -// `sourceFileVersions` primitive as `status` so the two paths can't drift. -func pendingCount(ctx context.Context, src migrations.Source, dsn string) (int, error) { - versions, err := sourceFileVersions(src) - if err != nil { - return 0, err - } - var pending int - err = migrations.WithMigrator(ctx, dsn, src, func(mg *migrate.Migrate) error { - if err := ensureClean(src, mg); err != nil { - return err - } - v, _, verr := readVersion(mg) - if verr != nil { - return verr - } - for _, fv := range versions { - if uint(fv) > v { - pending++ - } - } - return nil - }) - if err != nil { - return 0, err - } - return pending, nil -} - func newDownCmd(state *commandState) *cobra.Command { return &cobra.Command{ Use: "down N", - Short: "Roll back the N most-recent applied migrations for the selected source", - Long: `Roll back the N most-recent applied migrations for the selected source. - -Down migrations can lose data by design — a rolled-back column loses -its contents. Refuses to run while the source's tracking table is -dirty; clear it with 'force' first.`, - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - n, err := strconv.Atoi(args[0]) - if err != nil || n < 1 { - return fmt.Errorf("expected a positive integer for N, got %q", args[0]) + Short: "Roll back the latest N migrations", + Long: "Roll back the latest N migrations. A down migration can delete data.", + Args: cobra.ExactArgs(1), + RunE: func(command *cobra.Command, args []string) error { + count, err := strconv.Atoi(args[0]) + if err != nil || count < 1 { + return fmt.Errorf("n must be a positive integer, got %q", args[0]) } dsn, err := state.resolveDSN() if err != nil { return err } - src, err := state.resolveSource(cmd.Context()) + source, err := state.resolveSource(command.Context()) if err != nil { return err } - return migrations.WithMigrator(cmd.Context(), dsn, src, func(mg *migrate.Migrate) error { - if err := ensureClean(src, mg); err != nil { + versions, err := sourceFileVersions(source) + if err != nil { + return err + } + return migrations.WithProvider(command.Context(), dsn, source, func(provider *goose.Provider) error { + current, err := readVersion(command.Context(), provider) + if err != nil { return err } - preV, _, err := readVersion(mg) + if current > versions[len(versions)-1] { + return fmt.Errorf("database version %d exceeds embedded version %d", current, versions[len(versions)-1]) + } + status, err := provider.Status(command.Context()) if err != nil { return err } - // Guarded here rather than relying on golang-migrate: Steps - // on an empty track reports a confusing "file does not - // exist" instead of ErrNoChange. - if preV == 0 { - fmt.Fprintln(cmd.OutOrStdout(), "no migrations to roll back") + applied := make([]int64, 0, len(status)) + for _, migration := range status { + if migration.State == goose.StateApplied { + applied = append(applied, migration.Source.Version) + } + } + if len(applied) == 0 { + fmt.Fprintln(command.OutOrStdout(), "no migrations to roll back") return nil } - if err := mg.Steps(-n); err != nil { - if errors.Is(err, migrate.ErrNoChange) { - fmt.Fprintln(cmd.OutOrStdout(), "no migrations to roll back") - return nil - } - return err + targetIndex := len(applied) - count - 1 + target := int64(0) + if targetIndex >= 0 { + target = applied[targetIndex] } - postV, _, verr := readVersion(mg) - if verr != nil { - return fmt.Errorf("read version after rollback: %w", verr) + results, err := provider.DownTo(command.Context(), target) + if err != nil { + return err } - rolled := countVersionsBetween(src, postV, preV) - fmt.Fprintf(cmd.OutOrStdout(), "rolled back %d migration(s)\n", rolled) + fmt.Fprintf(command.OutOrStdout(), "rolled back %d migration(s)\n", len(results)) return nil }) }, } } +type lineRow struct { + source migrations.Source + applied int + pending int + dbVersion int64 + ahead bool +} + func newStatusCmd(state *commandState) *cobra.Command { var output string - cmd := &cobra.Command{ + command := &cobra.Command{ Use: "status", - Short: "Show how many migrations are applied vs pending across all sources", + Short: "Show migration status", Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { + RunE: func(command *cobra.Command, _ []string) error { if state.source != "" { - return errors.New("status aggregates across all registered sources; --source is not applicable") + return errors.New("status shows all sources. --source is not applicable") } if output != "text" && output != "json" { - return fmt.Errorf("invalid --output %q; supported: text, json", output) + return fmt.Errorf("invalid --output %q. Use text or json", output) } dsn, err := state.resolveDSN() if err != nil { return err } - srcs, err := state.getSources(cmd.Context()) + sources, err := state.getSources(command.Context()) if err != nil { return err } - if len(srcs) == 0 { - return errors.New("no migration sources registered") + if len(sources) == 0 { + return errors.New("no migration sources are registered") } - lines := make([]lineRow, 0, len(srcs)) - appliedTotal, pendingTotal := 0, 0 - // Single-source builds print no per-source breakdown, so the - // stderr desync warning below is gated to them as their only - // signal; multi-source builds carry it in the stdout breakdown. - multiSource := len(srcs) > 1 - for _, src := range srcs { - versions, err := sourceFileVersions(src) + lines := make([]lineRow, 0, len(sources)) + appliedTotal := 0 + pendingTotal := 0 + for _, source := range sources { + line := lineRow{source: source} + versions, err := sourceFileVersions(source) if err != nil { return err } - maxFileVersion := 0 - if len(versions) > 0 { - maxFileVersion = versions[len(versions)-1] - } - var applied, dbVersion int - var downgraded, dirty bool - if rerr := migrations.WithMigrator(cmd.Context(), dsn, src, func(mg *migrate.Migrate) error { - v, d, err := readVersion(mg) + err = migrations.WithProvider(command.Context(), dsn, source, func(provider *goose.Provider) error { + status, err := provider.Status(command.Context()) if err != nil { return err } - dbVersion = int(v) - dirty = d - // Count files at/below the DB version as applied - // (counting by version, not file count, survives gaps - // like a deleted v5). - for _, fv := range versions { - if fv <= dbVersion { - applied++ - } - } - if dbVersion > maxFileVersion { - // Older binary against a DB migrated by a newer - // build. Warn, don't fail. - downgraded = true - if !multiSource { - fmt.Fprintf(cmd.ErrOrStderr(), - "warning: %s reports version %d but this binary's highest shipped migration is v%d (older binary against newer DB?)\n", - src.Name, v, maxFileVersion) + for _, migration := range status { + if migration.State == goose.StateApplied { + line.applied++ + } else { + line.pending++ } } - return nil - }); rerr != nil { - return rerr + line.dbVersion, err = readVersion(command.Context(), provider) + return err + }) + if err != nil { + return err } - pending := len(versions) - applied - lines = append(lines, lineRow{src: src, applied: applied, pending: pending, dbVersion: dbVersion, downgraded: downgraded, dirty: dirty}) - appliedTotal += applied - pendingTotal += pending + if len(versions) > 0 { + line.ahead = line.dbVersion > versions[len(versions)-1] + } + lines = append(lines, line) + appliedTotal += line.applied + pendingTotal += line.pending } - out := cmd.OutOrStdout() if output == "json" { - return writeStatusJSON(out, lines, appliedTotal, pendingTotal) - } - if multiSource { - fmt.Fprintf(out, "%d migration(s) applied, %d pending\n", appliedTotal, pendingTotal) - // Reuses the same `multiSource` gate as the stderr desync - // warning above on purpose: a per-source skip branch must - // not let the two diverge (a desync gets a source warned - // twice or not at all). - for _, l := range lines { - if l.downgraded { - fmt.Fprintf(out, " %s: %d applied, %d pending (db reports v%d%s — binary out of date)\n", - l.src.Name, l.applied, l.pending, l.dbVersion, dirtyTag(l.dirty)) - } else { - fmt.Fprintf(out, " %s: %d applied (at v%d%s), %d pending\n", - l.src.Name, l.applied, l.dbVersion, dirtyTag(l.dirty), l.pending) - } - } - } else { - // Single-source: fold the version into the headline so - // operators needn't run `version` separately. dbVersion is - // the raw tracking-table value (matches `force V`). - l := lines[0] - fmt.Fprintf(out, "%d migration(s) applied (at v%d%s), %d pending\n", - l.applied, l.dbVersion, dirtyTag(l.dirty), l.pending) + return writeStatusJSON(command.OutOrStdout(), lines, appliedTotal, pendingTotal) } + writeStatusText(command.OutOrStdout(), lines, appliedTotal, pendingTotal) return nil }, } - // No -o shorthand: the kagent root command already owns -o - // (--output-format) as a persistent flag, and cobra panics on a - // shorthand redefinition. - cmd.Flags().StringVar(&output, "output", "text", - `Output format: "text" (default) or "json"`) - return cmd + command.Flags().StringVar(&output, "output", "text", `Output format: "text" or "json"`) + return command +} + +func writeStatusText(out io.Writer, lines []lineRow, appliedTotal, pendingTotal int) { + if len(lines) == 1 { + line := lines[0] + fmt.Fprintf(out, "%d migration(s) applied (at v%d), %d pending\n", line.applied, line.dbVersion, line.pending) + return + } + fmt.Fprintf(out, "%d migration(s) applied, %d pending\n", appliedTotal, pendingTotal) + for _, line := range lines { + if line.ahead { + fmt.Fprintf(out, " %s: %d applied, %d pending (database reports v%d. The binary is old)\n", + line.source.Name, line.applied, line.pending, line.dbVersion) + continue + } + fmt.Fprintf(out, " %s: %d applied (at v%d), %d pending\n", + line.source.Name, line.applied, line.dbVersion, line.pending) + } } -// statusJSON is the wire format for `kagent db migrate status --output json`. -// Operators consume it via `jq`, so the field names and types are a frozen -// contract; TestStatusJSONShape locks them and fails CI on a rename or -// retype. type statusJSON struct { Applied int `json:"applied"` Pending int `json:"pending"` Sources []statusSourceJSON `json:"sources"` } -// statusSourceJSON is the per-source object inside statusJSON; same -// frozen-shape contract, also locked by TestStatusJSONShape. type statusSourceJSON struct { Name string `json:"name"` Applied int `json:"applied"` Pending int `json:"pending"` - Version int `json:"version"` + Version int64 `json:"version"` Downgraded bool `json:"downgraded"` - Dirty bool `json:"dirty"` } func writeStatusJSON(out io.Writer, lines []lineRow, appliedTotal, pendingTotal int) error { @@ -544,101 +391,60 @@ func writeStatusJSON(out io.Writer, lines []lineRow, appliedTotal, pendingTotal Pending: pendingTotal, Sources: make([]statusSourceJSON, 0, len(lines)), } - for _, l := range lines { + for _, line := range lines { payload.Sources = append(payload.Sources, statusSourceJSON{ - Name: l.src.Name, - Applied: l.applied, - Pending: l.pending, - Version: l.dbVersion, - Downgraded: l.downgraded, - Dirty: l.dirty, + Name: line.source.Name, + Applied: line.applied, + Pending: line.pending, + Version: line.dbVersion, + Downgraded: line.ahead, }) } - enc := json.NewEncoder(out) - enc.SetIndent("", " ") - return enc.Encode(payload) -} - -// dirtyTag returns " (dirty)" when the source is mid-failed-migration, "" -// otherwise. Used to annotate the version in status/version text output -// without adding a separate line. -func dirtyTag(dirty bool) string { - if dirty { - return " (dirty)" - } - return "" -} - -// versionAnnotation renders the trailing annotation for `version` output. -// Disambiguates an unapplied-migrations state (v=0, !dirty) from a versioned -// state by tagging the former; dirty wins over the "no migrations applied" -// tag because it's the more actionable signal. -func versionAnnotation(v uint, dirty bool) string { - if dirty { - return " (dirty)" - } - if v == 0 { - return " (no migrations applied)" - } - return "" + encoder := json.NewEncoder(out) + encoder.SetIndent("", " ") + return encoder.Encode(payload) } func newVersionCmd(state *commandState) *cobra.Command { return &cobra.Command{ Use: "version", - Short: "Print the highest applied migration version", - Long: `Print the highest applied migration version. -For a single registered source the value is on one line; multi-source -binaries print one line per source. When multiple sources are -registered, --source filters to a single track.`, - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { + Short: "Show the applied migration version", + Args: cobra.NoArgs, + RunE: func(command *cobra.Command, _ []string) error { dsn, err := state.resolveDSN() if err != nil { return err } - srcs, err := state.getSources(cmd.Context()) + sources, err := state.getSources(command.Context()) if err != nil { return err } - if len(srcs) == 0 { - return errors.New("no migration sources registered") + if len(sources) == 0 { + return errors.New("no migration sources are registered") } - // --source filters the output even though version is otherwise - // an aggregate op. Empty flag = print all. if state.source != "" { - picked := -1 - for i, s := range srcs { - if s.Name == state.source { - picked = i - break - } - } - if picked < 0 { - return fmt.Errorf("--source %q not registered; registered sources: %s", state.source, sourceNames(srcs)) + index := slices.IndexFunc(sources, func(source migrations.Source) bool { + return source.Name == state.source + }) + if index < 0 { + return fmt.Errorf("source %q is not registered", state.source) } - srcs = []migrations.Source{srcs[picked]} + sources = sources[index : index+1] } - out := cmd.OutOrStdout() - if len(srcs) == 1 { - return migrations.WithMigrator(cmd.Context(), dsn, srcs[0], func(mg *migrate.Migrate) error { - v, dirty, err := readVersion(mg) + for _, source := range sources { + err := migrations.WithProvider(command.Context(), dsn, source, func(provider *goose.Provider) error { + version, err := readVersion(command.Context(), provider) if err != nil { return err } - fmt.Fprintf(out, "%d%s\n", v, versionAnnotation(v, dirty)) - return nil - }) - } - for _, src := range srcs { - if err := migrations.WithMigrator(cmd.Context(), dsn, src, func(mg *migrate.Migrate) error { - v, dirty, err := readVersion(mg) - if err != nil { - return err + if len(sources) == 1 { + fmt.Fprintf(command.OutOrStdout(), "%d%s\n", version, versionAnnotation(version)) + } else { + fmt.Fprintf(command.OutOrStdout(), "%s: %d%s\n", source.Name, version, versionAnnotation(version)) } - fmt.Fprintf(out, "%s: %d%s\n", src.Name, v, versionAnnotation(v, dirty)) return nil - }); err != nil { + }) + if err != nil { return err } } @@ -647,140 +453,69 @@ registered, --source filters to a single track.`, } } +func versionAnnotation(version int64) string { + if version == 0 { + return " (no migrations applied)" + } + return "" +} + func newGotoCmd(state *commandState) *cobra.Command { return &cobra.Command{ Use: "goto V", - Short: "Move the selected source's schema to version V", - Long: `Move the selected source's schema to version V (forward or backward). -V=0 is the special "empty schema" target: every applied migration in -the source is rolled back. - -Refuses to run while the source's tracking table is dirty; clear it -with 'force' first.`, - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - v, err := strconv.Atoi(args[0]) - if err != nil || v < 0 { - return fmt.Errorf("expected a non-negative integer for V, got %q", args[0]) + Short: "Move one source to version V", + Long: "Move one source to version V. Version zero removes its schema.", + Args: cobra.ExactArgs(1), + RunE: func(command *cobra.Command, args []string) error { + target, err := strconv.ParseInt(args[0], 10, 64) + if err != nil || target < 0 { + return fmt.Errorf("v must be a non-negative integer, got %q", args[0]) } dsn, err := state.resolveDSN() if err != nil { return err } - src, err := state.resolveSource(cmd.Context()) + source, err := state.resolveSource(command.Context()) if err != nil { return err } - return migrations.WithMigrator(cmd.Context(), dsn, src, func(mg *migrate.Migrate) error { - if err := ensureClean(src, mg); err != nil { - return err - } - if v == 0 { - if err := mg.Down(); err != nil && !errors.Is(err, migrate.ErrNoChange) { - return err - } - fmt.Fprintln(cmd.OutOrStdout(), "schema is at version 0 (empty)") - return nil - } - if err := mg.Migrate(uint(v)); err != nil && !errors.Is(err, migrate.ErrNoChange) { - return err - } - actual, dirty, aerr := readVersion(mg) - if aerr != nil { - return aerr - } - fmt.Fprintf(cmd.OutOrStdout(), "schema is at version %d%s\n", actual, versionAnnotation(actual, dirty)) - return nil - }) - }, - } -} - -func newForceCmd(state *commandState) *cobra.Command { - return &cobra.Command{ - Use: "force V", - Short: "Mark version V as applied without running its SQL", - Long: `Used to reconcile the selected source's tracking table after manual -remediation, e.g. to clear a dirty flag left by a failed migration. -V=0 clears the version record entirely (the "no migrations applied" -state). Any other V must correspond to a shipped migration file in -the selected source — otherwise the tracking row would point at a -version the binary cannot apply or roll back to, wedging the DB.`, - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - v, err := strconv.Atoi(args[0]) - if err != nil || v < 0 { - return fmt.Errorf("expected a non-negative integer for V, got %q", args[0]) - } - dsn, err := state.resolveDSN() + versions, err := sourceFileVersions(source) if err != nil { return err } - src, err := state.resolveSource(cmd.Context()) - if err != nil { - return err + if target != 0 && !slices.Contains(versions, target) { + return fmt.Errorf("version %d is not available. Valid versions are %s", target, formatVersionList(versions)) } - if v > 0 { - versions, err := sourceFileVersions(src) + return migrations.WithProvider(command.Context(), dsn, source, func(provider *goose.Provider) error { + current, err := readVersion(command.Context(), provider) if err != nil { return err } - if !slices.Contains(versions, v) { - return fmt.Errorf( - "version %d is not a shipped migration for source %q; valid versions are %s", - v, src.Name, formatVersionList(versions)) + if current > versions[len(versions)-1] { + return fmt.Errorf("database version %d exceeds embedded version %d", current, versions[len(versions)-1]) } - } - return migrations.WithMigrator(cmd.Context(), dsn, src, func(mg *migrate.Migrate) error { - if v == 0 { - // golang-migrate's Force takes -1 to delete the version - // record; 0 is not a valid stored version. - if err := mg.Force(-1); err != nil { - return err - } - fmt.Fprintln(cmd.OutOrStdout(), "version record cleared (no migrations applied)") - return nil + if target > current { + _, err = provider.UpTo(command.Context(), target) + } else if target < current { + _, err = provider.DownTo(command.Context(), target) } - if err := mg.Force(v); err != nil { + if err != nil { return err } - fmt.Fprintf(cmd.OutOrStdout(), "version %d marked as applied\n", v) + fmt.Fprintf(command.OutOrStdout(), "schema is at version %d%s\n", target, versionAnnotation(target)) return nil }) }, } } -// countVersionsBetween returns the count of shipped source migrations in the -// half-open interval (low, high]. Used by `down N` to report the actual -// number of migrations rolled back regardless of whether the user-supplied N -// exceeded the applied count. Returns 0 on a source-enumeration error; the -// call site already surfaces success/failure via Steps(-N) and the count is -// operator-facing display only. -func countVersionsBetween(src migrations.Source, low, high uint) int { - versions, err := sourceFileVersions(src) - if err != nil { - return 0 - } - count := 0 - for _, v := range versions { - uv := uint(v) - if uv > low && uv <= high { - count++ - } - } - return count -} - -// formatVersionList renders a small []int as a human-readable list for error -// messages: "1, 2, 5" or "(none)" if empty. -func formatVersionList(versions []int) string { +func formatVersionList(versions []int64) string { if len(versions) == 0 { return "(none)" } parts := make([]string, len(versions)) - for i, v := range versions { - parts[i] = strconv.Itoa(v) + for i, version := range versions { + parts[i] = strconv.FormatInt(version, 10) } return strings.Join(parts, ", ") } diff --git a/go/core/pkg/cli/db/migrate/migrate_test.go b/go/core/pkg/cli/db/migrate/migrate_test.go index 4fb3a531e..fd7a885f0 100644 --- a/go/core/pkg/cli/db/migrate/migrate_test.go +++ b/go/core/pkg/cli/db/migrate/migrate_test.go @@ -3,7 +3,6 @@ package migrate import ( "bytes" "context" - "database/sql" "encoding/json" "errors" "io" @@ -11,8 +10,6 @@ import ( "testing" "testing/fstest" - _ "github.com/jackc/pgx/v5/stdlib" - "github.com/kagent-dev/kagent/go/core/internal/dbtest" "github.com/kagent-dev/kagent/go/core/pkg/migrations" ) @@ -20,15 +17,25 @@ import ( // --- fixtures --- var alphaFS = fstest.MapFS{ - "alpha/000001_create.up.sql": {Data: []byte(`CREATE TABLE IF NOT EXISTS cli_alpha (id SERIAL PRIMARY KEY);`)}, - "alpha/000001_create.down.sql": {Data: []byte(`DROP TABLE IF EXISTS cli_alpha;`)}, - "alpha/000002_alter.up.sql": {Data: []byte(`ALTER TABLE cli_alpha ADD COLUMN IF NOT EXISTS name TEXT;`)}, - "alpha/000002_alter.down.sql": {Data: []byte(`ALTER TABLE cli_alpha DROP COLUMN IF EXISTS name;`)}, + "alpha/000001_create.sql": {Data: migrationFile( + `CREATE TABLE IF NOT EXISTS cli_alpha (id SERIAL PRIMARY KEY);`, + `DROP TABLE IF EXISTS cli_alpha;`, + )}, + "alpha/000002_alter.sql": {Data: migrationFile( + `ALTER TABLE cli_alpha ADD COLUMN IF NOT EXISTS name TEXT;`, + `ALTER TABLE cli_alpha DROP COLUMN IF EXISTS name;`, + )}, } var betaFS = fstest.MapFS{ - "beta/000001_create.up.sql": {Data: []byte(`CREATE TABLE IF NOT EXISTS cli_beta (id SERIAL PRIMARY KEY);`)}, - "beta/000001_create.down.sql": {Data: []byte(`DROP TABLE IF EXISTS cli_beta;`)}, + "beta/000001_create.sql": {Data: migrationFile( + `CREATE TABLE IF NOT EXISTS cli_beta (id SERIAL PRIMARY KEY);`, + `DROP TABLE IF EXISTS cli_beta;`, + )}, +} + +func migrationFile(up, down string) []byte { + return []byte("-- +goose Up\n" + up + "\n-- +goose Down\n" + down + "\n") } func testSources() []migrations.Source { @@ -121,7 +128,7 @@ func TestResolveSource(t *testing.T) { {name: "single source inferred", sources: single, want: "alpha"}, {name: "single source explicit match", sources: single, flag: "alpha", want: "alpha"}, {name: "single source mismatch", sources: single, flag: "beta", wantErr: "not registered"}, - {name: "multi requires flag", sources: multi, wantErr: "pass --source"}, + {name: "multi requires flag", sources: multi, wantErr: "Pass --source"}, {name: "multi explicit", sources: multi, flag: "beta", want: "beta"}, {name: "multi unknown", sources: multi, flag: "nope", wantErr: "not registered"}, {name: "none registered", sources: nil, wantErr: "no migration sources"}, @@ -161,11 +168,10 @@ func TestArgValidation(t *testing.T) { {name: "down zero", args: []string{"down", "0"}, wantErr: "positive integer"}, {name: "goto non-integer", args: []string{"goto", "abc"}, wantErr: "non-negative integer"}, {name: "goto negative", args: []string{"goto", "--", "-1"}, wantErr: "non-negative integer"}, - {name: "force non-integer", args: []string{"force", "abc"}, wantErr: "non-negative integer"}, {name: "up rejects source flag", args: []string{"up", "--source", "alpha"}, wantErr: "--source is not applicable"}, {name: "status rejects source flag", args: []string{"status", "--source", "alpha"}, wantErr: "--source is not applicable"}, {name: "status invalid output", args: []string{"status", "--output", "yaml"}, wantErr: "invalid --output"}, - {name: "no dsn", args: []string{"version"}, wantErr: "database URL not set"}, + {name: "no dsn", args: []string{"version"}, wantErr: "set the database URL"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -216,7 +222,7 @@ func TestNewCommandFromFunc(t *testing.T) { cmd.SetErr(io.Discard) cmd.SetArgs([]string{"version", "--db-url", "postgres://unused"}) err := cmd.ExecuteContext(context.Background()) - if err == nil || !strings.Contains(err.Error(), "configured twice") { + if err == nil || !strings.Contains(err.Error(), "appears twice") { t.Fatalf("error = %v, want duplicate-name error", err) } }) @@ -229,13 +235,13 @@ func TestStatusJSONShape(t *testing.T) { payload := statusJSON{ Applied: 3, Pending: 1, - Sources: []statusSourceJSON{{Name: "alpha", Applied: 2, Pending: 1, Version: 2, Downgraded: false, Dirty: true}}, + Sources: []statusSourceJSON{{Name: "alpha", Applied: 2, Pending: 1, Version: 2, Downgraded: false}}, } got, err := json.Marshal(payload) if err != nil { t.Fatal(err) } - want := `{"applied":3,"pending":1,"sources":[{"name":"alpha","applied":2,"pending":1,"version":2,"downgraded":false,"dirty":true}]}` + want := `{"applied":3,"pending":1,"sources":[{"name":"alpha","applied":2,"pending":1,"version":2,"downgraded":false}]}` if string(got) != want { t.Errorf("status JSON shape changed:\n got: %s\nwant: %s", got, want) } @@ -244,7 +250,7 @@ func TestStatusJSONShape(t *testing.T) { // --- database-backed tests --- // TestCLIAgainstPostgres walks the operator surface end to end against a real -// Postgres: up, status, version, down, goto, dirty refusal, and force. The +// Postgres: up, status, version, down, and goto. The // subtests share one container and run in order — each builds on the schema // state the previous one left. func TestCLIAgainstPostgres(t *testing.T) { @@ -271,11 +277,11 @@ func TestCLIAgainstPostgres(t *testing.T) { } t.Run("up applies all sources", func(t *testing.T) { - mustContain(t, mustRun(t, "up"), "applied 3 migration(s)") + mustContain(t, mustRun(t, "up"), "schema is up to date") }) t.Run("up is idempotent", func(t *testing.T) { - mustContain(t, mustRun(t, "up"), "no pending migrations") + mustContain(t, mustRun(t, "up"), "schema is up to date") }) t.Run("status text", func(t *testing.T) { @@ -312,7 +318,7 @@ func TestCLIAgainstPostgres(t *testing.T) { }) t.Run("goto zero empties the source", func(t *testing.T) { - mustContain(t, mustRun(t, "goto", "0", "--source", "beta"), "version 0 (empty)") + mustContain(t, mustRun(t, "goto", "0", "--source", "beta"), "version 0 (no migrations applied)") mustContain(t, mustRun(t, "version", "--source", "beta"), "no migrations applied") }) @@ -320,67 +326,35 @@ func TestCLIAgainstPostgres(t *testing.T) { mustContain(t, mustRun(t, "down", "1", "--source", "beta"), "no migrations to roll back") }) - t.Run("dirty source is refused and force recovers", func(t *testing.T) { - markDirty(t, dsn, "alpha_schema_migrations") - for _, args := range [][]string{{"up"}, {"down", "1", "--source", "alpha"}, {"goto", "1", "--source", "alpha"}} { - _, _, err := runCLI(t, sources, append(args, "--db-url", dsn)...) - if err == nil || !strings.Contains(err.Error(), "dirty") { - t.Fatalf("%v: error = %v, want dirty refusal", args, err) - } - } - // status still reports rather than refusing, and annotates the row. - mustContain(t, mustRun(t, "status"), "(dirty)") - - mustContain(t, mustRun(t, "force", "2", "--source", "alpha"), "version 2 marked as applied") - mustContain(t, mustRun(t, "up"), "applied 1 migration(s)") // beta was left at 0 by the goto above - }) - - t.Run("force rejects unshipped version", func(t *testing.T) { - _, _, err := runCLI(t, sources, "force", "99", "--source", "alpha", "--db-url", dsn) - if err == nil || !strings.Contains(err.Error(), "not a shipped migration") { - t.Fatalf("error = %v, want unshipped-version rejection", err) - } - }) - - t.Run("force zero clears the version record", func(t *testing.T) { - mustContain(t, mustRun(t, "force", "0", "--source", "beta"), "version record cleared") - mustContain(t, mustRun(t, "version", "--source", "beta"), "no migrations applied") - mustContain(t, mustRun(t, "up"), "applied 1 migration(s)") + t.Run("up restores a source after goto zero", func(t *testing.T) { + mustContain(t, mustRun(t, "up"), "schema is up to date") }) } -// markDirty flips the dirty flag on a tracking table, simulating a process -// that died mid-migration. -func markDirty(t *testing.T, dsn, table string) { - t.Helper() - db, err := sql.Open("pgx", dsn) - if err != nil { - t.Fatal(err) - } - defer db.Close() - if _, err := db.Exec("UPDATE " + table + " SET dirty = true"); err != nil { - t.Fatalf("mark %s dirty: %v", table, err) - } -} - func TestSourceFileVersions(t *testing.T) { tests := []struct { - name string - files []string - want []int + name string + files []string + want []int64 + wantErr bool }{ - {"standard format", []string{"000001_create.up.sql", "000002_alter.up.sql"}, []int{1, 2}}, - {"no underscore ignored", []string{"000003foo.up.sql"}, nil}, - {"no leading digits ignored", []string{"notes.up.sql"}, nil}, - {"down files ignored", []string{"000001_create.down.sql", "000001_create.up.sql"}, []int{1}}, + {"standard format", []string{"000001_create.sql", "000002_alter.sql"}, []int64{1, 2}, false}, + {"non-SQL ignored", []string{"README.md"}, nil, false}, + {"legacy split rejected", []string{"000001_create.up.sql"}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mfs := fstest.MapFS{} for _, f := range tt.files { - mfs["m/"+f] = &fstest.MapFile{Data: []byte("-- sql")} + mfs["m/"+f] = &fstest.MapFile{Data: migrationFile("SELECT 1;", "SELECT 1;")} } got, err := sourceFileVersions(migrations.Source{FS: mfs, Dir: "m"}) + if tt.wantErr { + if err == nil { + t.Fatal("sourceFileVersions succeeded") + } + return + } if err != nil { t.Fatal(err) } diff --git a/go/core/pkg/migrations/core/000001_initial.down.sql b/go/core/pkg/migrations/core/000001_initial.down.sql deleted file mode 100644 index e7362c1ec..000000000 --- a/go/core/pkg/migrations/core/000001_initial.down.sql +++ /dev/null @@ -1,6 +0,0 @@ -DROP TABLE IF EXISTS crewai_flow_state; -DROP TABLE IF EXISTS crewai_agent_memory; -DROP TABLE IF EXISTS lg_checkpoint_write; -DROP TABLE IF EXISTS lg_checkpoint; -DROP TABLE IF EXISTS toolserver; -DROP TABLE IF EXISTS tool; diff --git a/go/core/pkg/migrations/core/000001_initial.sql b/go/core/pkg/migrations/core/000001_initial.sql new file mode 100644 index 000000000..fafed6b50 --- /dev/null +++ b/go/core/pkg/migrations/core/000001_initial.sql @@ -0,0 +1,258 @@ +-- +goose Up + +-- Kagent 1.0 baseline. These definitions match the schema produced by the +-- pre-Goose migration sequence on a fresh database. + +CREATE TABLE tool ( + id TEXT NOT NULL, + server_name TEXT NOT NULL, + group_kind TEXT NOT NULL, + created_at TIMESTAMPTZ, + updated_at TIMESTAMPTZ, + deleted_at TIMESTAMPTZ, + description TEXT, + PRIMARY KEY (id, server_name, group_kind) +); +CREATE INDEX idx_tool_deleted_at ON tool(deleted_at); + +CREATE TABLE toolserver ( + name TEXT NOT NULL, + group_kind TEXT NOT NULL, + created_at TIMESTAMPTZ, + updated_at TIMESTAMPTZ, + deleted_at TIMESTAMPTZ, + description TEXT, + last_connected TIMESTAMPTZ, + PRIMARY KEY (name, group_kind) +); +CREATE INDEX idx_toolserver_deleted_at ON toolserver(deleted_at); + +CREATE TABLE lg_checkpoint ( + user_id TEXT NOT NULL, + thread_id TEXT NOT NULL, + checkpoint_ns TEXT NOT NULL DEFAULT '', + checkpoint_id TEXT NOT NULL, + parent_checkpoint_id TEXT, + created_at TIMESTAMPTZ, + updated_at TIMESTAMPTZ, + deleted_at TIMESTAMPTZ, + metadata TEXT NOT NULL, + checkpoint TEXT NOT NULL, + checkpoint_type TEXT NOT NULL, + version BIGINT NOT NULL DEFAULT 1, + PRIMARY KEY (user_id, thread_id, checkpoint_ns, checkpoint_id) +); +CREATE INDEX idx_lg_checkpoint_parent_checkpoint_id ON lg_checkpoint(parent_checkpoint_id); +CREATE INDEX idx_lgcp_list ON lg_checkpoint(created_at); +CREATE INDEX idx_lg_checkpoint_deleted_at ON lg_checkpoint(deleted_at); + +CREATE TABLE lg_checkpoint_write ( + user_id TEXT NOT NULL, + thread_id TEXT NOT NULL, + checkpoint_ns TEXT NOT NULL DEFAULT '', + checkpoint_id TEXT NOT NULL, + write_idx BIGINT NOT NULL, + value TEXT NOT NULL, + value_type TEXT NOT NULL, + channel TEXT NOT NULL, + task_id TEXT NOT NULL, + created_at TIMESTAMPTZ, + updated_at TIMESTAMPTZ, + deleted_at TIMESTAMPTZ, + PRIMARY KEY (user_id, thread_id, checkpoint_ns, checkpoint_id, write_idx) +); +CREATE INDEX idx_lg_checkpoint_write_deleted_at ON lg_checkpoint_write(deleted_at); + +CREATE TABLE crewai_agent_memory ( + user_id TEXT NOT NULL, + thread_id TEXT NOT NULL, + created_at TIMESTAMPTZ, + updated_at TIMESTAMPTZ, + deleted_at TIMESTAMPTZ, + memory_data TEXT NOT NULL, + PRIMARY KEY (user_id, thread_id) +); +CREATE INDEX idx_crewai_memory_list ON crewai_agent_memory(created_at); +CREATE INDEX idx_crewai_agent_memory_deleted_at ON crewai_agent_memory(deleted_at); + +CREATE TABLE crewai_flow_state ( + user_id TEXT NOT NULL, + thread_id TEXT NOT NULL, + method_name TEXT NOT NULL, + created_at TIMESTAMPTZ, + updated_at TIMESTAMPTZ, + deleted_at TIMESTAMPTZ, + state_data TEXT NOT NULL, + PRIMARY KEY (user_id, thread_id, method_name) +); +CREATE INDEX idx_crewai_flow_state_list ON crewai_flow_state(created_at); +CREATE INDEX idx_crewai_flow_state_deleted_at ON crewai_flow_state(deleted_at); + +CREATE TABLE runtime_revision ( + revision TEXT PRIMARY KEY, + namespace TEXT NOT NULL, + agent_template_name TEXT NOT NULL, + agent_template_uid TEXT NOT NULL, + harness_name TEXT NOT NULL, + harness_uid TEXT NOT NULL, + source_snapshot JSONB NOT NULL, + egress_destinations TEXT[] NOT NULL DEFAULT '{}', + actor_template_atespace TEXT CONSTRAINT runtime_revision_actor_template_namespace_not_null NOT NULL, + actor_template_name TEXT NOT NULL, + actor_template_uid TEXT NOT NULL DEFAULT '', + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + agent_card JSONB NOT NULL, + CONSTRAINT runtime_revision_actor_template_namespace_actor_template_na_key + UNIQUE (actor_template_atespace, actor_template_name) +); + +CREATE TABLE agent_template_harness_pair ( + namespace TEXT NOT NULL, + agent_template_name TEXT NOT NULL, + agent_template_uid TEXT NOT NULL, + harness_name TEXT NOT NULL, + harness_uid TEXT NOT NULL, + desired_revision TEXT NOT NULL, + latest_successful_revision TEXT REFERENCES runtime_revision(revision) ON DELETE RESTRICT, + retired_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + agent_template_labels JSONB NOT NULL DEFAULT '{}', + PRIMARY KEY (namespace, agent_template_uid, harness_uid) +); +CREATE INDEX agent_template_harness_pair_name_idx + ON agent_template_harness_pair (namespace, agent_template_name, harness_name); + +CREATE TABLE a2a_context ( + id UUID PRIMARY KEY, + namespace TEXT NOT NULL, + user_id TEXT NOT NULL CHECK (user_id <> ''), + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE TABLE agent_instance_checkpoint ( + id UUID PRIMARY KEY, + namespace TEXT NOT NULL, + source_instance_id UUID NOT NULL, + user_id TEXT NOT NULL, + request_id TEXT NOT NULL, + head_task_id TEXT NOT NULL, + history_sequence BIGINT NOT NULL, + snapshot_atespace TEXT NOT NULL, + snapshot_name TEXT NOT NULL, + snapshot_uid TEXT NOT NULL, + snapshot_content_scope TEXT NOT NULL, + tag_uid TEXT NOT NULL DEFAULT '', + state TEXT NOT NULL, + failure TEXT NOT NULL DEFAULT '', + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + source_context_id UUID NOT NULL REFERENCES a2a_context(id) ON DELETE RESTRICT, + prepared_revision TEXT REFERENCES runtime_revision(revision) ON DELETE RESTRICT, + source_labels JSONB NOT NULL DEFAULT '{}' + CHECK (jsonb_typeof(source_labels) = 'object'), + CHECK (snapshot_content_scope IN ('FULL', 'DATA')), + CHECK (state IN ('CREATING', 'READY', 'FAILED', 'DELETING')), + UNIQUE (user_id, namespace, request_id) +); +CREATE INDEX agent_instance_checkpoint_list_idx + ON agent_instance_checkpoint (namespace, source_instance_id, id); +CREATE UNIQUE INDEX agent_instance_checkpoint_one_creating_idx + ON agent_instance_checkpoint (source_instance_id) + WHERE state = 'CREATING'; + +CREATE TABLE agent_instance ( + id UUID PRIMARY KEY, + namespace TEXT NOT NULL, + user_id TEXT NOT NULL CHECK (user_id <> ''), + request_id TEXT NOT NULL, + prepared_revision TEXT REFERENCES runtime_revision(revision) ON DELETE RESTRICT, + state TEXT NOT NULL, + labels JSONB NOT NULL DEFAULT '{}', + data BYTEA NOT NULL, + operation TEXT NOT NULL DEFAULT 'NONE', + context_id UUID NOT NULL REFERENCES a2a_context(id) ON DELETE RESTRICT, + source_checkpoint_id UUID REFERENCES agent_instance_checkpoint(id) ON DELETE RESTRICT, + name TEXT NOT NULL DEFAULT '', + CONSTRAINT agent_instance_operation_check + CHECK (operation IN ('NONE', 'CREATE', 'SUSPEND', 'RESUME', 'DELETE')), + CHECK (state IN ('CREATING', 'READY', 'SUSPENDED', 'FAILED')), + UNIQUE (user_id, namespace, request_id) +); +CREATE INDEX agent_instance_namespace_user_id_id_idx + ON agent_instance (namespace, user_id, id); + +CREATE TABLE agent_instance_share ( + id UUID PRIMARY KEY, + namespace TEXT NOT NULL, + instance_id UUID NOT NULL REFERENCES agent_instance(id) ON DELETE CASCADE, + permission TEXT NOT NULL CHECK (permission IN ('READ_ONLY', 'READ_WRITE')), + token_hash BYTEA NOT NULL UNIQUE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE INDEX agent_instance_share_instance_idx + ON agent_instance_share (namespace, instance_id, id); + +CREATE TABLE agent_instance_task ( + context_id UUID CONSTRAINT agent_instance_task_instance_id_not_null NOT NULL REFERENCES a2a_context(id) ON DELETE CASCADE, + id TEXT NOT NULL, + state TEXT NOT NULL, + status_timestamp TIMESTAMPTZ, + data BYTEA NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + initial_message_id TEXT, + request_hash BYTEA, + snapshot_atespace TEXT, + snapshot_name TEXT, + snapshot_uid TEXT, + snapshot_content_scope TEXT, + history_sequence BIGINT, + PRIMARY KEY (context_id, id) +); +CREATE UNIQUE INDEX agent_instance_one_active_task_idx + ON agent_instance_task (context_id) + WHERE state NOT IN ( + 'TASK_STATE_COMPLETED', + 'TASK_STATE_CANCELED', + 'TASK_STATE_FAILED', + 'TASK_STATE_REJECTED', + 'TASK_STATE_INPUT_REQUIRED', + 'TASK_STATE_AUTH_REQUIRED' + ); +CREATE INDEX agent_instance_task_list_idx + ON agent_instance_task (context_id, id); +CREATE UNIQUE INDEX agent_instance_task_message_idx + ON agent_instance_task (context_id, initial_message_id) + WHERE initial_message_id IS NOT NULL; + +CREATE TABLE agent_instance_task_event ( + sequence BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + context_id UUID CONSTRAINT agent_instance_task_event_instance_id_not_null NOT NULL REFERENCES a2a_context(id) ON DELETE CASCADE, + task_id TEXT, + data BYTEA NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + message_id TEXT +); +CREATE INDEX agent_instance_task_event_instance_sequence_idx + ON agent_instance_task_event (context_id, sequence); +CREATE UNIQUE INDEX agent_instance_task_event_message_idx + ON agent_instance_task_event (context_id, task_id, message_id) + WHERE message_id IS NOT NULL; + +-- +goose Down + +DROP TABLE agent_instance_share; +DROP TABLE agent_instance_task_event; +DROP TABLE agent_instance_task; +DROP TABLE agent_instance; +DROP TABLE agent_instance_checkpoint; +DROP TABLE a2a_context; +DROP TABLE agent_template_harness_pair; +DROP TABLE runtime_revision; +DROP TABLE crewai_flow_state; +DROP TABLE crewai_agent_memory; +DROP TABLE lg_checkpoint_write; +DROP TABLE lg_checkpoint; +DROP TABLE toolserver; +DROP TABLE tool; diff --git a/go/core/pkg/migrations/core/000001_initial.up.sql b/go/core/pkg/migrations/core/000001_initial.up.sql deleted file mode 100644 index c8a3c9170..000000000 --- a/go/core/pkg/migrations/core/000001_initial.up.sql +++ /dev/null @@ -1,92 +0,0 @@ --- Baseline migration: matches the schema produced by GORM AutoMigrate as of --- kagent v0.8.0. Upgrading to v0.8.0 before this version is required. --- --- Notes on column definitions vs. what you might expect: --- - created_at/updated_at are nullable: GORM sets these in Go code, not via a --- DB default or NOT NULL constraint. --- - version, write_idx, access_count are BIGINT: GORM maps Go `int` to bigint. - -CREATE TABLE IF NOT EXISTS tool ( - id TEXT NOT NULL, - server_name TEXT NOT NULL, - group_kind TEXT NOT NULL, - created_at TIMESTAMPTZ, - updated_at TIMESTAMPTZ, - deleted_at TIMESTAMPTZ, - description TEXT, - PRIMARY KEY (id, server_name, group_kind) -); -CREATE INDEX IF NOT EXISTS idx_tool_deleted_at ON tool(deleted_at); - -CREATE TABLE IF NOT EXISTS toolserver ( - name TEXT NOT NULL, - group_kind TEXT NOT NULL, - created_at TIMESTAMPTZ, - updated_at TIMESTAMPTZ, - deleted_at TIMESTAMPTZ, - description TEXT, - last_connected TIMESTAMPTZ, - PRIMARY KEY (name, group_kind) -); -CREATE INDEX IF NOT EXISTS idx_toolserver_deleted_at ON toolserver(deleted_at); - -CREATE TABLE IF NOT EXISTS lg_checkpoint ( - user_id TEXT NOT NULL, - thread_id TEXT NOT NULL, - checkpoint_ns TEXT NOT NULL DEFAULT '', - checkpoint_id TEXT NOT NULL, - parent_checkpoint_id TEXT, - created_at TIMESTAMPTZ, - updated_at TIMESTAMPTZ, - deleted_at TIMESTAMPTZ, - metadata TEXT NOT NULL, - checkpoint TEXT NOT NULL, - checkpoint_type TEXT NOT NULL, - version BIGINT NOT NULL DEFAULT 1, - PRIMARY KEY (user_id, thread_id, checkpoint_ns, checkpoint_id) -); -CREATE INDEX IF NOT EXISTS idx_lg_checkpoint_parent_checkpoint_id ON lg_checkpoint(parent_checkpoint_id); -CREATE INDEX IF NOT EXISTS idx_lgcp_list ON lg_checkpoint(created_at); -CREATE INDEX IF NOT EXISTS idx_lg_checkpoint_deleted_at ON lg_checkpoint(deleted_at); - -CREATE TABLE IF NOT EXISTS lg_checkpoint_write ( - user_id TEXT NOT NULL, - thread_id TEXT NOT NULL, - checkpoint_ns TEXT NOT NULL DEFAULT '', - checkpoint_id TEXT NOT NULL, - write_idx BIGINT NOT NULL, - value TEXT NOT NULL, - value_type TEXT NOT NULL, - channel TEXT NOT NULL, - task_id TEXT NOT NULL, - created_at TIMESTAMPTZ, - updated_at TIMESTAMPTZ, - deleted_at TIMESTAMPTZ, - PRIMARY KEY (user_id, thread_id, checkpoint_ns, checkpoint_id, write_idx) -); -CREATE INDEX IF NOT EXISTS idx_lg_checkpoint_write_deleted_at ON lg_checkpoint_write(deleted_at); - -CREATE TABLE IF NOT EXISTS crewai_agent_memory ( - user_id TEXT NOT NULL, - thread_id TEXT NOT NULL, - created_at TIMESTAMPTZ, - updated_at TIMESTAMPTZ, - deleted_at TIMESTAMPTZ, - memory_data TEXT NOT NULL, - PRIMARY KEY (user_id, thread_id) -); -CREATE INDEX IF NOT EXISTS idx_crewai_memory_list ON crewai_agent_memory(created_at); -CREATE INDEX IF NOT EXISTS idx_crewai_agent_memory_deleted_at ON crewai_agent_memory(deleted_at); - -CREATE TABLE IF NOT EXISTS crewai_flow_state ( - user_id TEXT NOT NULL, - thread_id TEXT NOT NULL, - method_name TEXT NOT NULL, - created_at TIMESTAMPTZ, - updated_at TIMESTAMPTZ, - deleted_at TIMESTAMPTZ, - state_data TEXT NOT NULL, - PRIMARY KEY (user_id, thread_id, method_name) -); -CREATE INDEX IF NOT EXISTS idx_crewai_flow_state_list ON crewai_flow_state(created_at); -CREATE INDEX IF NOT EXISTS idx_crewai_flow_state_deleted_at ON crewai_flow_state(deleted_at); diff --git a/go/core/pkg/migrations/core/000002_not_null_defaults.down.sql b/go/core/pkg/migrations/core/000002_not_null_defaults.down.sql deleted file mode 100644 index 0c6bc144a..000000000 --- a/go/core/pkg/migrations/core/000002_not_null_defaults.down.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE lg_checkpoint ALTER COLUMN version DROP NOT NULL; diff --git a/go/core/pkg/migrations/core/000002_not_null_defaults.up.sql b/go/core/pkg/migrations/core/000002_not_null_defaults.up.sql deleted file mode 100644 index f834822d8..000000000 --- a/go/core/pkg/migrations/core/000002_not_null_defaults.up.sql +++ /dev/null @@ -1,5 +0,0 @@ --- Backfill any NULLs (none expected, but safe) then add NOT NULL constraints. --- These columns always had DEFAULT values but were missing NOT NULL in 000001. - -UPDATE lg_checkpoint SET version = 1 WHERE version IS NULL; -ALTER TABLE lg_checkpoint ALTER COLUMN version SET NOT NULL; diff --git a/go/core/pkg/migrations/core/000008_runtime_revisions.down.sql b/go/core/pkg/migrations/core/000008_runtime_revisions.down.sql deleted file mode 100644 index 6c4b71cc4..000000000 --- a/go/core/pkg/migrations/core/000008_runtime_revisions.down.sql +++ /dev/null @@ -1,2 +0,0 @@ -DROP TABLE IF EXISTS agent_template_harness_pair; -DROP TABLE IF EXISTS runtime_revision; diff --git a/go/core/pkg/migrations/core/000008_runtime_revisions.up.sql b/go/core/pkg/migrations/core/000008_runtime_revisions.up.sql deleted file mode 100644 index 1b311ff0b..000000000 --- a/go/core/pkg/migrations/core/000008_runtime_revisions.up.sql +++ /dev/null @@ -1,35 +0,0 @@ -CREATE TABLE IF NOT EXISTS runtime_revision ( - revision TEXT PRIMARY KEY, - namespace TEXT NOT NULL, - agent_template_name TEXT NOT NULL, - agent_template_uid TEXT NOT NULL, - harness_name TEXT NOT NULL, - harness_uid TEXT NOT NULL, - source_snapshot JSONB NOT NULL, - egress_destinations TEXT[] NOT NULL DEFAULT '{}', - actor_template_namespace TEXT NOT NULL, - actor_template_name TEXT NOT NULL, - actor_template_uid TEXT NOT NULL DEFAULT '', - phase TEXT NOT NULL, - golden_snapshot TEXT NOT NULL DEFAULT '', - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - UNIQUE (actor_template_namespace, actor_template_name) -); - -CREATE TABLE IF NOT EXISTS agent_template_harness_pair ( - namespace TEXT NOT NULL, - agent_template_name TEXT NOT NULL, - agent_template_uid TEXT NOT NULL, - harness_name TEXT NOT NULL, - harness_uid TEXT NOT NULL, - desired_revision TEXT NOT NULL, - latest_successful_revision TEXT REFERENCES runtime_revision(revision) ON DELETE RESTRICT, - retired_at TIMESTAMPTZ, - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - PRIMARY KEY (namespace, agent_template_uid, harness_uid) -); - -CREATE INDEX IF NOT EXISTS agent_template_harness_pair_name_idx - ON agent_template_harness_pair (namespace, agent_template_name, harness_name); diff --git a/go/core/pkg/migrations/core/000009_agent_instances.down.sql b/go/core/pkg/migrations/core/000009_agent_instances.down.sql deleted file mode 100644 index 8a942f107..000000000 --- a/go/core/pkg/migrations/core/000009_agent_instances.down.sql +++ /dev/null @@ -1,6 +0,0 @@ -DROP INDEX IF EXISTS agent_instance_share_instance_idx; -DROP TABLE IF EXISTS agent_instance_share; -DROP INDEX IF EXISTS agent_instance_namespace_user_id_id_idx; -DROP TABLE IF EXISTS agent_instance; -ALTER TABLE agent_template_harness_pair - DROP COLUMN IF EXISTS agent_template_labels; diff --git a/go/core/pkg/migrations/core/000009_agent_instances.up.sql b/go/core/pkg/migrations/core/000009_agent_instances.up.sql deleted file mode 100644 index 81bda30b4..000000000 --- a/go/core/pkg/migrations/core/000009_agent_instances.up.sql +++ /dev/null @@ -1,31 +0,0 @@ -ALTER TABLE agent_template_harness_pair - ADD COLUMN IF NOT EXISTS agent_template_labels JSONB NOT NULL DEFAULT '{}'; - -CREATE TABLE IF NOT EXISTS agent_instance ( - id TEXT PRIMARY KEY, - namespace TEXT NOT NULL, - user_id TEXT NOT NULL CHECK (user_id <> ''), - request_id TEXT NOT NULL, - prepared_revision TEXT REFERENCES runtime_revision(revision) ON DELETE RESTRICT, - state TEXT NOT NULL, - labels JSONB NOT NULL DEFAULT '{}', - data BYTEA NOT NULL, - CHECK (state IN ('CREATING', 'READY', 'SUSPENDED', 'FAILED')), - UNIQUE (user_id, namespace, request_id) -); - -CREATE INDEX IF NOT EXISTS agent_instance_namespace_user_id_id_idx - ON agent_instance (namespace, user_id, id); - -CREATE TABLE IF NOT EXISTS agent_instance_share ( - id TEXT PRIMARY KEY, - namespace TEXT NOT NULL, - instance_id TEXT NOT NULL REFERENCES agent_instance(id) ON DELETE CASCADE, - permission TEXT NOT NULL, - token_hash BYTEA NOT NULL UNIQUE, - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - CHECK (permission IN ('READ_ONLY', 'READ_WRITE')) -); - -CREATE INDEX IF NOT EXISTS agent_instance_share_instance_idx - ON agent_instance_share (namespace, instance_id, id); diff --git a/go/core/pkg/migrations/core/000010_agent_instance_operation.down.sql b/go/core/pkg/migrations/core/000010_agent_instance_operation.down.sql deleted file mode 100644 index e25813a5d..000000000 --- a/go/core/pkg/migrations/core/000010_agent_instance_operation.down.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE agent_instance DROP COLUMN IF EXISTS operation; diff --git a/go/core/pkg/migrations/core/000010_agent_instance_operation.up.sql b/go/core/pkg/migrations/core/000010_agent_instance_operation.up.sql deleted file mode 100644 index c9b42a8a1..000000000 --- a/go/core/pkg/migrations/core/000010_agent_instance_operation.up.sql +++ /dev/null @@ -1,10 +0,0 @@ --- The protobuf remains the public record; this column exists only so lifecycle --- operations can use an atomic compare-and-set across controller replicas. -ALTER TABLE agent_instance - ADD COLUMN IF NOT EXISTS operation TEXT NOT NULL DEFAULT 'NONE', - ADD CONSTRAINT agent_instance_operation_check - CHECK (operation IN ('NONE', 'CREATE', 'SUSPEND', 'RESUME', 'DELETE')); - -UPDATE agent_instance -SET operation = 'CREATE' -WHERE state = 'CREATING'; diff --git a/go/core/pkg/migrations/core/000011_agent_instance_tasks.down.sql b/go/core/pkg/migrations/core/000011_agent_instance_tasks.down.sql deleted file mode 100644 index 3f321bc7b..000000000 --- a/go/core/pkg/migrations/core/000011_agent_instance_tasks.down.sql +++ /dev/null @@ -1,5 +0,0 @@ -DROP INDEX IF EXISTS agent_instance_task_event_instance_sequence_idx; -DROP TABLE IF EXISTS agent_instance_task_event; -DROP INDEX IF EXISTS agent_instance_task_list_idx; -DROP INDEX IF EXISTS agent_instance_one_active_task_idx; -DROP TABLE IF EXISTS agent_instance_task; diff --git a/go/core/pkg/migrations/core/000011_agent_instance_tasks.up.sql b/go/core/pkg/migrations/core/000011_agent_instance_tasks.up.sql deleted file mode 100644 index b39179739..000000000 --- a/go/core/pkg/migrations/core/000011_agent_instance_tasks.up.sql +++ /dev/null @@ -1,33 +0,0 @@ -CREATE TABLE IF NOT EXISTS agent_instance_task ( - instance_id TEXT NOT NULL REFERENCES agent_instance(id) ON DELETE CASCADE, - id TEXT NOT NULL, - state TEXT NOT NULL, - status_timestamp TIMESTAMPTZ, - data BYTEA NOT NULL, - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - PRIMARY KEY (instance_id, id) -); - -CREATE UNIQUE INDEX IF NOT EXISTS agent_instance_one_active_task_idx - ON agent_instance_task (instance_id) - WHERE state NOT IN ( - 'TASK_STATE_COMPLETED', - 'TASK_STATE_CANCELED', - 'TASK_STATE_FAILED', - 'TASK_STATE_REJECTED' - ); - -CREATE INDEX IF NOT EXISTS agent_instance_task_list_idx - ON agent_instance_task (instance_id, id); - -CREATE TABLE IF NOT EXISTS agent_instance_task_event ( - sequence BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, - instance_id TEXT NOT NULL REFERENCES agent_instance(id) ON DELETE CASCADE, - task_id TEXT, - data BYTEA NOT NULL, - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() -); - -CREATE INDEX IF NOT EXISTS agent_instance_task_event_instance_sequence_idx - ON agent_instance_task_event (instance_id, sequence); diff --git a/go/core/pkg/migrations/core/000012_agent_instance_message_idempotency.down.sql b/go/core/pkg/migrations/core/000012_agent_instance_message_idempotency.down.sql deleted file mode 100644 index cbe02b80f..000000000 --- a/go/core/pkg/migrations/core/000012_agent_instance_message_idempotency.down.sql +++ /dev/null @@ -1,4 +0,0 @@ -DROP INDEX IF EXISTS agent_instance_task_message_idx; -ALTER TABLE agent_instance_task - DROP COLUMN IF EXISTS request_hash, - DROP COLUMN IF EXISTS initial_message_id; diff --git a/go/core/pkg/migrations/core/000012_agent_instance_message_idempotency.up.sql b/go/core/pkg/migrations/core/000012_agent_instance_message_idempotency.up.sql deleted file mode 100644 index 5ab17eefa..000000000 --- a/go/core/pkg/migrations/core/000012_agent_instance_message_idempotency.up.sql +++ /dev/null @@ -1,7 +0,0 @@ -ALTER TABLE agent_instance_task - ADD COLUMN IF NOT EXISTS initial_message_id TEXT, - ADD COLUMN IF NOT EXISTS request_hash BYTEA; - -CREATE UNIQUE INDEX IF NOT EXISTS agent_instance_task_message_idx - ON agent_instance_task (instance_id, initial_message_id) - WHERE initial_message_id IS NOT NULL; diff --git a/go/core/pkg/migrations/core/000013_runtime_revision_agent_card.down.sql b/go/core/pkg/migrations/core/000013_runtime_revision_agent_card.down.sql deleted file mode 100644 index 7f592815a..000000000 --- a/go/core/pkg/migrations/core/000013_runtime_revision_agent_card.down.sql +++ /dev/null @@ -1,2 +0,0 @@ -ALTER TABLE runtime_revision - DROP COLUMN IF EXISTS agent_card; diff --git a/go/core/pkg/migrations/core/000013_runtime_revision_agent_card.up.sql b/go/core/pkg/migrations/core/000013_runtime_revision_agent_card.up.sql deleted file mode 100644 index 363b61f14..000000000 --- a/go/core/pkg/migrations/core/000013_runtime_revision_agent_card.up.sql +++ /dev/null @@ -1,5 +0,0 @@ -ALTER TABLE runtime_revision - ADD COLUMN IF NOT EXISTS agent_card JSONB NOT NULL DEFAULT '{}'::jsonb; - -ALTER TABLE runtime_revision - ALTER COLUMN agent_card DROP DEFAULT; diff --git a/go/core/pkg/migrations/core/000014_agent_instance_task_snapshots.down.sql b/go/core/pkg/migrations/core/000014_agent_instance_task_snapshots.down.sql deleted file mode 100644 index b14af538b..000000000 --- a/go/core/pkg/migrations/core/000014_agent_instance_task_snapshots.down.sql +++ /dev/null @@ -1,16 +0,0 @@ -DROP INDEX IF EXISTS agent_instance_one_active_task_idx; -CREATE UNIQUE INDEX IF NOT EXISTS agent_instance_one_active_task_idx - ON agent_instance_task (instance_id) - WHERE state NOT IN ( - 'TASK_STATE_COMPLETED', - 'TASK_STATE_CANCELED', - 'TASK_STATE_FAILED', - 'TASK_STATE_REJECTED' - ); - -ALTER TABLE agent_instance_task - DROP COLUMN IF EXISTS history_sequence, - DROP COLUMN IF EXISTS snapshot_content_scope, - DROP COLUMN IF EXISTS snapshot_uid, - DROP COLUMN IF EXISTS snapshot_name, - DROP COLUMN IF EXISTS snapshot_atespace; diff --git a/go/core/pkg/migrations/core/000014_agent_instance_task_snapshots.up.sql b/go/core/pkg/migrations/core/000014_agent_instance_task_snapshots.up.sql deleted file mode 100644 index b0536982a..000000000 --- a/go/core/pkg/migrations/core/000014_agent_instance_task_snapshots.up.sql +++ /dev/null @@ -1,18 +0,0 @@ -ALTER TABLE agent_instance_task - ADD COLUMN IF NOT EXISTS snapshot_atespace TEXT, - ADD COLUMN IF NOT EXISTS snapshot_name TEXT, - ADD COLUMN IF NOT EXISTS snapshot_uid TEXT, - ADD COLUMN IF NOT EXISTS snapshot_content_scope TEXT, - ADD COLUMN IF NOT EXISTS history_sequence BIGINT; - -DROP INDEX IF EXISTS agent_instance_one_active_task_idx; -CREATE UNIQUE INDEX IF NOT EXISTS agent_instance_one_active_task_idx - ON agent_instance_task (instance_id) - WHERE state NOT IN ( - 'TASK_STATE_COMPLETED', - 'TASK_STATE_CANCELED', - 'TASK_STATE_FAILED', - 'TASK_STATE_REJECTED', - 'TASK_STATE_INPUT_REQUIRED', - 'TASK_STATE_AUTH_REQUIRED' - ); diff --git a/go/core/pkg/migrations/core/000015_agent_instance_checkpoints.down.sql b/go/core/pkg/migrations/core/000015_agent_instance_checkpoints.down.sql deleted file mode 100644 index 55827d162..000000000 --- a/go/core/pkg/migrations/core/000015_agent_instance_checkpoints.down.sql +++ /dev/null @@ -1,3 +0,0 @@ -DROP INDEX IF EXISTS agent_instance_checkpoint_list_idx; -DROP INDEX IF EXISTS agent_instance_checkpoint_one_creating_idx; -DROP TABLE IF EXISTS agent_instance_checkpoint; diff --git a/go/core/pkg/migrations/core/000015_agent_instance_checkpoints.up.sql b/go/core/pkg/migrations/core/000015_agent_instance_checkpoints.up.sql deleted file mode 100644 index 41f00ba81..000000000 --- a/go/core/pkg/migrations/core/000015_agent_instance_checkpoints.up.sql +++ /dev/null @@ -1,28 +0,0 @@ -CREATE TABLE IF NOT EXISTS agent_instance_checkpoint ( - id TEXT PRIMARY KEY, - namespace TEXT NOT NULL, - -- Provenance only: checkpoints outlive their source and may initialize other Actors. - source_instance_id TEXT NOT NULL, - user_id TEXT NOT NULL, - request_id TEXT NOT NULL, - head_task_id TEXT NOT NULL, - history_sequence BIGINT NOT NULL, - snapshot_atespace TEXT NOT NULL, - snapshot_name TEXT NOT NULL, - snapshot_uid TEXT NOT NULL, - snapshot_content_scope TEXT NOT NULL, - tag_uid TEXT NOT NULL DEFAULT '', - state TEXT NOT NULL, - failure TEXT NOT NULL DEFAULT '', - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - CHECK (snapshot_content_scope IN ('FULL', 'DATA')), - CHECK (state IN ('CREATING', 'READY', 'FAILED', 'DELETING')), - UNIQUE (user_id, namespace, request_id) -); - -CREATE INDEX IF NOT EXISTS agent_instance_checkpoint_list_idx - ON agent_instance_checkpoint (namespace, source_instance_id, id); - -CREATE UNIQUE INDEX IF NOT EXISTS agent_instance_checkpoint_one_creating_idx - ON agent_instance_checkpoint (source_instance_id) - WHERE state = 'CREATING'; diff --git a/go/core/pkg/migrations/core/000016_agent_instance_forks.down.sql b/go/core/pkg/migrations/core/000016_agent_instance_forks.down.sql deleted file mode 100644 index 08ba73a5b..000000000 --- a/go/core/pkg/migrations/core/000016_agent_instance_forks.down.sql +++ /dev/null @@ -1,35 +0,0 @@ -DROP INDEX IF EXISTS agent_instance_task_event_message_idx; - -ALTER TABLE agent_instance_task_event - DROP COLUMN IF EXISTS message_id; - -ALTER TABLE agent_instance_task_event - DROP CONSTRAINT IF EXISTS agent_instance_task_event_context_id_fkey; - -ALTER TABLE agent_instance_task_event - RENAME COLUMN context_id TO instance_id; - -ALTER TABLE agent_instance_task_event - ADD CONSTRAINT agent_instance_task_event_instance_id_fkey - FOREIGN KEY (instance_id) REFERENCES agent_instance(id) ON DELETE CASCADE; - -ALTER TABLE agent_instance_task - DROP CONSTRAINT IF EXISTS agent_instance_task_context_id_fkey; - -ALTER TABLE agent_instance_task - RENAME COLUMN context_id TO instance_id; - -ALTER TABLE agent_instance_task - ADD CONSTRAINT agent_instance_task_instance_id_fkey - FOREIGN KEY (instance_id) REFERENCES agent_instance(id) ON DELETE CASCADE; - -ALTER TABLE agent_instance - DROP COLUMN IF EXISTS source_checkpoint_id, - DROP COLUMN IF EXISTS context_id; - -ALTER TABLE agent_instance_checkpoint - DROP COLUMN IF EXISTS source_context_id, - DROP COLUMN IF EXISTS source_labels, - DROP COLUMN IF EXISTS prepared_revision; - -DROP TABLE IF EXISTS a2a_context; diff --git a/go/core/pkg/migrations/core/000016_agent_instance_forks.up.sql b/go/core/pkg/migrations/core/000016_agent_instance_forks.up.sql deleted file mode 100644 index e8b9ade7f..000000000 --- a/go/core/pkg/migrations/core/000016_agent_instance_forks.up.sql +++ /dev/null @@ -1,78 +0,0 @@ -CREATE TABLE IF NOT EXISTS a2a_context ( - id TEXT PRIMARY KEY, - namespace TEXT NOT NULL, - user_id TEXT NOT NULL CHECK (user_id <> ''), - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() -); - -INSERT INTO a2a_context (id, namespace, user_id) -SELECT id, namespace, user_id FROM agent_instance -ON CONFLICT DO NOTHING; - -INSERT INTO a2a_context (id, namespace, user_id, created_at) -SELECT source_instance_id, namespace, user_id, MIN(created_at) -FROM agent_instance_checkpoint -GROUP BY source_instance_id, namespace, user_id -ON CONFLICT DO NOTHING; - -ALTER TABLE agent_instance - ADD COLUMN IF NOT EXISTS context_id TEXT; - -UPDATE agent_instance SET context_id = id WHERE context_id IS NULL; - -ALTER TABLE agent_instance - ALTER COLUMN context_id SET NOT NULL, - ADD CONSTRAINT agent_instance_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE RESTRICT; - -ALTER TABLE agent_instance_task - DROP CONSTRAINT IF EXISTS agent_instance_task_instance_id_fkey; - -ALTER TABLE agent_instance_task - RENAME COLUMN instance_id TO context_id; - -ALTER TABLE agent_instance_task - ADD CONSTRAINT agent_instance_task_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE CASCADE; - -ALTER TABLE agent_instance_task_event - DROP CONSTRAINT IF EXISTS agent_instance_task_event_instance_id_fkey; - -ALTER TABLE agent_instance_task_event - RENAME COLUMN instance_id TO context_id; - -ALTER TABLE agent_instance_task_event - ADD CONSTRAINT agent_instance_task_event_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE CASCADE; - -ALTER TABLE agent_instance_task_event - ADD COLUMN IF NOT EXISTS message_id TEXT; - -CREATE UNIQUE INDEX IF NOT EXISTS agent_instance_task_event_message_idx - ON agent_instance_task_event (context_id, task_id, message_id) - WHERE message_id IS NOT NULL; - -ALTER TABLE agent_instance_checkpoint - ADD COLUMN IF NOT EXISTS source_context_id TEXT, - ADD COLUMN IF NOT EXISTS prepared_revision TEXT REFERENCES runtime_revision(revision) ON DELETE RESTRICT, - ADD COLUMN IF NOT EXISTS source_labels JSONB NOT NULL DEFAULT '{}' - CHECK (jsonb_typeof(source_labels) = 'object'); - -UPDATE agent_instance_checkpoint -SET source_context_id = source_instance_id -WHERE source_context_id IS NULL; - -ALTER TABLE agent_instance_checkpoint - ALTER COLUMN source_context_id SET NOT NULL, - ADD CONSTRAINT agent_instance_checkpoint_source_context_id_fkey - FOREIGN KEY (source_context_id) REFERENCES a2a_context(id) ON DELETE RESTRICT; - -UPDATE agent_instance_checkpoint c -SET prepared_revision = i.prepared_revision, - source_labels = i.labels -FROM agent_instance i -WHERE i.id = c.source_instance_id - AND c.prepared_revision IS NULL; - -ALTER TABLE agent_instance - ADD COLUMN IF NOT EXISTS source_checkpoint_id TEXT REFERENCES agent_instance_checkpoint(id) ON DELETE RESTRICT; diff --git a/go/core/pkg/migrations/core/000017_agent_instance_name.down.sql b/go/core/pkg/migrations/core/000017_agent_instance_name.down.sql deleted file mode 100644 index a0b66d34b..000000000 --- a/go/core/pkg/migrations/core/000017_agent_instance_name.down.sql +++ /dev/null @@ -1,2 +0,0 @@ -ALTER TABLE agent_instance - DROP COLUMN IF EXISTS name; diff --git a/go/core/pkg/migrations/core/000017_agent_instance_name.up.sql b/go/core/pkg/migrations/core/000017_agent_instance_name.up.sql deleted file mode 100644 index 0786dd47a..000000000 --- a/go/core/pkg/migrations/core/000017_agent_instance_name.up.sql +++ /dev/null @@ -1,6 +0,0 @@ --- A reader-supplied display name for the conversation an AgentInstance is. --- Deliberately not unique: unlike a Kubernetes name this is a label for a human, --- and two conversations with the same agent may reasonably carry the same title. --- The default keeps the column additive — every existing row reads as unnamed. -ALTER TABLE agent_instance - ADD COLUMN IF NOT EXISTS name TEXT NOT NULL DEFAULT ''; diff --git a/go/core/pkg/migrations/core/000018_agent_instance_uuid_ids.down.sql b/go/core/pkg/migrations/core/000018_agent_instance_uuid_ids.down.sql deleted file mode 100644 index b0a46cbb0..000000000 --- a/go/core/pkg/migrations/core/000018_agent_instance_uuid_ids.down.sql +++ /dev/null @@ -1,34 +0,0 @@ -ALTER TABLE agent_instance_share DROP CONSTRAINT agent_instance_share_instance_id_fkey; -ALTER TABLE agent_instance_task DROP CONSTRAINT agent_instance_task_context_id_fkey; -ALTER TABLE agent_instance_task_event DROP CONSTRAINT agent_instance_task_event_context_id_fkey; -ALTER TABLE agent_instance_checkpoint DROP CONSTRAINT agent_instance_checkpoint_source_context_id_fkey; -ALTER TABLE agent_instance DROP CONSTRAINT agent_instance_context_id_fkey; -ALTER TABLE agent_instance DROP CONSTRAINT agent_instance_source_checkpoint_id_fkey; - -ALTER TABLE a2a_context ALTER COLUMN id TYPE TEXT USING id::text; -ALTER TABLE agent_instance - ALTER COLUMN id TYPE TEXT USING id::text, - ALTER COLUMN context_id TYPE TEXT USING context_id::text, - ALTER COLUMN source_checkpoint_id TYPE TEXT USING source_checkpoint_id::text; -ALTER TABLE agent_instance_share - ALTER COLUMN id TYPE TEXT USING id::text, - ALTER COLUMN instance_id TYPE TEXT USING instance_id::text; -ALTER TABLE agent_instance_task ALTER COLUMN context_id TYPE TEXT USING context_id::text; -ALTER TABLE agent_instance_task_event ALTER COLUMN context_id TYPE TEXT USING context_id::text; -ALTER TABLE agent_instance_checkpoint - ALTER COLUMN id TYPE TEXT USING id::text, - ALTER COLUMN source_instance_id TYPE TEXT USING source_instance_id::text, - ALTER COLUMN source_context_id TYPE TEXT USING source_context_id::text; - -ALTER TABLE agent_instance_share ADD CONSTRAINT agent_instance_share_instance_id_fkey - FOREIGN KEY (instance_id) REFERENCES agent_instance(id) ON DELETE CASCADE; -ALTER TABLE agent_instance_task ADD CONSTRAINT agent_instance_task_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE CASCADE; -ALTER TABLE agent_instance_task_event ADD CONSTRAINT agent_instance_task_event_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE CASCADE; -ALTER TABLE agent_instance_checkpoint ADD CONSTRAINT agent_instance_checkpoint_source_context_id_fkey - FOREIGN KEY (source_context_id) REFERENCES a2a_context(id) ON DELETE RESTRICT; -ALTER TABLE agent_instance ADD CONSTRAINT agent_instance_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE RESTRICT; -ALTER TABLE agent_instance ADD CONSTRAINT agent_instance_source_checkpoint_id_fkey - FOREIGN KEY (source_checkpoint_id) REFERENCES agent_instance_checkpoint(id) ON DELETE RESTRICT; diff --git a/go/core/pkg/migrations/core/000018_agent_instance_uuid_ids.up.sql b/go/core/pkg/migrations/core/000018_agent_instance_uuid_ids.up.sql deleted file mode 100644 index 50de0fc58..000000000 --- a/go/core/pkg/migrations/core/000018_agent_instance_uuid_ids.up.sql +++ /dev/null @@ -1,34 +0,0 @@ -ALTER TABLE agent_instance_share DROP CONSTRAINT agent_instance_share_instance_id_fkey; -ALTER TABLE agent_instance_task DROP CONSTRAINT agent_instance_task_context_id_fkey; -ALTER TABLE agent_instance_task_event DROP CONSTRAINT agent_instance_task_event_context_id_fkey; -ALTER TABLE agent_instance_checkpoint DROP CONSTRAINT agent_instance_checkpoint_source_context_id_fkey; -ALTER TABLE agent_instance DROP CONSTRAINT agent_instance_context_id_fkey; -ALTER TABLE agent_instance DROP CONSTRAINT agent_instance_source_checkpoint_id_fkey; - -ALTER TABLE a2a_context ALTER COLUMN id TYPE UUID USING id::uuid; -ALTER TABLE agent_instance - ALTER COLUMN id TYPE UUID USING id::uuid, - ALTER COLUMN context_id TYPE UUID USING context_id::uuid, - ALTER COLUMN source_checkpoint_id TYPE UUID USING source_checkpoint_id::uuid; -ALTER TABLE agent_instance_share - ALTER COLUMN id TYPE UUID USING id::uuid, - ALTER COLUMN instance_id TYPE UUID USING instance_id::uuid; -ALTER TABLE agent_instance_task ALTER COLUMN context_id TYPE UUID USING context_id::uuid; -ALTER TABLE agent_instance_task_event ALTER COLUMN context_id TYPE UUID USING context_id::uuid; -ALTER TABLE agent_instance_checkpoint - ALTER COLUMN id TYPE UUID USING id::uuid, - ALTER COLUMN source_instance_id TYPE UUID USING source_instance_id::uuid, - ALTER COLUMN source_context_id TYPE UUID USING source_context_id::uuid; - -ALTER TABLE agent_instance_share ADD CONSTRAINT agent_instance_share_instance_id_fkey - FOREIGN KEY (instance_id) REFERENCES agent_instance(id) ON DELETE CASCADE; -ALTER TABLE agent_instance_task ADD CONSTRAINT agent_instance_task_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE CASCADE; -ALTER TABLE agent_instance_task_event ADD CONSTRAINT agent_instance_task_event_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE CASCADE; -ALTER TABLE agent_instance_checkpoint ADD CONSTRAINT agent_instance_checkpoint_source_context_id_fkey - FOREIGN KEY (source_context_id) REFERENCES a2a_context(id) ON DELETE RESTRICT; -ALTER TABLE agent_instance ADD CONSTRAINT agent_instance_context_id_fkey - FOREIGN KEY (context_id) REFERENCES a2a_context(id) ON DELETE RESTRICT; -ALTER TABLE agent_instance ADD CONSTRAINT agent_instance_source_checkpoint_id_fkey - FOREIGN KEY (source_checkpoint_id) REFERENCES agent_instance_checkpoint(id) ON DELETE RESTRICT; diff --git a/go/core/pkg/migrations/core/000019_runtime_revision_ate_api.down.sql b/go/core/pkg/migrations/core/000019_runtime_revision_ate_api.down.sql deleted file mode 100644 index f6b0863fb..000000000 --- a/go/core/pkg/migrations/core/000019_runtime_revision_ate_api.down.sql +++ /dev/null @@ -1,9 +0,0 @@ -ALTER TABLE runtime_revision - RENAME COLUMN actor_template_atespace TO actor_template_namespace; - -ALTER TABLE runtime_revision - ADD COLUMN phase TEXT NOT NULL DEFAULT 'Pending', - ADD COLUMN golden_snapshot TEXT NOT NULL DEFAULT ''; - -ALTER TABLE runtime_revision - ALTER COLUMN phase DROP DEFAULT; diff --git a/go/core/pkg/migrations/core/000019_runtime_revision_ate_api.up.sql b/go/core/pkg/migrations/core/000019_runtime_revision_ate_api.up.sql deleted file mode 100644 index d10cf6329..000000000 --- a/go/core/pkg/migrations/core/000019_runtime_revision_ate_api.up.sql +++ /dev/null @@ -1,6 +0,0 @@ -ALTER TABLE runtime_revision - RENAME COLUMN actor_template_namespace TO actor_template_atespace; - -ALTER TABLE runtime_revision - DROP COLUMN phase, - DROP COLUMN golden_snapshot; diff --git a/go/core/pkg/migrations/cross_track_test.go b/go/core/pkg/migrations/cross_track_test.go index e439cef6f..0bc5318f6 100644 --- a/go/core/pkg/migrations/cross_track_test.go +++ b/go/core/pkg/migrations/cross_track_test.go @@ -24,18 +24,33 @@ var ( createIndexRe = regexp.MustCompile(`(?i)CREATE\s+(?:UNIQUE\s+)?INDEX\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:\w+\s+)?ON\s+(\w+)`) ) -// ownedTables returns the set of table names created by up migrations in fsys. +func migrationSections(data []byte) (string, string, error) { + content := string(data) + lower := strings.ToLower(content) + upStart := strings.Index(lower, "-- +goose up") + downStart := strings.Index(lower, "-- +goose down") + if upStart < 0 || downStart < 0 || downStart <= upStart { + return "", "", fmt.Errorf("invalid Goose sections") + } + return content[upStart:downStart], content[downStart:], nil +} + +// ownedTables returns the table names from Goose Up sections. func ownedTables(fsys fs.FS) (map[string]string, error) { tables := make(map[string]string) // table name → file that created it err := fs.WalkDir(fsys, ".", func(path string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() || !strings.HasSuffix(path, ".up.sql") { + if err != nil || d.IsDir() || !strings.HasSuffix(path, ".sql") { return err } data, err := fs.ReadFile(fsys, path) if err != nil { return err } - for _, m := range createTableRe.FindAllSubmatch(data, -1) { + up, _, err := migrationSections(data) + if err != nil { + return fmt.Errorf("%s: %w", path, err) + } + for _, m := range createTableRe.FindAllStringSubmatch(up, -1) { name := strings.ToLower(string(m[1])) tables[name] = path } @@ -56,14 +71,17 @@ type violation struct { func crossTrackViolations(fsys fs.FS, foreignTables map[string]string) ([]violation, error) { var violations []violation err := fs.WalkDir(fsys, ".", func(path string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() || !strings.HasSuffix(path, ".up.sql") { + if err != nil || d.IsDir() || !strings.HasSuffix(path, ".sql") { return err } data, err := fs.ReadFile(fsys, path) if err != nil { return err } - content := string(data) + content, _, err := migrationSections(data) + if err != nil { + return fmt.Errorf("%s: %w", path, err) + } check := func(matches [][]string) { for _, m := range matches { @@ -86,83 +104,11 @@ func crossTrackViolations(fsys fs.FS, foreignTables map[string]string) ([]violat } // sqlCheck pairs a name with a regex used by the static migration checks below. -// How re is interpreted depends on the check: the guard checks capture the first -// token after a keyword and require it to be "if"; other checks match on presence. type sqlCheck struct { name string re *regexp.Regexp } -// upGuardChecks are statements in up migrations that must use IF NOT EXISTS. -var upGuardChecks = []sqlCheck{ - {"CREATE TABLE", regexp.MustCompile(`(?i)\bCREATE\s+TABLE\s+(\w+)`)}, - {"CREATE INDEX", regexp.MustCompile(`(?i)\bCREATE\s+(?:UNIQUE\s+)?INDEX\s+(?:CONCURRENTLY\s+)?(\w+)`)}, - {"CREATE EXTENSION", regexp.MustCompile(`(?i)\bCREATE\s+EXTENSION\s+(\w+)`)}, - {"ADD COLUMN", regexp.MustCompile(`(?i)\bADD\s+COLUMN\s+(\w+)`)}, -} - -// downGuardChecks are statements in down migrations that must use IF EXISTS. -var downGuardChecks = []sqlCheck{ - {"DROP TABLE", regexp.MustCompile(`(?i)\bDROP\s+TABLE\s+(\w+)`)}, - {"DROP INDEX", regexp.MustCompile(`(?i)\bDROP\s+INDEX\s+(\w+)`)}, - {"DROP EXTENSION", regexp.MustCompile(`(?i)\bDROP\s+EXTENSION\s+(\w+)`)}, - {"DROP COLUMN", regexp.MustCompile(`(?i)\bDROP\s+COLUMN\s+(\w+)`)}, -} - -// TestMigrationGuards enforces idempotency guards across all migration files: -// - Up migrations: CREATE TABLE/INDEX/EXTENSION and ADD COLUMN must use IF NOT EXISTS. -// - Down migrations: DROP TABLE/INDEX/EXTENSION/COLUMN must use IF EXISTS. -// -// This ensures migrations are safe to re-run and that the two-track rollback -// logic can call down migrations more than once without errors. -func TestMigrationGuards(t *testing.T) { - tracks := []string{"core", "vector"} - - for _, track := range tracks { - sub, err := fs.Sub(migrations.FS, track) - if err != nil { - t.Fatalf("fs.Sub(%q): %v", track, err) - } - - err = fs.WalkDir(sub, ".", func(path string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() { - return err - } - - var checks []sqlCheck - switch { - case strings.HasSuffix(path, ".up.sql"): - checks = upGuardChecks - case strings.HasSuffix(path, ".down.sql"): - checks = downGuardChecks - default: - return nil - } - - data, err := fs.ReadFile(sub, path) - if err != nil { - return err - } - content := string(data) - - for _, c := range checks { - for _, m := range c.re.FindAllStringSubmatch(content, -1) { - if !strings.EqualFold(m[1], "if") { - t.Errorf( - "missing guard in %s/%s: %q — %s requires IF NOT EXISTS / IF EXISTS", - track, path, m[0], c.name, - ) - } - } - } - return nil - }) - if err != nil { - t.Fatalf("WalkDir(%q): %v", track, err) - } - } -} - // TestNoCrossTrackDDL verifies that no migration track modifies tables owned // by another track. Each track must only ALTER or index its own tables. func TestNoCrossTrackDDL(t *testing.T) { @@ -229,10 +175,8 @@ func stripSQLComments(s string) string { // --- Schema-agnostic SQL --- // -// Migration SQL must not name a schema: the schema a migration lands in is -// chosen by the connection (search_path), not the file, so the same files apply -// into whatever schema the connection selects. See database-migrations.md, -// "Schema-agnostic SQL". Static check over every migration file — no database. +// Migration SQL must not name a schema. The connection selects the schema. +// See the Database changes section in .claude/skills/kagent-dev/SKILL.md. var schemaQualifiedChecks = []sqlCheck{ {"CREATE SCHEMA", regexp.MustCompile(`(?i)\bCREATE\s+SCHEMA\b`)}, diff --git a/go/core/pkg/migrations/runner.go b/go/core/pkg/migrations/runner.go index 6daf2c58a..50b85995c 100644 --- a/go/core/pkg/migrations/runner.go +++ b/go/core/pkg/migrations/runner.go @@ -5,85 +5,53 @@ import ( "database/sql" "errors" "fmt" + "hash/crc32" "io/fs" nurl "net/url" + "path" "regexp" "slices" + "strconv" "strings" - "github.com/golang-migrate/migrate/v4" - migratepgx "github.com/golang-migrate/migrate/v4/database/pgx/v5" - "github.com/golang-migrate/migrate/v4/source" - "github.com/golang-migrate/migrate/v4/source/iofs" _ "github.com/jackc/pgx/v5/stdlib" - ctrl "sigs.k8s.io/controller-runtime" + "github.com/pressly/goose/v3" + "github.com/pressly/goose/v3/lock" ) -var log = ctrl.Log.WithName("migrations") +const ( + advisoryLockIDSalt uint32 = 1486364155 + coreTrackingTable = "schema_migrations" + vectorTrackingTable = "vector_schema_migrations" +) + +var ( + identifierRE = regexp.MustCompile(`^[a-z_][a-z0-9_]*$`) + migrationFileRE = regexp.MustCompile(`^([0-9]+)_.+\.sql$`) +) -// Source describes one migration track for the orchestrator to apply. Downstream -// consumers register their own Sources alongside the built-in ones rather than -// owning a runner, so track ordering and failure handling stay centralized. +// Source describes one migration source. type Source struct { - // Name labels the source in logs and errors (e.g. "core", "vector"). - Name string - // Schema is the Postgres schema the track lives in. Empty means the - // connection's default schema (resolved via search_path / current_schema), - // which is what the built-in tracks use. A non-empty value scopes the - // tracking table and migration objects to that schema: the orchestrator - // creates it (CREATE SCHEMA IF NOT EXISTS) and sets search_path on the - // connection. Schema is treated as an untrusted identifier and validated. - // - // When Schema is empty (the built-in tracks), the DSN is left untouched: - // the connection keeps the server's default search_path ("$user", public), so - // migration objects and the tracking table land in public exactly as before - // this orchestrator existed. The schema handling below applies only when - // Schema is set. - // - // When Schema is set, search_path is pinned to it alone (pg_catalog is always - // implicitly searched, so built-in types and functions still resolve). public - // is NOT on the path, which keeps a schema-scoped track strictly isolated. A - // migration that needs a shared extension installed in public (e.g. the - // pgvector "vector" type) must therefore either install/relocate the extension - // into this schema or schema-qualify the reference; it cannot rely on public. - // - // Do not register two sources whose schemas resolve to the same value with the - // same TrackingTable — e.g. one source with Schema == "" and another naming the - // connection's current_schema() explicitly. They would share one tracking-table - // row and one advisory lock and corrupt each other's state. The collision unit - // is (resolved schema, TrackingTable). validateSources catches collisions on the - // literal Schema; RunUp additionally resolves "" to current_schema() and rejects - // collisions that only appear after resolution. - Schema string - // TrackingTable is the golang-migrate bookkeeping table for this track. + Name string + Schema string TrackingTable string - // FS holds the embedded migration files. - FS fs.FS - // Dir is the subdirectory within FS that holds this track's files. - Dir string - // PreCheck, if set, runs before any source is applied. A non-nil error - // aborts the whole run before any migration executes (fail-fast). - PreCheck func(url string) error + FS fs.FS + Dir string + PreCheck func(url string) error } -// BuiltinSources returns the built-in source set: the core track always, and the -// vector track when vectorEnabled. app.Start prepends these to any -// downstream-registered extra sources before calling RunUp, so the built-in -// tracks always run first and downstream consumers only supply their own extras -// (never assembling this slice themselves). A caller that invokes RunUp directly -// (e.g. a migration CLI) composes the list the same way: BuiltinSources first, -// then extras. +// BuiltinSources returns the built-in migration sources. func BuiltinSources(vectorEnabled bool) []Source { sources := []Source{{ Name: "core", - TrackingTable: "schema_migrations", + TrackingTable: coreTrackingTable, FS: FS, Dir: "core", }} if vectorEnabled { sources = append(sources, Source{ Name: "vector", - TrackingTable: "vector_schema_migrations", + TrackingTable: vectorTrackingTable, FS: FS, Dir: "vector", PreCheck: checkPgvector, @@ -92,18 +60,7 @@ func BuiltinSources(vectorEnabled bool) []Source { return sources } -// RunUp applies all pending migrations for each source, in slice order. -// -// All PreChecks run first, before any source is applied, so a failed precheck -// aborts the run before touching the database. Each source is then applied with -// the same per-track safety behavior: it tolerates a database ahead of this -// binary (compatibility mode), refuses a dirty-and-ahead database, and rolls -// itself back if its own Up fails. If a later source fails, previously-applied -// sources are rolled back to their pre-run versions in reverse order. -// -// ctx is honored at source boundaries and during schema setup and prechecks. -// golang-migrate's apply is not context-aware, so an in-flight migration is not -// cancellable. +// RunUp applies all pending migrations in source order. func RunUp(ctx context.Context, url string, sources []Source) error { if len(sources) == 0 { return nil @@ -111,86 +68,38 @@ func RunUp(ctx context.Context, url string, sources []Source) error { if err := validateSources(sources); err != nil { return err } - // Catch collisions that only appear once "" schemas resolve to current_schema(). if err := checkResolvedSchemaCollisions(ctx, url, sources); err != nil { return err } - // Run every precheck up front so a failure aborts before any source applies - // (e.g. pgvector is verified before the core track runs). for _, src := range sources { if src.PreCheck == nil { continue } if err := ctx.Err(); err != nil { - return fmt.Errorf("migrations cancelled before %s precheck: %w", src.Name, err) + return fmt.Errorf("cancel before %s precheck: %w", src.Name, err) } if err := src.PreCheck(url); err != nil { return fmt.Errorf("%s precheck: %w", src.Name, err) } } - type applied struct { - src Source - prev uint - } - var done []applied - for _, src := range sources { if err := ctx.Err(); err != nil { - return fmt.Errorf("migrations cancelled before %s: %w", src.Name, err) + return fmt.Errorf("cancel before %s migrations: %w", src.Name, err) } - prev, err := applySource(ctx, url, src) + err := WithProvider(ctx, url, src, func(provider *goose.Provider) error { + _, err := provider.Up(ctx) + return err + }) if err != nil { - // Compensating rollback: undo previously-applied sources in reverse - // order, each to its own pre-run version. The failing source has - // already rolled itself back in applySource. - // - // Run the rollback under a context detached from cancellation. If the - // caller's ctx is already canceled (e.g. SIGTERM arriving mid-startup - // as a source fails), using it here would abort the rollback at schema - // setup and leave the database mid-migration. WithoutCancel keeps any - // request-scoped values but lets best-effort cleanup finish; - // golang-migrate's own steps aren't ctx-aware regardless, so only the - // schema-setup ExecContext consults it. - rbCtx := context.WithoutCancel(ctx) - var compErrs []error - for _, a := range slices.Backward(done) { - if a.prev == 0 { - log.Info("skipping compensating rollback to version 0 to protect pre-existing data", "source", a.src.Name) - continue - } - log.Info("rolling back source after later failure", "source", a.src.Name, "targetVersion", a.prev) - if rbErr := rollbackSource(rbCtx, url, a.src, a.prev); rbErr != nil { - compErrs = append(compErrs, rbErr) - } - } - runErr := fmt.Errorf("%s migrations: %w", src.Name, err) - if len(compErrs) > 0 { - // A compensating rollback failed: the database may be left in a - // partially rolled-back state. Surface it alongside the original - // failure rather than only in the logs. - return errors.Join(append([]error{runErr}, compErrs...)...) - } - return runErr + return fmt.Errorf("%s migrations: %w", src.Name, err) } - done = append(done, applied{src: src, prev: prev}) } - return nil } -// VerifyMigrated checks, without applying or reverting anything, that every -// source's migrations have been applied to the database. It is the boot-time -// guard for the SKIP_MIGRATIONS deployment mode, where migrations run -// out-of-band (a pipeline or pre-upgrade hook) and the server must refuse to -// serve a wrong-shaped schema. It issues only SELECTs — never golang-migrate, -// which creates the tracking table on open — so it is safe on a connection -// whose role has no DDL privileges. -// -// Per source: a missing tracking table or a version behind this binary's -// embedded max is an error; a dirty tracking table is an error; a database -// ahead of the binary is tolerated (compatibility mode), matching RunUp. +// VerifyMigrated checks migration state without database writes. func VerifyMigrated(ctx context.Context, url string, sources []Source) error { if len(sources) == 0 { return nil @@ -198,134 +107,270 @@ func VerifyMigrated(ctx context.Context, url string, sources []Source) error { if err := validateSources(sources); err != nil { return err } - // Reject the same resolved-schema collisions RunUp rejects: a colliding - // source set shares one tracking table, so verification would read the - // same row twice and "pass" an unsafe configuration. if err := checkResolvedSchemaCollisions(ctx, url, sources); err != nil { return err } db, err := sql.Open("pgx", url) if err != nil { - return fmt.Errorf("open database to verify migrations: %w", err) + return fmt.Errorf("open database: %w", err) } defer db.Close() for _, src := range sources { - if err := ctx.Err(); err != nil { - return fmt.Errorf("migration verification cancelled before %s: %w", src.Name, err) + versions, err := migrationVersions(src) + if err != nil { + return fmt.Errorf("read %s migrations: %w", src.Name, err) } - maxVer, err := maxEmbeddedVersion(src.FS, src.Dir) + table := sourceTableName(src, src.TrackingTable) + exists, err := tableExists(ctx, db, table) if err != nil { - return fmt.Errorf("determine max embedded version for %s: %w", src.Name, err) + return fmt.Errorf("check %s migration table: %w", src.Name, err) + } + if !exists { + return fmt.Errorf("source %s has no migration table", src.Name) } - // For Schema == "" the unqualified name resolves via the connection's - // search_path — the same place RunUp put the table. - table := quoteIdentifier(src.TrackingTable) - if src.Schema != "" { - table = quoteIdentifier(src.Schema) + "." + table + rows, err := db.QueryContext(ctx, "SELECT version_id, is_applied FROM "+table) + if err != nil { + return fmt.Errorf("read %s migration table: %w", src.Name, err) + } + applied := make(map[int64]bool) + for rows.Next() { + var version int64 + var isApplied bool + if err := rows.Scan(&version, &isApplied); err != nil { + rows.Close() + return fmt.Errorf("scan %s migration table: %w", src.Name, err) + } + if version > 0 && isApplied { + applied[version] = true + } + } + if err := rows.Close(); err != nil { + return fmt.Errorf("close %s migration rows: %w", src.Name, err) + } + if err := rows.Err(); err != nil { + return fmt.Errorf("read %s migration rows: %w", src.Name, err) } - var exists bool - if err := db.QueryRowContext(ctx, "SELECT to_regclass($1) IS NOT NULL", table).Scan(&exists); err != nil { - return fmt.Errorf("check tracking table for %s: %w", src.Name, err) + embedded := make(map[int64]bool, len(versions)) + for _, version := range versions { + embedded[version] = true + if !applied[version] { + return fmt.Errorf("source %s needs migration version %d", src.Name, version) + } } - if !exists { - return fmt.Errorf("source %s: tracking table %s does not exist - the database has not been migrated; apply migrations out-of-band or unset SKIP_MIGRATIONS", src.Name, table) + latest := versions[len(versions)-1] + for version := range applied { + if version <= latest && !embedded[version] { + return fmt.Errorf("source %s has unknown version %d", src.Name, version) + } } + } + return nil +} - var version int64 - var dirty bool - err = db.QueryRowContext(ctx, "SELECT version, dirty FROM "+table+" LIMIT 1").Scan(&version, &dirty) - if errors.Is(err, sql.ErrNoRows) { - version, dirty = 0, false // table exists but nothing applied yet - } else if err != nil { - return fmt.Errorf("read tracking table for %s: %w", src.Name, err) +// WithProvider runs fn while one source lock is held. +func WithProvider(ctx context.Context, url string, src Source, fn func(*goose.Provider) error) (retErr error) { + if err := validateSources([]Source{src}); err != nil { + return err + } + connURL := url + if src.Schema != "" { + var err error + connURL, err = withSearchPath(url, src.Schema) + if err != nil { + return fmt.Errorf("set search path for %s: %w", src.Name, err) } + } - switch { - case dirty: - return fmt.Errorf("source %s is dirty at version %d: a previous migration attempt failed and must be resolved before starting with SKIP_MIGRATIONS", src.Name, version) - case version < int64(maxVer): - return fmt.Errorf("source %s is at version %d but this binary requires version %d: apply migrations out-of-band or unset SKIP_MIGRATIONS", src.Name, version, maxVer) - case version > int64(maxVer): - log.Info("database schema is ahead of this binary; running in compatibility mode", - "track", src.Name, "dbVersion", version, "binaryMax", maxVer) + db, err := sql.Open("pgx", connURL) + if err != nil { + return fmt.Errorf("open database for %s: %w", src.Name, err) + } + defer func() { + retErr = errors.Join(retErr, db.Close()) + }() + + if src.Schema != "" { + if _, err := db.ExecContext(ctx, "CREATE SCHEMA IF NOT EXISTS "+quoteIdentifier(src.Schema)); err != nil { + return fmt.Errorf("create schema %s: %w", src.Schema, err) } } + + var databaseName string + var schemaName sql.NullString + if err := db.QueryRowContext(ctx, "SELECT current_database(), current_schema()").Scan(&databaseName, &schemaName); err != nil { + return fmt.Errorf("resolve database identity: %w", err) + } + if !schemaName.Valid { + return errors.New("the connection has no current schema") + } + if err := rejectOldTrackingTable(ctx, db, schemaName.String, src); err != nil { + return err + } + + locker, err := lock.NewPostgresSessionLocker(lock.WithLockID(advisoryLockID(databaseName, schemaName.String, src.TrackingTable))) + if err != nil { + return fmt.Errorf("create %s migration lock: %w", src.Name, err) + } + lockConn, err := db.Conn(ctx) + if err != nil { + return fmt.Errorf("open %s lock connection: %w", src.Name, err) + } + defer func() { + retErr = errors.Join(retErr, lockConn.Close()) + }() + if err := locker.SessionLock(ctx, lockConn); err != nil { + return fmt.Errorf("lock %s migrations: %w", src.Name, err) + } + defer func() { + if err := locker.SessionUnlock(context.WithoutCancel(ctx), lockConn); err != nil { + retErr = errors.Join(retErr, fmt.Errorf("unlock %s migrations: %w", src.Name, err)) + } + }() + + sourceFS, err := fs.Sub(src.FS, src.Dir) + if err != nil { + return fmt.Errorf("open migration directory %s: %w", src.Dir, err) + } + provider, err := goose.NewProvider( + goose.DialectPostgres, + db, + sourceFS, + goose.WithTableName(src.TrackingTable), + goose.WithDisableGlobalRegistry(true), + goose.WithLogger(goose.NopLogger()), + ) + if err != nil { + return fmt.Errorf("create %s migration provider: %w", src.Name, err) + } + return fn(provider) +} + +func rejectOldTrackingTable(ctx context.Context, db *sql.DB, schema string, src Source) error { + var old bool + err := db.QueryRowContext(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_schema = $1 AND table_name = $2 AND column_name = 'dirty' + )`, schema, src.TrackingTable).Scan(&old) + if err != nil { + return fmt.Errorf("check %s migration table: %w", src.Name, err) + } + if old { + return fmt.Errorf("source %s uses an unsupported migration table. Use a new PostgreSQL database", src.Name) + } return nil } -// validateSources rejects a source set that cannot be run safely. It checks two -// things. -// -// First, required fields. Source is a public extension point, so a source with a -// missing field is a caller mistake that should fail fast with an actionable -// error rather than surface later as a vague golang-migrate failure. Name, FS, -// Dir, and TrackingTable are required; Schema is intentionally optional -// ("" = the connection's default schema) and PreCheck is optional (nil = none). -// TrackingTable has no safe default: sources can share a schema (core and vector -// both live in the connection default), so an empty TrackingTable would silently -// fall back to golang-migrate's "schema_migrations" and collide. -// -// Second, collisions on the (schema, tracking table) pair. That pair is the -// collision unit: it determines the golang-migrate version row and the -// advisory-lock id, so two such sources would fight over one row and lock -// regardless of their Dir/FS. Distinct tracking tables in the same schema are -// fine (each gets its own bookkeeping). -// -// The collision check keys on the *literal* Schema string and is intentionally -// DB-free so it stays unit-testable. It therefore cannot see a collision between -// a source with Schema == "" and one naming the connection's current_schema() -// explicitly, since "" resolves to that schema only at connection time. -// checkResolvedSchemaCollisions (called from RunUp, where a connection is -// available) closes that gap. func validateSources(sources []Source) error { seen := make(map[string]string, len(sources)) - for _, s := range sources { + for _, src := range sources { switch { - case s.Name == "": - return fmt.Errorf("migration source has empty Name") - case s.TrackingTable == "": - return fmt.Errorf("migration source %q has empty TrackingTable", s.Name) - case s.FS == nil: - return fmt.Errorf("migration source %q has nil FS", s.Name) - case s.Dir == "": - return fmt.Errorf("migration source %q has empty Dir", s.Name) - } - // Validate schema names up front so a bad name on a later source aborts - // the whole run before any earlier source applies, matching the fail-fast - // guarantee RunUp gives prechecks. newMigrate re-validates as a safety net - // for callers that bypass RunUp (e.g. applySource directly in tests). - if s.Schema != "" { - if err := validateSchemaName(s.Schema); err != nil { - return fmt.Errorf("source %q: %w", s.Name, err) + case src.Name == "": + return errors.New("migration source has an empty name") + case src.TrackingTable == "": + return fmt.Errorf("source %s has an empty tracking table", src.Name) + case src.FS == nil: + return fmt.Errorf("source %s has no migration filesystem", src.Name) + case src.Dir == "": + return fmt.Errorf("source %s has an empty migration directory", src.Name) + } + if src.Schema != "" { + if err := validateIdentifier("schema", src.Schema); err != nil { + return fmt.Errorf("source %s: %w", src.Name, err) } } - key := s.Schema + "\x00" + s.TrackingTable + if err := validateIdentifier("tracking table", src.TrackingTable); err != nil { + return fmt.Errorf("source %s: %w", src.Name, err) + } + if _, err := migrationVersions(src); err != nil { + return fmt.Errorf("source %s: %w", src.Name, err) + } + key := src.Schema + "\x00" + src.TrackingTable if other, ok := seen[key]; ok { - return fmt.Errorf("sources %q and %q share tracking table %q in schema %q", other, s.Name, s.TrackingTable, s.Schema) + return fmt.Errorf("sources %s and %s share one tracking table", other, src.Name) } - seen[key] = s.Name + seen[key] = src.Name } return nil } -// checkResolvedSchemaCollisions rejects sources that collide only after their -// schemas are resolved — a source with Schema == "" and another naming the -// connection's current_schema() explicitly both land in the same schema, so with -// the same TrackingTable they would share one golang-migrate version row and one -// advisory lock. validateSources cannot see this because it keys on the literal -// Schema; resolving "" requires a connection, which is why this lives here. -// -// The query runs only when the source set mixes empty and explicit schemas; -// all-empty (the built-in core+vector default) and all-explicit sets are already fully -// covered by validateSources' literal-key check, so the common paths pay nothing. +func migrationVersions(src Source) ([]int64, error) { + entries, err := fs.ReadDir(src.FS, src.Dir) + if err != nil { + return nil, fmt.Errorf("read migration directory %s: %w", src.Dir, err) + } + versions := make([]int64, 0, len(entries)) + seen := make(map[int64]string, len(entries)) + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".sql") { + continue + } + if strings.HasSuffix(entry.Name(), ".up.sql") || strings.HasSuffix(entry.Name(), ".down.sql") { + return nil, fmt.Errorf("invalid migration file %s", entry.Name()) + } + match := migrationFileRE.FindStringSubmatch(entry.Name()) + if match == nil { + return nil, fmt.Errorf("invalid migration file %s", entry.Name()) + } + version, err := strconv.ParseInt(match[1], 10, 64) + if err != nil || version < 1 { + return nil, fmt.Errorf("invalid migration version in %s", entry.Name()) + } + if other, ok := seen[version]; ok { + return nil, fmt.Errorf("files %s and %s use version %d", other, entry.Name(), version) + } + body, err := fs.ReadFile(src.FS, path.Join(src.Dir, entry.Name())) + if err != nil { + return nil, fmt.Errorf("read migration file %s: %w", entry.Name(), err) + } + var hasUp, hasDown bool + for line := range strings.SplitSeq(string(body), "\n") { + switch gooseAnnotation(line) { + case "no transaction": + return nil, fmt.Errorf("migration %s disables transactions", entry.Name()) + case "up": + hasUp = true + case "down": + hasDown = true + } + } + if !hasUp { + return nil, fmt.Errorf("migration %s has no Goose Up section", entry.Name()) + } + if !hasDown { + return nil, fmt.Errorf("migration %s has no Goose Down section", entry.Name()) + } + seen[version] = entry.Name() + versions = append(versions, version) + } + if len(versions) == 0 { + return nil, fmt.Errorf("directory %s has no migrations", src.Dir) + } + slices.Sort(versions) + return versions, nil +} + +func gooseAnnotation(line string) string { + if (strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t")) || + !strings.HasPrefix(strings.TrimSpace(line), "--") || !strings.Contains(line, "+goose") { + return "" + } + command := strings.ReplaceAll(line, "--", "") + command = strings.Replace(command, "+goose", "", 1) + if strings.Contains(command, "+goose") { + return "" + } + return strings.ToLower(strings.TrimSpace(command)) +} + func checkResolvedSchemaCollisions(ctx context.Context, url string, sources []Source) error { var hasDefault, hasExplicit bool - for _, s := range sources { - if s.Schema == "" { + for _, src := range sources { + if src.Schema == "" { hasDefault = true } else { hasExplicit = true @@ -337,348 +382,91 @@ func checkResolvedSchemaCollisions(ctx context.Context, url string, sources []So db, err := sql.Open("pgx", url) if err != nil { - return fmt.Errorf("open database to resolve default schema: %w", err) + return fmt.Errorf("open database to resolve schema: %w", err) } defer db.Close() - var current sql.NullString if err := db.QueryRowContext(ctx, "SELECT current_schema()").Scan(¤t); err != nil { - return fmt.Errorf("resolve default schema: %w", err) + return fmt.Errorf("resolve current schema: %w", err) } if !current.Valid { - // search_path resolves to no existing schema; "" sources have no resolved - // schema to collide on yet (their tables would fail to create later anyway). return nil } seen := make(map[string]string, len(sources)) - for _, s := range sources { - schema := s.Schema + for _, src := range sources { + schema := src.Schema if schema == "" { schema = current.String } - key := schema + "\x00" + s.TrackingTable + key := schema + "\x00" + src.TrackingTable if other, ok := seen[key]; ok { - return fmt.Errorf("sources %q and %q resolve to the same tracking table %q in schema %q (an empty Schema resolves to current_schema() = %q)", - other, s.Name, s.TrackingTable, schema, current.String) + return fmt.Errorf("sources %s and %s resolve to one tracking table", other, src.Name) } - seen[key] = s.Name + seen[key] = src.Name } return nil } -// applySource runs Up for one source and rolls it back on failure. If prevVersion -// is 0 (no migrations have ever been applied) rollback is skipped to avoid -// dropping pre-existing tables on a GORM-to-golang-migrate upgrade. It returns -// the pre-run version so the caller can compensate this source if a later one fails. -func applySource(ctx context.Context, url string, src Source) (prevVersion uint, err error) { - mg, err := newMigrate(ctx, url, src) - if err != nil { - return 0, err - } - defer closeMigrate(src.Name, mg) - - var dirty bool - prevVersion, dirty, err = mg.Version() - if err != nil && !errors.Is(err, migrate.ErrNilVersion) { - return 0, fmt.Errorf("get pre-migration version for %s: %w", src.Name, err) - } - // prevVersion == 0 when ErrNilVersion (no migrations applied yet). - - // If the database is ahead of this binary's max known version, skip Up - // entirely. The expand-then-contract policy in database-migrations.md - // guarantees that each release's code is compatible with the schema applied - // by the previous release, so rolling back one release at a time is safe. - // We cannot enforce a tighter constraint here because migration version - // numbers don't align with release versions. - // A dirty database is excluded: dirty state means a previous migration - // attempt failed and must be resolved, not silently accepted. - if maxVer, scanErr := maxEmbeddedVersion(src.FS, src.Dir); scanErr != nil { - log.Error(scanErr, "could not determine max embedded migration version; proceeding with Up", "track", src.Name) - } else if prevVersion > maxVer { - if dirty { - // DB is both dirty and ahead of this binary. Attempting Up/rollback would - // fail (the migration files for prevVersion don't exist), producing noisy - // and misleading logs. Return a clear error so operators act on the real - // problem rather than chasing rollback noise. - return prevVersion, fmt.Errorf("database is dirty at version %d and ahead of this binary's max known version %d for track %s: manual operator intervention required: %w", - prevVersion, maxVer, src.Name, migrate.ErrDirty{Version: int(prevVersion)}) - } - log.Info("database schema is ahead of this binary; running in compatibility mode", - "track", src.Name, "dbVersion", prevVersion, "binaryMax", maxVer) - return prevVersion, nil - } - - if upErr := mg.Up(); upErr != nil { - if errors.Is(upErr, migrate.ErrNoChange) { - return prevVersion, nil - } - if prevVersion == 0 { - log.Info("migration failed; skipping rollback to version 0 to protect pre-existing data", "track", src.Name) - } else { - log.Info("migration failed, attempting rollback", "track", src.Name, "targetVersion", prevVersion) - if rbErr := rollbackToVersion(mg, src.Name, prevVersion); rbErr != nil { - log.Error(rbErr, "rollback failed", "track", src.Name) - } else { - log.Info("rollback complete", "track", src.Name, "version", prevVersion) - } - } - return prevVersion, fmt.Errorf("run migrations for %s: %w", src.Name, upErr) - } - return prevVersion, nil -} - -// WithMigrator opens a migrator for src against url, runs fn against it, and -// closes it. The migrator carries the same schema handling, tracking-table -// configuration, and advisory-lock identity as the orchestrator's own runs, so -// out-of-band tooling (the `kagent db migrate` CLI) built on this serializes -// correctly against a concurrently booting server and cannot drift from the -// startup path. fn's migration operations (Up/Down/Steps/Migrate/Force) each -// take golang-migrate's per-(database, schema) advisory lock; Version reads do -// not. -func WithMigrator(ctx context.Context, url string, src Source, fn func(*migrate.Migrate) error) error { - mg, err := newMigrate(ctx, url, src) - if err != nil { - return err - } - defer closeMigrate(src.Name, mg) - return fn(mg) -} - -// rollbackSource opens a fresh migrate instance and rolls a source back to -// targetVersion. Used to compensate a previously-succeeded source when a later -// source fails. It returns an error (also logged) when the rollback fails, so -// the orchestrator can surface that the database may be left partially rolled -// back rather than leaving it as a log-only signal. -func rollbackSource(ctx context.Context, url string, src Source, targetVersion uint) error { - mg, err := newMigrate(ctx, url, src) - if err != nil { - log.Error(err, "rollback failed (open)", "track", src.Name) - return fmt.Errorf("open %s for rollback: %w", src.Name, err) - } - defer closeMigrate(src.Name, mg) - if err := rollbackToVersion(mg, src.Name, targetVersion); err != nil { - log.Error(err, "rollback failed", "track", src.Name) - return fmt.Errorf("roll back %s to version %d: %w", src.Name, targetVersion, err) - } - log.Info("rollback complete", "track", src.Name, "version", targetVersion) - return nil -} - -// rollbackToVersion rolls the migration state back to targetVersion. -// It handles the dirty-state cleanup golang-migrate requires after a failed -// Up run before down steps can be applied. -func rollbackToVersion(mg *migrate.Migrate, name string, targetVersion uint) error { - currentVersion, dirty, err := mg.Version() - if err != nil { - if errors.Is(err, migrate.ErrNilVersion) { - return nil // nothing was applied; nothing to roll back - } - return fmt.Errorf("get version after failure for %s: %w", name, err) - } - - if dirty { - // The failed migration is recorded as dirty at currentVersion. - // Force to the last clean version so Steps can run. - cleanVersion := int(currentVersion) - 1 - forceTarget := cleanVersion - if forceTarget < 1 { - forceTarget = -1 // negative tells golang-migrate to remove the version record entirely - } - if err := mg.Force(forceTarget); err != nil { - return fmt.Errorf("clear dirty state for %s: %w", name, err) - } - if forceTarget < 0 { - return nil // first migration failed and was cleared; nothing left to roll back - } - currentVersion = uint(cleanVersion) - } - - steps := int(currentVersion) - int(targetVersion) - if steps <= 0 { - return nil - } - if err := mg.Steps(-steps); err != nil && !errors.Is(err, migrate.ErrNoChange) { - return fmt.Errorf("roll back %d step(s) for %s: %w", steps, name, err) - } - return nil -} - -// checkPgvector verifies that the pgvector extension is available on the database. -// This is called before running vector migrations to fail fast with a clear error -// rather than failing mid-migration and triggering a rollback. func checkPgvector(url string) error { db, err := sql.Open("pgx", url) if err != nil { return fmt.Errorf("open database: %w", err) } defer db.Close() - var available bool - err = db.QueryRow("SELECT EXISTS(SELECT 1 FROM pg_available_extensions WHERE name = 'vector')").Scan(&available) - if err != nil { - return fmt.Errorf("check pgvector availability: %w", err) + if err := db.QueryRow("SELECT EXISTS(SELECT 1 FROM pg_available_extensions WHERE name = 'vector')").Scan(&available); err != nil { + return fmt.Errorf("check pgvector: %w", err) } if !available { - return fmt.Errorf("the pgvector extension is not installed on this PostgreSQL instance; either install pgvector or set --database-vector-enabled=false") + return errors.New("pgvector is unavailable. Install it or disable database vectors") } return nil } -// newMigrate opens a database handle (sql.Open with the pgx stdlib shim) and -// constructs a migrate.Migrate for the given source. The caller must call -// closeMigrate when done. -// -// sql.Open returns a *sql.DB pool, but the session-scoped advisory lock and the -// migration run are safe regardless: the migratepgx driver checks out a single -// dedicated *sql.Conn and pins all lock/migration work to it. The schema setup -// below (CREATE SCHEMA, and the driver's current_schema() probe) may run on any -// pooled connection, which is fine because the schema name is quoted and -// search_path is set on the DSN — so every pooled connection targets the same -// schema. The orchestrator deliberately does not cap the pool to one connection. -// -// When src.Schema is set, the connection's search_path is pinned to that schema -// (so migration DDL lands there) and the schema is created if missing. The -// tracking table is also scoped to the schema via migratepgx.Config.SchemaName. -func newMigrate(ctx context.Context, dbURL string, src Source) (*migrate.Migrate, error) { - connURL := dbURL - if src.Schema != "" { - if err := validateSchemaName(src.Schema); err != nil { - return nil, fmt.Errorf("source %q: %w", src.Name, err) - } - var err error - connURL, err = withSearchPath(dbURL, src.Schema) - if err != nil { - return nil, fmt.Errorf("set search_path for %s: %w", src.Name, err) - } - } - - db, err := sql.Open("pgx", connURL) - if err != nil { - return nil, fmt.Errorf("open database for %s: %w", src.Name, err) - } - - if src.Schema != "" { - if _, err := db.ExecContext(ctx, "CREATE SCHEMA IF NOT EXISTS "+quoteIdentifier(src.Schema)); err != nil { - _ = db.Close() - return nil, fmt.Errorf("create schema %q for %s: %w", src.Schema, src.Name, err) - } - } - - srcDriver, err := iofs.New(src.FS, src.Dir) - if err != nil { - _ = db.Close() - return nil, fmt.Errorf("load migration files from %s: %w", src.Dir, err) - } - - cfg := &migratepgx.Config{MigrationsTable: src.TrackingTable} - if src.Schema != "" { - cfg.SchemaName = src.Schema - } - driver, err := migratepgx.WithInstance(db, cfg) - if err != nil { - _ = db.Close() - return nil, fmt.Errorf("create migration driver for %s: %w", src.Name, err) - } - - mg, err := migrate.NewWithInstance("iofs", srcDriver, "postgres", driver) - if err != nil { - _ = srcDriver.Close() - _ = db.Close() - return nil, fmt.Errorf("create migrator for %s: %w", src.Name, err) - } - return mg, nil -} - -// withSearchPath returns dbURL with the search_path connection parameter set to -// schema, so every connection in the pool (including the one golang-migrate -// checks out) targets that schema for migration DDL. dbURL must be a postgres:// -// or postgresql:// URL. Other inputs are rejected: net/url parses a libpq -// keyword/value DSN or a bare "host:port/db" without error (the latter as scheme -// "host"), so requiring a known Postgres scheme is what makes this fail fast -// rather than silently rewrite a meaningless URL. func withSearchPath(dbURL, schema string) (string, error) { u, err := nurl.Parse(dbURL) if err != nil { - return "", fmt.Errorf("parse database url: %w", err) + return "", fmt.Errorf("parse database URL: %w", err) } if u.Scheme != "postgres" && u.Scheme != "postgresql" { - return "", fmt.Errorf("database url must be a postgres:// or postgresql:// DSN to scope a schema; got scheme %q", u.Scheme) + return "", fmt.Errorf("database URL has unsupported scheme %q", u.Scheme) } - q := u.Query() - q.Set("search_path", schema) - u.RawQuery = q.Encode() + query := u.Query() + query.Set("search_path", schema) + u.RawQuery = query.Encode() return u.String(), nil } -// schemaNameRe constrains a schema name to a lowercase identifier. The name is -// used both quoted (CREATE SCHEMA, the tracking table's SchemaName) and unquoted -// (the search_path connection parameter); Postgres case-folds the unquoted form, -// so a mixed-case name like "MySchema" would create the quoted schema "MySchema" -// while search_path resolved to the folded "myschema" and never matched. Keeping -// the name lowercase makes the two forms identical. -var schemaNameRe = regexp.MustCompile(`^[a-z_][a-z0-9_]*$`) - -// validateSchemaName rejects schema identifiers that are not safe to interpolate -// into DDL. Schema names come from downstream-registered Sources and cannot be -// passed as bind parameters, so they are constrained to a conservative pattern. -func validateSchemaName(schema string) error { - if len(schema) == 0 || len(schema) > 63 { - return fmt.Errorf("invalid schema name %q: must be 1-63 characters", schema) +func validateIdentifier(kind, value string) error { + if len(value) == 0 || len(value) > 63 { + return fmt.Errorf("%s %q must contain 1 to 63 characters", kind, value) } - if !schemaNameRe.MatchString(schema) { - return fmt.Errorf("invalid schema name %q: must match %s", schema, schemaNameRe.String()) + if !identifierRE.MatchString(value) { + return fmt.Errorf("%s %q must match %s", kind, value, identifierRE.String()) } return nil } -// quoteIdentifier double-quotes a SQL identifier, escaping embedded quotes. -func quoteIdentifier(id string) string { - return `"` + strings.ReplaceAll(id, `"`, `""`) + `"` +func sourceTableName(src Source, table string) string { + if src.Schema == "" { + return quoteIdentifier(table) + } + return quoteIdentifier(src.Schema) + "." + quoteIdentifier(table) } -// maxEmbeddedVersion scans dir inside migrationsFS and returns the highest migration -// version number found. Only files with a ".up.sql" suffix are considered. Version -// numbers are parsed from the leading decimal digits of each filename; the remainder -// of the name is not validated. Returns an error if the directory cannot be read or -// contains no recognisable migration files. -func maxEmbeddedVersion(migrationsFS fs.FS, dir string) (uint, error) { - entries, err := fs.ReadDir(migrationsFS, dir) - if err != nil { - return 0, fmt.Errorf("read migration dir %s: %w", dir, err) - } - var highest uint - var foundUpSQL, foundVersioned bool - for _, e := range entries { - if e.IsDir() || !strings.HasSuffix(e.Name(), ".up.sql") { - continue - } - foundUpSQL = true - m, parseErr := source.DefaultParse(e.Name()) - if parseErr != nil || m.Direction != source.Up { - continue - } - foundVersioned = true - if m.Version > highest { - highest = m.Version - } - } - if !foundUpSQL { - return 0, fmt.Errorf("no .up.sql migration files found in %s", dir) - } - if !foundVersioned { - return 0, fmt.Errorf("no versioned .up.sql migration files found in %s; expected names like 000001_description.up.sql", dir) - } - return highest, nil +func tableExists(ctx context.Context, db *sql.DB, table string) (bool, error) { + var exists bool + err := db.QueryRowContext(ctx, "SELECT to_regclass($1) IS NOT NULL", table).Scan(&exists) + return exists, err } -// closeMigrate closes mg, logging source and database close errors separately. -func closeMigrate(name string, mg *migrate.Migrate) { - srcErr, dbErr := mg.Close() - if srcErr != nil { - log.Error(srcErr, "closing migration source", "track", name) - } - if dbErr != nil { - log.Error(dbErr, "closing migration database", "track", name) - } +func quoteIdentifier(value string) string { + return `"` + strings.ReplaceAll(value, `"`, `""`) + `"` +} + +func advisoryLockID(database, schema, table string) int64 { + name := strings.Join([]string{schema, table, database}, "\x00") + sum := crc32.ChecksumIEEE([]byte(name)) + return int64(sum * advisoryLockIDSalt) } diff --git a/go/core/pkg/migrations/runner_test.go b/go/core/pkg/migrations/runner_test.go index f72ba30cd..01c616059 100644 --- a/go/core/pkg/migrations/runner_test.go +++ b/go/core/pkg/migrations/runner_test.go @@ -4,124 +4,50 @@ import ( "context" "database/sql" "errors" - "fmt" - "maps" - "net/url" + "slices" "strings" "testing" "testing/fstest" "time" - "github.com/golang-migrate/migrate/v4" _ "github.com/jackc/pgx/v5/stdlib" + "github.com/pressly/goose/v3" testcontainers "github.com/testcontainers/testcontainers-go" tcpostgres "github.com/testcontainers/testcontainers-go/modules/postgres" "github.com/testcontainers/testcontainers-go/wait" ) -// --- migration fixtures --- - -// goodCoreFS has two valid core migrations. -var goodCoreFS = fstest.MapFS{ - "core/000001_create.up.sql": {Data: []byte(`CREATE TABLE mig_test (id SERIAL PRIMARY KEY);`)}, - "core/000001_create.down.sql": {Data: []byte(`DROP TABLE IF EXISTS mig_test;`)}, - "core/000002_alter.up.sql": {Data: []byte(`ALTER TABLE mig_test ADD COLUMN name TEXT;`)}, - "core/000002_alter.down.sql": {Data: []byte(`ALTER TABLE mig_test DROP COLUMN IF EXISTS name;`)}, -} - -// oneCoreFS is just the first migration from goodCoreFS. -var oneCoreFS = fstest.MapFS{ - "core/000001_create.up.sql": {Data: []byte(`CREATE TABLE mig_test (id SERIAL PRIMARY KEY);`)}, - "core/000001_create.down.sql": {Data: []byte(`DROP TABLE IF EXISTS mig_test;`)}, -} - -// failOnFirstCoreFS fails immediately on the first migration. -var failOnFirstCoreFS = fstest.MapFS{ - "core/000001_bad.up.sql": {Data: []byte(`ALTER TABLE no_such_table ADD COLUMN x TEXT;`)}, - "core/000001_bad.down.sql": {Data: []byte(`SELECT 1;`)}, -} - -// failOnSecondCoreFS succeeds on migration 1 then fails on migration 2. -var failOnSecondCoreFS = fstest.MapFS{ - "core/000001_create.up.sql": {Data: []byte(`CREATE TABLE mig_test (id SERIAL PRIMARY KEY);`)}, - "core/000001_create.down.sql": {Data: []byte(`DROP TABLE IF EXISTS mig_test;`)}, - "core/000002_bad.up.sql": {Data: []byte(`ALTER TABLE no_such_table ADD COLUMN x TEXT;`)}, - "core/000002_bad.down.sql": {Data: []byte(`SELECT 1;`)}, -} - -// failVectorFS has a vector migration that fails. -var failVectorFS = fstest.MapFS{ - "vector/000001_bad.up.sql": {Data: []byte(`ALTER TABLE no_such_table ADD COLUMN y TEXT;`)}, - "vector/000001_bad.down.sql": {Data: []byte(`SELECT 1;`)}, -} - -// expandCoreFS creates shared_data with two columns. Used to test cross-track -// rollback scenarios where the vector track depends on this table. -var expandCoreFS = fstest.MapFS{ - "core/000001_create_shared.up.sql": {Data: []byte(`CREATE TABLE IF NOT EXISTS shared_data (id SERIAL PRIMARY KEY, col_a TEXT);`)}, - "core/000001_create_shared.down.sql": {Data: []byte(`DROP TABLE IF EXISTS shared_data;`)}, - "core/000002_add_col_b.up.sql": {Data: []byte(`ALTER TABLE shared_data ADD COLUMN IF NOT EXISTS col_b TEXT;`)}, - "core/000002_add_col_b.down.sql": {Data: []byte(`ALTER TABLE shared_data DROP COLUMN IF EXISTS col_b;`)}, -} - -// failVectorWithDependencyFS is a vector migration that partially succeeds -// (adds a column to shared_data) then fails. Its down migration uses IF EXISTS -// so rollback is safe even if the column was never added. -var failVectorWithDependencyFS = fstest.MapFS{ - "vector/000001_bad_depends_on_core.up.sql": {Data: []byte(`ALTER TABLE shared_data ADD COLUMN IF NOT EXISTS vec_col VECTOR(3); ALTER TABLE no_such_table ADD COLUMN x TEXT;`)}, - "vector/000001_bad_depends_on_core.down.sql": {Data: []byte(`ALTER TABLE shared_data DROP COLUMN IF EXISTS vec_col;`)}, -} - -// goodVectorFS has a valid vector migration. -var goodVectorFS = fstest.MapFS{ - "vector/000001_create.up.sql": {Data: []byte(`CREATE EXTENSION IF NOT EXISTS vector; CREATE TABLE IF NOT EXISTS vec_test (id SERIAL PRIMARY KEY, embedding vector(3));`)}, - "vector/000001_create.down.sql": {Data: []byte(`DROP TABLE IF EXISTS vec_test; DROP EXTENSION IF EXISTS vector;`)}, -} - -// mergeFS combines multiple MapFS values into one. -func mergeFS(fsMaps ...fstest.MapFS) fstest.MapFS { - out := fstest.MapFS{} - for _, m := range fsMaps { - maps.Copy(out, m) - } - return out +var twoMigrationFS = fstest.MapFS{ + "migrations/000001_create.sql": {Data: migrationSQL( + "CREATE TABLE migration_test (id bigint PRIMARY KEY);", + "DROP TABLE IF EXISTS migration_test;", + )}, + "migrations/000002_alter.sql": {Data: migrationSQL( + "ALTER TABLE migration_test ADD COLUMN name text;", + "ALTER TABLE migration_test DROP COLUMN IF EXISTS name;", + )}, } -// coreSource builds a core Source backed by fsys (subdir "core"). -func coreSource(fsys fstest.MapFS) Source { - return Source{Name: "core", TrackingTable: "schema_migrations", FS: fsys, Dir: "core"} +func migrationSQL(up, down string) []byte { + return []byte("-- +goose Up\n" + up + "\n\n-- +goose Down\n" + down + "\n") } -// vectorSource builds a vector Source backed by fsys (subdir "vector"), with the -// pgvector precheck wired in to mirror BuiltinSources. -func vectorSource(fsys fstest.MapFS) Source { - return Source{Name: "vector", TrackingTable: "vector_schema_migrations", FS: fsys, Dir: "vector", PreCheck: checkPgvector} -} - -// trackVersion reads the current version from a golang-migrate tracking table. -// Returns 0 if the table is empty or does not exist (fully rolled back). -func trackVersion(t *testing.T, connStr, table string) uint { - t.Helper() - db, err := sql.Open("pgx", connStr) - if err != nil { - t.Fatalf("trackVersion: open db: %v", err) +func testSource(fsys fstest.MapFS) Source { + return Source{ + Name: "test", + TrackingTable: "test_schema_migrations", + FS: fsys, + Dir: "migrations", } - defer db.Close() - var v uint - err = db.QueryRowContext(context.Background(), - fmt.Sprintf(`SELECT version FROM %s LIMIT 1`, table)).Scan(&v) - if err != nil { - return 0 // sql.ErrNoRows or table doesn't exist - } - return v } -// startTestDB spins up a pgvector Postgres container and returns its connection -// string, registering cleanup with t. It does not run any migrations. func startTestDB(t *testing.T) string { t.Helper() + if testing.Short() { + t.Skip("skip the PostgreSQL test in short mode") + } ctx := context.Background() - pgContainer, err := tcpostgres.Run(ctx, + container, err := tcpostgres.Run(ctx, "pgvector/pgvector:pg18-trixie", tcpostgres.WithDatabase("kagent_test"), tcpostgres.WithUsername("postgres"), @@ -133,956 +59,425 @@ func startTestDB(t *testing.T) string { ), ) if err != nil { - t.Fatalf("startTestDB: start container: %v", err) + t.Fatalf("start PostgreSQL: %v", err) } t.Cleanup(func() { - if err := pgContainer.Terminate(ctx); err != nil { - t.Logf("warning: failed to terminate postgres container: %v", err) + if err := container.Terminate(ctx); err != nil { + t.Logf("terminate PostgreSQL: %v", err) } }) - connStr, err := pgContainer.ConnectionString(ctx, "sslmode=disable") + dsn, err := container.ConnectionString(ctx, "sslmode=disable") if err != nil { - t.Fatalf("startTestDB: connection string: %v", err) + t.Fatalf("get PostgreSQL URL: %v", err) } - return connStr + return dsn } -// startTestDBWithoutPgvector spins up a plain Postgres container (no pgvector) -// and returns its connection string, registering cleanup with t. -func startTestDBWithoutPgvector(t *testing.T) string { +func execSQL(t *testing.T, dsn, statement string, args ...any) { t.Helper() - ctx := context.Background() - pgContainer, err := tcpostgres.Run(ctx, - "postgres:18", - tcpostgres.WithDatabase("kagent_test"), - tcpostgres.WithUsername("postgres"), - tcpostgres.WithPassword("kagent"), - testcontainers.WithWaitStrategy( - wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2). - WithStartupTimeout(60*time.Second), - ), - ) + db, err := sql.Open("pgx", dsn) if err != nil { - t.Fatalf("startTestDBWithoutPgvector: start container: %v", err) + t.Fatal(err) } - t.Cleanup(func() { - if err := pgContainer.Terminate(ctx); err != nil { - t.Logf("warning: failed to terminate postgres container: %v", err) - } - }) - connStr, err := pgContainer.ConnectionString(ctx, "sslmode=disable") - if err != nil { - t.Fatalf("startTestDBWithoutPgvector: connection string: %v", err) + defer db.Close() + if _, err := db.ExecContext(context.Background(), statement, args...); err != nil { + t.Fatalf("execute SQL: %v", err) } - return connStr } -// tableExists checks whether a table exists in the public schema. -func tableExists(t *testing.T, connStr, table string) bool { +func testTableExists(t *testing.T, dsn, table string) bool { t.Helper() - return tableExistsInSchema(t, connStr, "public", table) -} - -// tableExistsInSchema checks whether a table exists in the given schema. -func tableExistsInSchema(t *testing.T, connStr, schema, table string) bool { - t.Helper() - db, err := sql.Open("pgx", connStr) + db, err := sql.Open("pgx", dsn) if err != nil { - t.Fatalf("tableExistsInSchema: open db: %v", err) + t.Fatal(err) } defer db.Close() var exists bool - err = db.QueryRowContext(context.Background(), - "SELECT EXISTS(SELECT 1 FROM information_schema.tables WHERE table_schema = $1 AND table_name = $2)", - schema, table).Scan(&exists) - if err != nil { - t.Fatalf("tableExistsInSchema: query: %v", err) + if err := db.QueryRowContext(context.Background(), "SELECT to_regclass($1) IS NOT NULL", table).Scan(&exists); err != nil { + t.Fatal(err) } return exists } -// --- applySource tests --- - -func TestApplySource_HappyPath(t *testing.T) { - connStr := startTestDB(t) - - prev, err := applySource(context.Background(), connStr, coreSource(goodCoreFS)) +func testExtensionExists(t *testing.T, dsn, extension string) bool { + t.Helper() + db, err := sql.Open("pgx", dsn) if err != nil { - t.Fatalf("applySource: %v", err) + t.Fatal(err) } - if prev != 0 { - t.Errorf("prevVersion = %d, want 0", prev) - } - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Errorf("version = %d, want 2", got) + defer db.Close() + var exists bool + if err := db.QueryRowContext(context.Background(), "SELECT EXISTS(SELECT 1 FROM pg_extension WHERE extname = $1)", extension).Scan(&exists); err != nil { + t.Fatal(err) } + return exists } -func TestApplySource_NoOpWhenAlreadyAtLatest(t *testing.T) { - connStr := startTestDB(t) - - if _, err := applySource(context.Background(), connStr, coreSource(goodCoreFS)); err != nil { - t.Fatalf("first apply: %v", err) - } - prev, err := applySource(context.Background(), connStr, coreSource(goodCoreFS)) +func testColumnExists(t *testing.T, dsn, table, column string) bool { + t.Helper() + db, err := sql.Open("pgx", dsn) if err != nil { - t.Fatalf("second apply: %v", err) - } - if prev != 2 { - t.Errorf("prevVersion on no-op = %d, want 2", prev) - } - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Errorf("version = %d, want 2", got) - } -} - -func TestApplySource_NoRollbackWhenFirstMigrationFails(t *testing.T) { - connStr := startTestDB(t) - - if _, err := applySource(context.Background(), connStr, coreSource(failOnFirstCoreFS)); err == nil { - t.Fatal("expected error, got nil") - } - // prevVersion was 0 so rollback is skipped to protect pre-existing data. - // golang-migrate marks version 1 as dirty (the failed migration). - if got := trackVersion(t, connStr, "schema_migrations"); got != 1 { - t.Errorf("version after failure = %d, want 1 (dirty, rollback skipped)", got) - } -} - -func TestApplySource_NoRollbackWhenLaterMigrationFails(t *testing.T) { - connStr := startTestDB(t) - - if _, err := applySource(context.Background(), connStr, coreSource(failOnSecondCoreFS)); err == nil { - t.Fatal("expected error, got nil") - } - // Migration 1 succeeded, migration 2 failed. Rollback is skipped because - // prevVersion was 0. golang-migrate marks version 2 as dirty. - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Errorf("version after failure = %d, want 2 (dirty, rollback skipped)", got) - } -} - -func TestApplySource_RollsBackToExistingVersion(t *testing.T) { - connStr := startTestDB(t) - - // Establish a baseline at version 1. - if _, err := applySource(context.Background(), connStr, coreSource(oneCoreFS)); err != nil { - t.Fatalf("setup: %v", err) + t.Fatal(err) } - - // Advance to version 2 — should fail and roll back to version 1, not 0. - if _, err := applySource(context.Background(), connStr, coreSource(failOnSecondCoreFS)); err == nil { - t.Fatal("expected error, got nil") - } - if got := trackVersion(t, connStr, "schema_migrations"); got != 1 { - t.Errorf("version after rollback = %d, want 1 (pre-run baseline)", got) - } -} - -// TestApplySource_RollsBackWithExistingVersion verifies that when migrations have -// previously been applied (prevVersion > 0), rollback always happens on failure. -// This ensures the rollback protection only affects the initial migration run -// (prevVersion == 0), not subsequent upgrades. -func TestApplySource_RollsBackWithExistingVersion(t *testing.T) { - connStr := startTestDB(t) - - // Establish a baseline at version 1. - if _, err := applySource(context.Background(), connStr, coreSource(oneCoreFS)); err != nil { - t.Fatalf("setup: %v", err) - } - - // Verify data exists at version 1. - if got := trackVersion(t, connStr, "schema_migrations"); got != 1 { - t.Fatalf("setup: version = %d, want 1", got) - } - - // Advance to version 2 — should roll back because prevVersion > 0. - if _, err := applySource(context.Background(), connStr, coreSource(failOnSecondCoreFS)); err == nil { - t.Fatal("expected error, got nil") - } - if got := trackVersion(t, connStr, "schema_migrations"); got != 1 { - t.Errorf("version after rollback = %d, want 1 (rollback should happen when prevVersion > 0)", got) - } -} - -func TestMaxEmbeddedVersion(t *testing.T) { - tests := []struct { - name string - fs fstest.MapFS - dir string - want uint - wantErr bool - }{ - { - name: "returns highest version from up.sql files", - fs: fstest.MapFS{ - "core/000001_a.up.sql": {}, - "core/000001_a.down.sql": {}, - "core/000003_b.up.sql": {}, - "core/000003_b.down.sql": {}, - }, - dir: "core", - want: 3, - }, - { - name: "ignores non-sql files", - fs: fstest.MapFS{ - "core/README.md": {}, - "core/000002_x.up.sql": {}, - "core/000002_x.down.sql": {}, - }, - dir: "core", - want: 2, - }, - { - name: "ignores down.sql when computing max", - fs: fstest.MapFS{ - "core/000001_a.up.sql": {}, - "core/000002_b.down.sql": {}, - }, - dir: "core", - want: 1, - }, - { - name: "up.sql files with unparseable names returns error", - fs: fstest.MapFS{"core/init.up.sql": {}}, - dir: "core", - wantErr: true, - }, - { - name: "no underscore returns error, matches golang-migrate", - fs: fstest.MapFS{"core/000003foo.up.sql": {}}, - dir: "core", - wantErr: true, - }, - { - name: "empty dir returns error", - fs: fstest.MapFS{"core/.keep": {}}, - dir: "core", - wantErr: true, - }, - { - name: "nonexistent dir returns error", - fs: fstest.MapFS{}, - dir: "missing", - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := maxEmbeddedVersion(tt.fs, tt.dir) - if (err != nil) != tt.wantErr { - t.Errorf("maxEmbeddedVersion() error = %v, wantErr %v", err, tt.wantErr) - } - if !tt.wantErr && got != tt.want { - t.Errorf("maxEmbeddedVersion() = %d, want %d", got, tt.want) - } - }) - } -} - -// TestApplySource_SucceedsWhenDBVersionAhead verifies that an older binary starting against -// a database that a newer binary has migrated does not crash-loop. It skips Up entirely -// and returns success, leaving the schema unchanged. Safe rollback relies on the -// expand-then-contract discipline in database-migrations.md and rolling back one release -// at a time. -func TestApplySource_SucceedsWhenDBVersionAhead(t *testing.T) { - connStr := startTestDB(t) - - // Newer binary applies v1 and v2. - if _, err := applySource(context.Background(), connStr, coreSource(goodCoreFS)); err != nil { - t.Fatalf("newer binary apply: %v", err) - } - - // Older binary (max v1) starts against the v2 schema — must not error. - if _, err := applySource(context.Background(), connStr, coreSource(oneCoreFS)); err != nil { - t.Fatalf("older binary apply against newer schema: %v", err) - } - - // Schema version must be unchanged — the older binary has no business rolling back. - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Errorf("version = %d, want 2 (older binary must not modify schema version)", got) + defer db.Close() + var exists bool + err = db.QueryRowContext(context.Background(), ` + SELECT EXISTS( + SELECT 1 FROM information_schema.columns + WHERE table_schema = current_schema() AND table_name = $1 AND column_name = $2 + )`, table, column).Scan(&exists) + if err != nil { + t.Fatal(err) } + return exists } -// TestApplySource_DirtyStateNotMaskedByCompatibilityMode verifies that a dirty database -// is not silently accepted by compatibility mode. If the DB is both dirty and ahead of -// the binary's max known version, the dirty state must still be surfaced as an error. -func TestApplySource_DirtyStateNotMaskedByCompatibilityMode(t *testing.T) { - connStr := startTestDB(t) - - // Apply v1 cleanly first so the tracking table exists. - if _, err := applySource(context.Background(), connStr, coreSource(oneCoreFS)); err != nil { - t.Fatalf("setup: %v", err) - } - - // Simulate a newer binary having applied v2 but leaving it dirty. - db, err := sql.Open("pgx", connStr) +func testVersions(t *testing.T, dsn, table string) []int64 { + t.Helper() + db, err := sql.Open("pgx", dsn) if err != nil { - t.Fatalf("open db: %v", err) + t.Fatal(err) } defer db.Close() - if _, err := db.Exec("UPDATE schema_migrations SET version = 2, dirty = true"); err != nil { - t.Fatalf("set dirty state: %v", err) - } - - // Older binary (max v1) starts: DB is at v2 dirty. Compatibility mode must NOT - // trigger — dirty state must be returned as an error so the operator can act. - _, err = applySource(context.Background(), connStr, coreSource(oneCoreFS)) - if err == nil { - t.Fatal("expected error for dirty database, got nil") - } - var dirtyErr migrate.ErrDirty - if !errors.As(err, &dirtyErr) { - t.Errorf("expected migrate.ErrDirty, got %T: %v", err, err) - } -} - -// --- rollbackSource tests --- - -func TestRollbackSource_RollsBackToTarget(t *testing.T) { - connStr := startTestDB(t) - - if _, err := applySource(context.Background(), connStr, coreSource(goodCoreFS)); err != nil { - t.Fatalf("setup: %v", err) - } - - rollbackSource(context.Background(), connStr, coreSource(goodCoreFS), 0) - - if got := trackVersion(t, connStr, "schema_migrations"); got != 0 { - t.Errorf("version after rollback = %d, want 0", got) - } -} - -func TestRollbackSource_PartialRollback(t *testing.T) { - connStr := startTestDB(t) - - if _, err := applySource(context.Background(), connStr, coreSource(goodCoreFS)); err != nil { - t.Fatalf("setup: %v", err) - } - - // Roll back only one step (2 → 1). - rollbackSource(context.Background(), connStr, coreSource(goodCoreFS), 1) - - if got := trackVersion(t, connStr, "schema_migrations"); got != 1 { - t.Errorf("version after partial rollback = %d, want 1", got) - } -} - -// --- cross-track rollback (per-source primitives) --- - -// TestCrossTrackRollback_CoreUnchangedWhenVectorFails covers the case where -// core has no new migrations (ErrNoChange) and vector fails. Core should not -// be downgraded by the cross-track rollback. -func TestCrossTrackRollback_CoreUnchangedWhenVectorFails(t *testing.T) { - connStr := startTestDB(t) - - combined := mergeFS(goodCoreFS, failVectorFS) - - // Establish core at its latest version before the run. - if _, err := applySource(context.Background(), connStr, coreSource(combined)); err != nil { - t.Fatalf("setup core: %v", err) - } - - // Core has no new migrations — applySource returns ErrNoChange. - corePrev, err := applySource(context.Background(), connStr, coreSource(combined)) + rows, err := db.QueryContext(context.Background(), "SELECT version_id FROM "+quoteIdentifier(table)+" WHERE is_applied ORDER BY version_id") if err != nil { - t.Fatalf("core apply (no-op): %v", err) + t.Fatal(err) } - if corePrev != 2 { - t.Fatalf("corePrev = %d, want 2", corePrev) - } - - // Vector fails and self-rolls-back. - if _, err := applySource(context.Background(), connStr, vectorSource(combined)); err == nil { - t.Fatal("expected vector error, got nil") + defer rows.Close() + var versions []int64 + for rows.Next() { + var version int64 + if err := rows.Scan(&version); err != nil { + t.Fatal(err) + } + versions = append(versions, version) } - - // Cross-track rollback: core should be untouched since corePrev == current version. - rollbackSource(context.Background(), connStr, coreSource(combined), corePrev) - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Errorf("core version = %d, want 2 (should not have been downgraded)", got) + if err := rows.Err(); err != nil { + t.Fatal(err) } + return versions } -func TestCrossTrackRollback_CoreRolledBackWhenVectorFails(t *testing.T) { - connStr := startTestDB(t) - - combined := mergeFS(goodCoreFS, failVectorFS) +func TestRunUpAndDown(t *testing.T) { + dsn := startTestDB(t) + source := testSource(twoMigrationFS) - // Core succeeds. - corePrev, err := applySource(context.Background(), connStr, coreSource(combined)) - if err != nil { - t.Fatalf("core apply: %v", err) - } - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Fatalf("core version = %d, want 2", got) - } - - // Vector fails. Self-rollback is skipped because vector prevVersion is 0. - if _, err := applySource(context.Background(), connStr, vectorSource(combined)); err == nil { - t.Fatal("expected vector error, got nil") - } - if got := trackVersion(t, connStr, "vector_schema_migrations"); got != 1 { - t.Errorf("vector version after failure = %d, want 1 (dirty, rollback skipped)", got) - } - - // Cross-track rollback: core should be rolled back to its pre-run version. - rollbackSource(context.Background(), connStr, coreSource(combined), corePrev) - if got := trackVersion(t, connStr, "schema_migrations"); got != corePrev { - t.Errorf("core version after cross-track rollback = %d, want %d", got, corePrev) + if err := RunUp(context.Background(), dsn, []Source{source}); err != nil { + t.Fatalf("RunUp: %v", err) } -} - -// TestCrossTrackRollback_IfExistsGuardsSafeOnVectorFailure verifies that when a -// vector migration fails and triggers a core cross-track rollback, the IF EXISTS -// guards in both down migrations prevent errors even though the vector migration -// only partially applied and shared_data is being dropped by core's rollback. -func TestCrossTrackRollback_IfExistsGuardsSafeOnVectorFailure(t *testing.T) { - connStr := startTestDB(t) - - combined := mergeFS(expandCoreFS, failVectorWithDependencyFS) - - // Core succeeds (shared_data created with col_a and col_b). - corePrev, err := applySource(context.Background(), connStr, coreSource(combined)) - if err != nil { - t.Fatalf("core apply: %v", err) + if !testTableExists(t, dsn, "migration_test") { + t.Fatal("migration_test does not exist") } - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Fatalf("core version = %d, want 2", got) + if !testColumnExists(t, dsn, "migration_test", "name") { + t.Fatal("migration_test.name does not exist") } - - // Vector fails. Self-rollback is skipped because vector prevVersion is 0. - if _, err := applySource(context.Background(), connStr, vectorSource(combined)); err == nil { - t.Fatal("expected vector error, got nil") + if got := testVersions(t, dsn, source.TrackingTable); !slices.Equal(got, []int64{0, 1, 2}) { + t.Fatalf("versions = %v", got) } - if got := trackVersion(t, connStr, "vector_schema_migrations"); got != 1 { - t.Errorf("vector version after failure = %d, want 1 (dirty, rollback skipped)", got) + if err := VerifyMigrated(context.Background(), dsn, []Source{source}); err != nil { + t.Fatalf("VerifyMigrated: %v", err) } - // Cross-track rollback: core rolls back to its pre-run version. - rollbackSource(context.Background(), connStr, coreSource(combined), corePrev) - if got := trackVersion(t, connStr, "schema_migrations"); got != corePrev { - t.Errorf("core version after cross-track rollback = %d, want %d", got, corePrev) + if err := WithProvider(context.Background(), dsn, source, func(provider *goose.Provider) error { + _, err := provider.DownTo(context.Background(), 0) + return err + }); err != nil { + t.Fatalf("DownTo: %v", err) } -} - -// --- checkPgvector tests --- - -func TestCheckPgvector_SucceedsOnPgvectorDB(t *testing.T) { - connStr := startTestDB(t) // pgvector image - if err := checkPgvector(connStr); err != nil { - t.Errorf("checkPgvector on pgvector db: %v", err) + if testTableExists(t, dsn, "migration_test") { + t.Fatal("migration_test still exists") } -} - -func TestCheckPgvector_FailsOnPlainPostgres(t *testing.T) { - connStr := startTestDBWithoutPgvector(t) // plain postgres image - if err := checkPgvector(connStr); err == nil { - t.Error("checkPgvector on plain postgres: expected error, got nil") + if got := testVersions(t, dsn, source.TrackingTable); !slices.Equal(got, []int64{0}) { + t.Fatalf("versions after down = %v", got) } } -// --- RunUp end-to-end tests --- - -func TestRunUp_CoreAndVector(t *testing.T) { - connStr := startTestDB(t) - combined := mergeFS(goodCoreFS, goodVectorFS) +func TestBuiltinMigrationsRoundTrip(t *testing.T) { + dsn := startTestDB(t) + sources := BuiltinSources(true) - if err := RunUp(context.Background(), connStr, []Source{coreSource(combined), vectorSource(combined)}); err != nil { - t.Fatalf("RunUp: %v", err) + if err := RunUp(context.Background(), dsn, sources); err != nil { + t.Fatalf("initial RunUp: %v", err) } - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Errorf("core version = %d, want 2", got) + if err := VerifyMigrated(context.Background(), dsn, sources); err != nil { + t.Fatalf("initial VerifyMigrated: %v", err) } - if got := trackVersion(t, connStr, "vector_schema_migrations"); got != 1 { - t.Errorf("vector version = %d, want 1", got) + // A context may outlive one AgentInstance, so these IDs intentionally differ. + contextID := "00000000-0000-0000-0000-000000000001" + instanceID := "00000000-0000-0000-0000-000000000002" + execSQL(t, dsn, "INSERT INTO a2a_context (id, namespace, user_id) VALUES ($1, 'test', 'user')", contextID) + execSQL(t, dsn, "INSERT INTO agent_instance (id, namespace, user_id, request_id, state, data, context_id) VALUES ($1, 'test', 'user', 'request', 'READY', $2, $3)", instanceID, []byte{}, contextID) + execSQL(t, dsn, "INSERT INTO agent_instance_task (context_id, id, state, data) VALUES ($1, 'task', 'TASK_STATE_INPUT_REQUIRED', $2)", contextID, []byte{}) + for _, source := range slices.Backward(sources) { + if err := WithProvider(context.Background(), dsn, source, func(provider *goose.Provider) error { + _, err := provider.DownTo(context.Background(), 0) + return err + }); err != nil { + t.Fatalf("down %s: %v", source.Name, err) + } } -} - -func TestRunUp_CoreOnlyWhenVectorDisabled(t *testing.T) { - connStr := startTestDB(t) - combined := mergeFS(goodCoreFS, goodVectorFS) - - if err := RunUp(context.Background(), connStr, []Source{coreSource(combined)}); err != nil { - t.Fatalf("RunUp: %v", err) + if !testExtensionExists(t, dsn, "vector") { + t.Fatal("the vector down migration removed the shared extension") } - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Errorf("core version = %d, want 2", got) + if err := RunUp(context.Background(), dsn, sources); err != nil { + t.Fatalf("second RunUp: %v", err) } - // Vector tracking table should not exist. - if tableExists(t, connStr, "vector_schema_migrations") { - t.Error("vector_schema_migrations should not exist when vector source is not registered") + if err := VerifyMigrated(context.Background(), dsn, sources); err != nil { + t.Fatalf("second VerifyMigrated: %v", err) } } -func TestRunUp_FailsBeforeMigrationsWhenPgvectorMissing(t *testing.T) { - connStr := startTestDBWithoutPgvector(t) +func TestMigrationIsAtomic(t *testing.T) { + dsn := startTestDB(t) + source := testSource(fstest.MapFS{ + "migrations/000001_fail.sql": {Data: migrationSQL( + "CREATE TABLE partial_result (id bigint); ALTER TABLE missing_table ADD COLUMN value text;", + "DROP TABLE IF EXISTS partial_result;", + )}, + }) - err := RunUp(context.Background(), connStr, []Source{coreSource(goodCoreFS), vectorSource(goodCoreFS)}) + err := RunUp(context.Background(), dsn, []Source{source}) if err == nil { - t.Fatal("expected error, got nil") + t.Fatal("RunUp succeeded") } - // Core migrations should NOT have run — no tracking table created. The vector - // precheck runs up front, before any source is applied. - if tableExists(t, connStr, "schema_migrations") { - t.Error("schema_migrations should not exist — pgvector check should fail before any migrations") + if testTableExists(t, dsn, "partial_result") { + t.Fatal("the failed migration left partial_result") } -} - -// TestRunUp_SkipsCoreRollbackWhenVectorFailsOnFirstRun verifies the cross-track -// rollback protection in RunUp: when vector fails and corePrev is 0 (initial run), -// core is not rolled back to protect pre-existing data. -func TestRunUp_SkipsCoreRollbackWhenVectorFailsOnFirstRun(t *testing.T) { - connStr := startTestDB(t) // pgvector available so checkPgvector passes - combined := mergeFS(goodCoreFS, failVectorFS) - - err := RunUp(context.Background(), connStr, []Source{coreSource(combined), vectorSource(combined)}) - if err == nil { - t.Fatal("expected error, got nil") - } - // Core should still be at version 2 — not rolled back to 0. - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Errorf("core version = %d, want 2 (should not be rolled back when corePrev == 0)", got) + if got := testVersions(t, dsn, source.TrackingTable); !slices.Equal(got, []int64{0}) { + t.Fatalf("versions = %v", got) } } -// TestRunUp_MultiSourceOrdering verifies sources apply in slice order: source "b" -// references a table created by source "a", so it can only succeed if "a" ran first. -func TestRunUp_MultiSourceOrdering(t *testing.T) { - connStr := startTestDB(t) - - ordFS := fstest.MapFS{ - "a/000001_a.up.sql": {Data: []byte(`CREATE TABLE ord_a (id INT PRIMARY KEY);`)}, - "a/000001_a.down.sql": {Data: []byte(`DROP TABLE IF EXISTS ord_a;`)}, - "b/000001_b.up.sql": {Data: []byte(`CREATE TABLE ord_b (id INT PRIMARY KEY, a_id INT REFERENCES ord_a(id));`)}, - "b/000001_b.down.sql": {Data: []byte(`DROP TABLE IF EXISTS ord_b;`)}, - "c/000001_c.up.sql": {Data: []byte(`CREATE TABLE ord_c (id INT PRIMARY KEY);`)}, - "c/000001_c.down.sql": {Data: []byte(`DROP TABLE IF EXISTS ord_c;`)}, - } - sources := []Source{ - {Name: "a", TrackingTable: "ord_a_migrations", FS: ordFS, Dir: "a"}, - {Name: "b", TrackingTable: "ord_b_migrations", FS: ordFS, Dir: "b"}, - {Name: "c", TrackingTable: "ord_c_migrations", FS: ordFS, Dir: "c"}, - } - - if err := RunUp(context.Background(), connStr, sources); err != nil { - t.Fatalf("RunUp: %v", err) +func TestConcurrentRunUp(t *testing.T) { + dsn := startTestDB(t) + source := testSource(twoMigrationFS) + start := make(chan struct{}) + errs := make(chan error, 2) + for range 2 { + go func() { + <-start + errs <- RunUp(context.Background(), dsn, []Source{source}) + }() } - for _, table := range []string{"ord_a", "ord_b", "ord_c"} { - if !tableExists(t, connStr, table) { - t.Errorf("table %s should exist", table) + close(start) + for range 2 { + if err := <-errs; err != nil { + t.Fatalf("RunUp: %v", err) } } - for _, tbl := range []string{"ord_a_migrations", "ord_b_migrations", "ord_c_migrations"} { - if got := trackVersion(t, connStr, tbl); got != 1 { - t.Errorf("%s version = %d, want 1", tbl, got) - } + if err := VerifyMigrated(context.Background(), dsn, []Source{source}); err != nil { + t.Fatalf("VerifyMigrated: %v", err) } } -// TestRunUp_CompensatingRollbackAcrossThreeSources verifies that when a later -// source fails, previously-applied sources are rolled back to their pre-run -// versions in reverse order — and that the prevVersion==0 guard skips a source -// applied for the first time. Source "a" is fresh (prev 0, not compensated); -// source "b" is pre-seeded at v1 (prev 1, rolled back); source "c" fails. -func TestRunUp_CompensatingRollbackAcrossThreeSources(t *testing.T) { - connStr := startTestDB(t) +func TestRunUpContinuesAfterLaterSourceFailure(t *testing.T) { + dsn := startTestDB(t) + first := testSource(fstest.MapFS{ + "first/000001_initial.sql": {Data: migrationSQL( + "CREATE TABLE first_result (id bigint);", + "DROP TABLE IF EXISTS first_result;", + )}, + }) + first.Name = "first" + first.Dir = "first" + first.TrackingTable = "first_schema_migrations" + second := testSource(fstest.MapFS{ + "second/000001_initial.sql": {Data: migrationSQL( + "CREATE TABLE partial_second_result (id bigint); ALTER TABLE missing_table ADD COLUMN value text;", + "DROP TABLE IF EXISTS partial_second_result;", + )}, + }) + second.Name = "second" + second.Dir = "second" + second.TrackingTable = "second_schema_migrations" - aFS := fstest.MapFS{ - "a/000001_a.up.sql": {Data: []byte(`CREATE TABLE comp_a (id INT PRIMARY KEY);`)}, - "a/000001_a.down.sql": {Data: []byte(`DROP TABLE IF EXISTS comp_a;`)}, - } - bV1FS := fstest.MapFS{ - "b/000001_b.up.sql": {Data: []byte(`CREATE TABLE comp_b (id INT PRIMARY KEY);`)}, - "b/000001_b.down.sql": {Data: []byte(`DROP TABLE IF EXISTS comp_b;`)}, + if err := RunUp(context.Background(), dsn, []Source{first, second}); err == nil { + t.Fatal("RunUp succeeded") } - bFullFS := fstest.MapFS{ - "b/000001_b.up.sql": {Data: []byte(`CREATE TABLE comp_b (id INT PRIMARY KEY);`)}, - "b/000001_b.down.sql": {Data: []byte(`DROP TABLE IF EXISTS comp_b;`)}, - "b/000002_b_addcol.up.sql": {Data: []byte(`ALTER TABLE comp_b ADD COLUMN extra TEXT;`)}, - "b/000002_b_addcol.down.sql": {Data: []byte(`ALTER TABLE comp_b DROP COLUMN IF EXISTS extra;`)}, + if !testTableExists(t, dsn, "first_result") { + t.Fatal("the completed first source was reversed") } - cFS := fstest.MapFS{ - "c/000001_bad.up.sql": {Data: []byte(`ALTER TABLE no_such_table ADD COLUMN x TEXT;`)}, - "c/000001_bad.down.sql": {Data: []byte(`SELECT 1;`)}, + if testTableExists(t, dsn, "partial_second_result") { + t.Fatal("the failed second source left a partial result") } - aSrc := Source{Name: "a", TrackingTable: "comp_a_migrations", FS: aFS, Dir: "a"} - bSrcFull := Source{Name: "b", TrackingTable: "comp_b_migrations", FS: bFullFS, Dir: "b"} - cSrc := Source{Name: "c", TrackingTable: "comp_c_migrations", FS: cFS, Dir: "c"} - - // Pre-seed b at v1 so its prevVersion is > 0 during the run below. - if _, err := applySource(context.Background(), connStr, Source{Name: "b", TrackingTable: "comp_b_migrations", FS: bV1FS, Dir: "b"}); err != nil { - t.Fatalf("seed b: %v", err) - } - - err := RunUp(context.Background(), connStr, []Source{aSrc, bSrcFull, cSrc}) - if err == nil { - t.Fatal("expected error from failing source c, got nil") + second.FS = fstest.MapFS{ + "second/000001_initial.sql": {Data: migrationSQL( + "CREATE TABLE second_result (id bigint);", + "DROP TABLE IF EXISTS second_result;", + )}, } - - // a: applied for the first time (prev 0) → compensation skipped → stays at v1. - if got := trackVersion(t, connStr, "comp_a_migrations"); got != 1 { - t.Errorf("comp_a version = %d, want 1 (prev==0 guard skips compensation)", got) + if err := RunUp(context.Background(), dsn, []Source{first, second}); err != nil { + t.Fatalf("second RunUp: %v", err) } - // b: prev was 1, advanced to 2, compensated back to 1. - if got := trackVersion(t, connStr, "comp_b_migrations"); got != 1 { - t.Errorf("comp_b version = %d, want 1 (rolled back to pre-run version)", got) + if !testTableExists(t, dsn, "second_result") { + t.Fatal("the second source did not continue") } } -// TestRunUp_SchemaScopedSource verifies a Source with a non-empty Schema creates -// the schema and lands both its objects and its tracking table there, not in public. -func TestRunUp_SchemaScopedSource(t *testing.T) { - connStr := startTestDB(t) - - schemaFS := fstest.MapFS{ - "s/000001_create.up.sql": {Data: []byte(`CREATE TABLE scoped_t (id INT PRIMARY KEY);`)}, - "s/000001_create.down.sql": {Data: []byte(`DROP TABLE IF EXISTS scoped_t;`)}, - } - src := Source{Name: "scoped", Schema: "myschema", TrackingTable: "schema_migrations", FS: schemaFS, Dir: "s"} - - if err := RunUp(context.Background(), connStr, []Source{src}); err != nil { - t.Fatalf("RunUp: %v", err) - } +func TestRunUpRejectsOldTrackingTable(t *testing.T) { + dsn := startTestDB(t) + source := testSource(fstest.MapFS{ + "migrations/000001_initial.sql": {Data: migrationSQL( + "CREATE TABLE baseline_ran (id bigint);", + "DROP TABLE IF EXISTS baseline_ran;", + )}, + }) + execSQL(t, dsn, "CREATE TABLE "+quoteIdentifier(source.TrackingTable)+" (version bigint PRIMARY KEY, dirty boolean NOT NULL)") + execSQL(t, dsn, "INSERT INTO "+quoteIdentifier(source.TrackingTable)+" (version, dirty) VALUES (19, false)") - if !tableExistsInSchema(t, connStr, "myschema", "scoped_t") { - t.Error("scoped_t should exist in myschema") - } - if tableExistsInSchema(t, connStr, "public", "scoped_t") { - t.Error("scoped_t should NOT exist in public") - } - if !tableExistsInSchema(t, connStr, "myschema", "schema_migrations") { - t.Error("tracking table should exist in myschema") + if err := RunUp(context.Background(), dsn, []Source{source}); err == nil || !strings.Contains(err.Error(), "new PostgreSQL database") { + t.Fatalf("RunUp error = %v", err) } - if tableExistsInSchema(t, connStr, "public", "schema_migrations") { - t.Error("tracking table should NOT exist in public") + if testTableExists(t, dsn, "baseline_ran") { + t.Fatal("the baseline ran against an old tracking table") } } -// TestRunUp_RejectsResolvedSchemaCollision verifies the runtime guard catches a -// collision validateSources cannot: an unscoped source (Schema "") and an explicit -// source naming the connection's current_schema() ("public" here) share one -// tracking table once "" resolves, so RunUp must reject the set before applying -// anything. validateSources alone passes them (distinct literal Schema keys). -func TestRunUp_RejectsResolvedSchemaCollision(t *testing.T) { - connStr := startTestDB(t) +func TestPrechecksRunBeforeMigrations(t *testing.T) { + dsn := startTestDB(t) + first := testSource(twoMigrationFS) + second := testSource(fstest.MapFS{ + "other/000001_initial.sql": {Data: migrationSQL("SELECT 1;", "SELECT 1;")}, + }) + second.Name = "other" + second.Dir = "other" + second.TrackingTable = "other_schema_migrations" + second.PreCheck = func(string) error { return errors.New("blocked") } - collide := []Source{ - {Name: "implicit", Schema: "", TrackingTable: "schema_migrations", FS: goodCoreFS, Dir: "core"}, - {Name: "explicit", Schema: "public", TrackingTable: "schema_migrations", FS: goodCoreFS, Dir: "core"}, + err := RunUp(context.Background(), dsn, []Source{first, second}) + if err == nil || !strings.Contains(err.Error(), "blocked") { + t.Fatalf("RunUp error = %v", err) } - - // validateSources keys on the literal Schema, so it does NOT catch this. - if err := validateSources(collide); err != nil { - t.Fatalf("validateSources should pass on distinct literal schemas, got %v", err) + if testTableExists(t, dsn, first.TrackingTable) || testTableExists(t, dsn, "migration_test") { + t.Fatal("the first source changed the database before preflight failed") } +} - // RunUp resolves "" to current_schema() (public) and must reject. - err := RunUp(context.Background(), connStr, collide) - if err == nil { - t.Fatal("expected resolved-collision error, got nil") +func TestRunUpAllowsDatabaseAhead(t *testing.T) { + dsn := startTestDB(t) + source := testSource(fstest.MapFS{ + "migrations/000001_initial.sql": {Data: migrationSQL("SELECT 1;", "SELECT 1;")}, + }) + if err := RunUp(context.Background(), dsn, []Source{source}); err != nil { + t.Fatal(err) } - if !strings.Contains(err.Error(), "resolve to the same tracking table") { - t.Errorf("error %q should describe a resolved tracking-table collision", err) + execSQL(t, dsn, "INSERT INTO "+quoteIdentifier(source.TrackingTable)+" (version_id, is_applied) VALUES (2, true)") + if err := RunUp(context.Background(), dsn, []Source{source}); err != nil { + t.Fatalf("RunUp: %v", err) } - // Guard runs before any source applies — no tracking table created. - if tableExists(t, connStr, "schema_migrations") { - t.Error("schema_migrations should not exist — guard must fire before any source applies") + if err := VerifyMigrated(context.Background(), dsn, []Source{source}); err != nil { + t.Fatalf("VerifyMigrated: %v", err) } } -// TestRunUp_PreCheckRunsBeforeAnyApply verifies that a failing PreCheck on a later -// source aborts the run before any earlier source is applied. -func TestRunUp_PreCheckRunsBeforeAnyApply(t *testing.T) { - connStr := startTestDB(t) - - preFS := fstest.MapFS{ - "p/000001_create.up.sql": {Data: []byte(`CREATE TABLE precheck_t (id INT PRIMARY KEY);`)}, - "p/000001_create.down.sql": {Data: []byte(`DROP TABLE IF EXISTS precheck_t;`)}, - } - first := Source{Name: "first", TrackingTable: "first_migrations", FS: preFS, Dir: "p"} - second := Source{Name: "second", TrackingTable: "second_migrations", FS: preFS, Dir: "p", - PreCheck: func(string) error { return fmt.Errorf("precheck boom") }} - - err := RunUp(context.Background(), connStr, []Source{first, second}) - if err == nil { - t.Fatal("expected error from failing precheck, got nil") - } - if tableExists(t, connStr, "precheck_t") { - t.Error("precheck_t should not exist — no source should apply when a precheck fails") +func TestMigrationFileValidation(t *testing.T) { + tests := []struct { + name string + file string + body string + want string + }{ + {name: "valid", body: "-- +goose Up\nSELECT 1;\n-- +goose Down\nSELECT 1;"}, + {name: "missing up", body: "-- +goose Down\nSELECT 1;", want: "no Goose Up"}, + {name: "missing down", body: "-- +goose Up\nSELECT 1;", want: "no Goose Down"}, + {name: "no transaction", body: "-- +goose NO TRANSACTION\n-- +goose Up\nSELECT 1;\n-- +goose Down\nSELECT 1;", want: "disables transactions"}, + {name: "no transaction with extra spacing", body: "-- +goose NO TRANSACTION\n-- +goose Up\nSELECT 1;\n-- +goose Down\nSELECT 1;", want: "disables transactions"}, + {name: "down text in SQL", body: "-- +goose Up\nSELECT '-- +goose Down';", want: "no Goose Down"}, + {name: "legacy up file", file: "000001_test.up.sql", body: "-- +goose Up\nSELECT 1;\n-- +goose Down\nSELECT 1;", want: "invalid migration file"}, + {name: "legacy down file", file: "000001_test.down.sql", body: "-- +goose Up\nSELECT 1;\n-- +goose Down\nSELECT 1;", want: "invalid migration file"}, } - if tableExists(t, connStr, "first_migrations") { - t.Error("first_migrations tracking table should not exist — first source must not have applied") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + file := tt.file + if file == "" { + file = "000001_test.sql" + } + source := testSource(fstest.MapFS{ + "migrations/" + file: {Data: []byte(tt.body)}, + }) + _, err := migrationVersions(source) + if tt.want == "" && err != nil { + t.Fatalf("migrationVersions: %v", err) + } + if tt.want != "" && (err == nil || !strings.Contains(err.Error(), tt.want)) { + t.Fatalf("migrationVersions error = %v", err) + } + }) } } -func TestValidateSchemaName(t *testing.T) { - valid := []string{"a", "myschema", "tenant_1", "_x", "s123"} - for _, s := range valid { - if err := validateSchemaName(s); err != nil { - t.Errorf("validateSchemaName(%q) = %v, want nil", s, err) - } - } - // Uppercase is rejected: the name is used unquoted in search_path (which - // Postgres case-folds) and quoted in CREATE SCHEMA, so a mixed-case name - // would split across two schemas. - invalid := []string{"", "1abc", "has space", "a;b", `a"b`, "a-b", "a.b", "drop table", "ABC", "MySchema", strings.Repeat("x", 64)} - for _, s := range invalid { - if err := validateSchemaName(s); err == nil { - t.Errorf("validateSchemaName(%q) = nil, want error", s) - } +func TestMigrationVersionsFromRoot(t *testing.T) { + source := testSource(fstest.MapFS{ + "000001_test.sql": {Data: migrationSQL("SELECT 1;", "SELECT 1;")}, + }) + source.Dir = "." + if _, err := migrationVersions(source); err != nil { + t.Fatal(err) } } -// --- container-free unit tests --- - -// TestValidateSources_RejectsDuplicateSchemaAndTable verifies the collision unit -// is (Schema, TrackingTable): two sources sharing both are rejected, while the -// same tracking-table name in different schemas, or different tables in the same -// schema, are allowed. -func TestValidateSources_RejectsDuplicateSchemaAndTable(t *testing.T) { - dummy := fstest.MapFS{} // non-nil so the required-field checks pass - - collide := []Source{ - {Name: "a", Schema: "s1", TrackingTable: "schema_migrations", FS: dummy, Dir: "a"}, - {Name: "b", Schema: "s1", TrackingTable: "schema_migrations", FS: dummy, Dir: "b"}, - } - if err := validateSources(collide); err == nil { - t.Error("expected collision error for same (schema, tracking table), got nil") +func TestValidateSources(t *testing.T) { + valid := testSource(twoMigrationFS) + if err := validateSources([]Source{valid}); err != nil { + t.Fatalf("validateSources: %v", err) } - ok := []Source{ - // Same table name, different schema — fine. - {Name: "a", Schema: "s1", TrackingTable: "schema_migrations", FS: dummy, Dir: "a"}, - {Name: "b", Schema: "s2", TrackingTable: "schema_migrations", FS: dummy, Dir: "b"}, - // Different table, same (default) schema — fine. - {Name: "core", TrackingTable: "schema_migrations", FS: dummy, Dir: "core"}, - {Name: "vector", TrackingTable: "vector_schema_migrations", FS: dummy, Dir: "vector"}, + tests := []Source{ + {Name: "", TrackingTable: valid.TrackingTable, FS: valid.FS, Dir: valid.Dir}, + {Name: "test", TrackingTable: "Bad-Table", FS: valid.FS, Dir: valid.Dir}, + {Name: "test", Schema: "Bad-Schema", TrackingTable: valid.TrackingTable, FS: valid.FS, Dir: valid.Dir}, } - if err := validateSources(ok); err != nil { - t.Errorf("validateSources(distinct sources) = %v, want nil", err) + for _, source := range tests { + if err := validateSources([]Source{source}); err == nil { + t.Fatalf("validateSources(%+v) succeeded", source) + } } - // An invalid schema name on any source is rejected up front, before any - // source applies (fail-fast). - badSchema := []Source{ - {Name: "good", TrackingTable: "schema_migrations", FS: dummy, Dir: "core"}, - {Name: "bad", Schema: "MixedCase", TrackingTable: "schema_migrations", FS: dummy, Dir: "x"}, - } - if err := validateSources(badSchema); err == nil { - t.Error("expected error for invalid schema name on a later source, got nil") + other := valid + other.Name = "other" + if err := validateSources([]Source{valid, other}); err == nil { + t.Fatal("validateSources accepted a collision") } } -// TestValidateSources_RejectsMissingRequiredFields verifies Source's required -// fields (Name, TrackingTable, FS, Dir) are enforced up front, while Schema -// stays optional ("" = connection default). -func TestValidateSources_RejectsMissingRequiredFields(t *testing.T) { - base := Source{Name: "x", TrackingTable: "x_migrations", FS: fstest.MapFS{}, Dir: "x"} - - // The fully-populated base (with empty Schema) is valid. - if err := validateSources([]Source{base}); err != nil { - t.Fatalf("validateSources(valid source) = %v, want nil", err) +func TestBuiltinTrackingTables(t *testing.T) { + sources := BuiltinSources(true) + if len(sources) != 2 { + t.Fatalf("sources = %d", len(sources)) } - - mutations := map[string]func(Source) Source{ - "empty Name": func(s Source) Source { s.Name = ""; return s }, - "empty TrackingTable": func(s Source) Source { s.TrackingTable = ""; return s }, - "nil FS": func(s Source) Source { s.FS = nil; return s }, - "empty Dir": func(s Source) Source { s.Dir = ""; return s }, - } - for name, mut := range mutations { - t.Run(name, func(t *testing.T) { - if err := validateSources([]Source{mut(base)}); err == nil { - t.Errorf("validateSources with %s = nil, want error", name) - } - }) - } -} - -// TestRunUp_EmptyAndNilSources verifies the no-op fast paths run no migration -// logic and need no database. -func TestRunUp_EmptyAndNilSources(t *testing.T) { - if err := RunUp(context.Background(), "postgres://invalid", nil); err != nil { - t.Errorf("RunUp(nil sources) = %v, want nil", err) + if sources[0].TrackingTable != coreTrackingTable { + t.Fatalf("core source = %+v", sources[0]) } - if err := RunUp(context.Background(), "postgres://invalid", []Source{}); err != nil { - t.Errorf("RunUp(empty sources) = %v, want nil", err) + if sources[1].TrackingTable != vectorTrackingTable { + t.Fatalf("vector source = %+v", sources[1]) } } -// TestWithSearchPath verifies a postgres:// or postgresql:// DSN gains the -// search_path param (preserving existing params) and that anything else is -// rejected rather than silently rewritten. func TestWithSearchPath(t *testing.T) { - for _, dsn := range []string{ - "postgres://u:p@host:5432/db?sslmode=disable", - "postgresql://u:p@host:5432/db", - } { - got, err := withSearchPath(dsn, "myschema") - if err != nil { - t.Fatalf("withSearchPath(%q) = %v, want nil", dsn, err) - } - if !strings.Contains(got, "search_path=myschema") { - t.Errorf("withSearchPath(%q) = %q, missing search_path=myschema", dsn, got) - } + got, err := withSearchPath("postgres://u:p@host/db?sslmode=disable", "tenant_1") + if err != nil { + t.Fatal(err) } - - // Existing query params are preserved. - if got, _ := withSearchPath("postgres://u:p@host:5432/db?sslmode=disable", "myschema"); !strings.Contains(got, "sslmode=disable") { - t.Errorf("result %q dropped existing sslmode param", got) + if !strings.Contains(got, "search_path=tenant_1") || !strings.Contains(got, "sslmode=disable") { + t.Fatalf("URL = %q", got) } - - // Non-postgres inputs fail fast rather than being silently rewritten. - for _, bad := range []string{ - "host=localhost dbname=foo user=bar", // libpq keyword/value DSN (scheme "") - "localhost:5432/db", // parses as scheme "localhost" - "mysql://u:p@host/db", // wrong scheme - "", // empty - } { - if _, err := withSearchPath(bad, "myschema"); err == nil { - t.Errorf("withSearchPath(%q) = nil, want error", bad) - } + if _, err := withSearchPath("mysql://host/db", "tenant_1"); err == nil { + t.Fatal("withSearchPath accepted MySQL") } } -// --- dirty state recovery tests --- - -// TestApplySource_DirtyStateRecoveryOnRestart simulates a restart after a failed -// migration left the database in a dirty state. On the second call, prevVersion -// is > 0 (the dirty version), so rollback is enabled. The runner should clear -// the dirty state and roll back to the last clean version. -func TestApplySource_DirtyStateRecoveryOnRestart(t *testing.T) { - connStr := startTestDB(t) - - // First run: apply version 1, then version 2 fails. prevVersion is 0, so - // rollback is skipped. Database left at version 2 dirty. - if _, err := applySource(context.Background(), connStr, coreSource(failOnSecondCoreFS)); err == nil { - t.Fatal("expected error, got nil") - } - if got := trackVersion(t, connStr, "schema_migrations"); got != 2 { - t.Fatalf("after first run: version = %d, want 2 (dirty)", got) +func TestAdvisoryLockIDIsStable(t *testing.T) { + if got := advisoryLockID("kagent_test", "public", "schema_migrations"); got != 4022843769 { + t.Fatalf("lock ID = %d", got) } +} - // Second run (simulating restart): prevVersion is now 2 (dirty). The runner - // should detect dirty state and attempt to clear it. mg.Up() will fail because - // the database is dirty, then rollbackToVersion clears dirty to version 1. - _, err := applySource(context.Background(), connStr, coreSource(failOnSecondCoreFS)) - if err == nil { - t.Fatal("expected error on second run, got nil") +func TestEmptySources(t *testing.T) { + if err := RunUp(context.Background(), "postgres://unused", nil); err != nil { + t.Fatal(err) } - // After rollback clears dirty state, version should be at 1 (last clean). - if got := trackVersion(t, connStr, "schema_migrations"); got != 1 { - t.Errorf("after restart: version = %d, want 1 (dirty cleared, rolled back)", got) + if err := VerifyMigrated(context.Background(), "postgres://unused", nil); err != nil { + t.Fatal(err) } } - -// TestVerifyMigrated covers the SKIP_MIGRATIONS boot guard: refuse an -// un-migrated or behind or dirty database, tolerate exact-match and ahead -// (compatibility mode), and work on a connection whose role has no DDL -// privileges — the deployment mode the guard exists for. -func TestVerifyMigrated(t *testing.T) { - ctx := context.Background() - connStr := startTestDB(t) - full := []Source{coreSource(goodCoreFS)} // max embedded version 2 - - t.Run("unmigrated database is refused", func(t *testing.T) { - err := VerifyMigrated(ctx, connStr, full) - if err == nil || !strings.Contains(err.Error(), "has not been migrated") { - t.Fatalf("error = %v, want missing-tracking-table refusal", err) - } - }) - - t.Run("pending migrations are refused", func(t *testing.T) { - if _, err := applySource(ctx, connStr, coreSource(oneCoreFS)); err != nil { - t.Fatalf("apply v1: %v", err) - } - err := VerifyMigrated(ctx, connStr, full) - if err == nil || !strings.Contains(err.Error(), "requires version 2") { - t.Fatalf("error = %v, want behind-binary refusal", err) - } - }) - - t.Run("fully migrated database passes", func(t *testing.T) { - if _, err := applySource(ctx, connStr, coreSource(goodCoreFS)); err != nil { - t.Fatalf("apply v2: %v", err) - } - if err := VerifyMigrated(ctx, connStr, full); err != nil { - t.Fatalf("VerifyMigrated() = %v, want nil", err) - } - }) - - t.Run("works without DDL privileges", func(t *testing.T) { - db, err := sql.Open("pgx", connStr) - if err != nil { - t.Fatal(err) - } - defer db.Close() - for _, q := range []string{ - `CREATE ROLE readonly LOGIN PASSWORD 'ro'`, - `GRANT USAGE ON SCHEMA public TO readonly`, - `GRANT SELECT ON ALL TABLES IN SCHEMA public TO readonly`, - } { - if _, err := db.ExecContext(ctx, q); err != nil { - t.Fatalf("%s: %v", q, err) - } - } - u, err := url.Parse(connStr) - if err != nil { - t.Fatal(err) - } - u.User = url.UserPassword("readonly", "ro") - if err := VerifyMigrated(ctx, u.String(), full); err != nil { - t.Fatalf("VerifyMigrated() as readonly = %v, want nil", err) - } - }) - - t.Run("database ahead of binary passes", func(t *testing.T) { - if err := VerifyMigrated(ctx, connStr, []Source{coreSource(oneCoreFS)}); err != nil { - t.Fatalf("VerifyMigrated() with older binary = %v, want nil (compatibility mode)", err) - } - }) - - t.Run("dirty tracking table is refused", func(t *testing.T) { - db, err := sql.Open("pgx", connStr) - if err != nil { - t.Fatal(err) - } - defer db.Close() - if _, err := db.ExecContext(ctx, `UPDATE schema_migrations SET dirty = true`); err != nil { - t.Fatal(err) - } - defer func() { - if _, err := db.ExecContext(ctx, `UPDATE schema_migrations SET dirty = false`); err != nil { - t.Fatal(err) - } - }() - err = VerifyMigrated(ctx, connStr, full) - if err == nil || !strings.Contains(err.Error(), "dirty") { - t.Fatalf("error = %v, want dirty refusal", err) - } - }) - - t.Run("resolved schema collision is refused", func(t *testing.T) { - // Schema "" resolves to public here, colliding with the explicit - // "public" source on the same tracking table — the same source set - // RunUp rejects. - collide := []Source{ - coreSource(goodCoreFS), - {Name: "explicit", Schema: "public", TrackingTable: "schema_migrations", FS: goodCoreFS, Dir: "core"}, - } - err := VerifyMigrated(ctx, connStr, collide) - if err == nil || !strings.Contains(err.Error(), "resolve to the same tracking table") { - t.Fatalf("error = %v, want resolved-schema collision refusal", err) - } - }) - - t.Run("no sources is a no-op", func(t *testing.T) { - if err := VerifyMigrated(ctx, "postgres://unused", nil); err != nil { - t.Fatalf("VerifyMigrated() with no sources = %v, want nil", err) - } - }) -} diff --git a/go/core/pkg/migrations/vector/000001_initial.sql b/go/core/pkg/migrations/vector/000001_initial.sql new file mode 100644 index 000000000..c99d3dfa1 --- /dev/null +++ b/go/core/pkg/migrations/vector/000001_initial.sql @@ -0,0 +1,24 @@ +-- +goose Up + +-- Kagent 1.0 vector baseline. + +CREATE EXTENSION IF NOT EXISTS vector; + +CREATE TABLE memory ( + id TEXT PRIMARY KEY DEFAULT gen_random_uuid(), + agent_name TEXT, + user_id TEXT, + content TEXT, + embedding vector(768), + metadata TEXT, + created_at TIMESTAMPTZ, + expires_at TIMESTAMPTZ, + access_count BIGINT DEFAULT 0 +); +CREATE INDEX idx_memory_agent_user ON memory(agent_name, user_id); +CREATE INDEX idx_memory_expires_at ON memory(expires_at); +CREATE INDEX idx_memory_embedding_hnsw ON memory USING hnsw (embedding vector_cosine_ops); + +-- +goose Down + +DROP TABLE memory; diff --git a/go/core/pkg/migrations/vector/000001_vector_support.down.sql b/go/core/pkg/migrations/vector/000001_vector_support.down.sql deleted file mode 100644 index b403931d8..000000000 --- a/go/core/pkg/migrations/vector/000001_vector_support.down.sql +++ /dev/null @@ -1,2 +0,0 @@ -DROP TABLE IF EXISTS memory; -DROP EXTENSION IF EXISTS vector; diff --git a/go/core/pkg/migrations/vector/000001_vector_support.up.sql b/go/core/pkg/migrations/vector/000001_vector_support.up.sql deleted file mode 100644 index 3b1e66234..000000000 --- a/go/core/pkg/migrations/vector/000001_vector_support.up.sql +++ /dev/null @@ -1,17 +0,0 @@ -CREATE EXTENSION IF NOT EXISTS vector; - --- Matches the schema GORM AutoMigrate produced for the Memory struct. --- GORM does not create HNSW indexes automatically; that is added in migration 000002. -CREATE TABLE IF NOT EXISTS memory ( - id TEXT PRIMARY KEY, - agent_name TEXT, - user_id TEXT, - content TEXT, - embedding vector(768), - metadata TEXT, - created_at TIMESTAMPTZ, - expires_at TIMESTAMPTZ, - access_count BIGINT DEFAULT 0 -); -CREATE INDEX IF NOT EXISTS idx_memory_agent_user ON memory(agent_name, user_id); -CREATE INDEX IF NOT EXISTS idx_memory_expires_at ON memory(expires_at); diff --git a/go/core/pkg/migrations/vector/000002_add_memory_hnsw_index.down.sql b/go/core/pkg/migrations/vector/000002_add_memory_hnsw_index.down.sql deleted file mode 100644 index ee9bc90ed..000000000 --- a/go/core/pkg/migrations/vector/000002_add_memory_hnsw_index.down.sql +++ /dev/null @@ -1 +0,0 @@ -DROP INDEX IF EXISTS idx_memory_embedding_hnsw; diff --git a/go/core/pkg/migrations/vector/000002_add_memory_hnsw_index.up.sql b/go/core/pkg/migrations/vector/000002_add_memory_hnsw_index.up.sql deleted file mode 100644 index ad4ddfdc1..000000000 --- a/go/core/pkg/migrations/vector/000002_add_memory_hnsw_index.up.sql +++ /dev/null @@ -1,4 +0,0 @@ --- Add HNSW index for fast approximate nearest-neighbor vector similarity search. --- GORM did not create this index automatically; pgvector's HNSW index significantly --- outperforms IVFFlat for production workloads (better recall, no reindex on insert). -CREATE INDEX IF NOT EXISTS idx_memory_embedding_hnsw ON memory USING hnsw (embedding vector_cosine_ops); diff --git a/go/core/pkg/migrations/vector/000003_memory_uuid_default.down.sql b/go/core/pkg/migrations/vector/000003_memory_uuid_default.down.sql deleted file mode 100644 index d636b8066..000000000 --- a/go/core/pkg/migrations/vector/000003_memory_uuid_default.down.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE memory ALTER COLUMN id DROP DEFAULT; diff --git a/go/core/pkg/migrations/vector/000003_memory_uuid_default.up.sql b/go/core/pkg/migrations/vector/000003_memory_uuid_default.up.sql deleted file mode 100644 index f04772e30..000000000 --- a/go/core/pkg/migrations/vector/000003_memory_uuid_default.up.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE memory ALTER COLUMN id SET DEFAULT gen_random_uuid(); diff --git a/go/core/test/e2e/interaction_test.go b/go/core/test/e2e/interaction_test.go index 7fa0394b2..45abad357 100644 --- a/go/core/test/e2e/interaction_test.go +++ b/go/core/test/e2e/interaction_test.go @@ -52,6 +52,13 @@ var interactionMocks embed.FS func TestAgentInstanceInteraction(t *testing.T) { t.Parallel() fixture := newInteractionFixture(t, interactionTarget(t), startInteractionMock(t)) + assertAgentInstanceInteraction(t, fixture) + assertAgentInstanceInteraction(t, fixture) +} + +func assertAgentInstanceInteraction(t *testing.T, fixture *interactionFixture) { + t.Helper() + _, _, task := fixture.send(t, "What is 2+2?") if task.Status.State != a2atype.TaskStateCompleted { t.Fatalf("A2A task state = %s, want COMPLETED", task.Status.State) @@ -59,10 +66,6 @@ func TestAgentInstanceInteraction(t *testing.T) { if text := taskText(task); !strings.Contains(text, "The answer is 4.") { t.Fatalf("A2A response text = %q, want mock LLM response", text) } - _, _, task = fixture.send(t, "What is 2+2?") - if task.Status.State != a2atype.TaskStateCompleted { - t.Fatalf("second A2A task state = %s, want COMPLETED", task.Status.State) - } } func TestOpaqueBYOAgentInteraction(t *testing.T) { diff --git a/go/core/test/upgrade/invoke_e2e_test.go b/go/core/test/upgrade/invoke_e2e_test.go index a51fbff51..cf54bd60e 100644 --- a/go/core/test/upgrade/invoke_e2e_test.go +++ b/go/core/test/upgrade/invoke_e2e_test.go @@ -6,68 +6,78 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "time" "github.com/stretchr/testify/require" ) -// invokeE2ETest is the representative slice of the e2e suite we run at each -// upgrade state: it deploys a declarative agent (default runtime), invokes it -// through the real controller against an in-process mock LLM, and asserts — i.e. -// it exercises kagent's actual query paths, not raw SQL. Run it from the tree -// whose version matches the serving controller so the API/CRD shapes line up. -const invokeE2ETest = "^TestE2EInvokeInlineAgent$" +const invokeE2ETest = "^TestAgentInstanceInteraction$" -// prevTreeGoDir returns the `go/` directory of the previous release's checkout -// (a git worktree at its tag, created by the run-upgrade-tests make target and -// passed via PREV_E2E_DIR), or "" when it isn't available. -func prevTreeGoDir() string { - dir := os.Getenv("PREV_E2E_DIR") - if dir == "" { - return "" +func checkoutPreviousRelease(t *testing.T, env upgradeEnv) string { + t.Helper() + + ref := env.upgradeFromVersion + if !strings.HasPrefix(ref, "v") { + ref = "v" + ref } - return filepath.Join(dir, "go") + worktree := filepath.Join(t.TempDir(), "previous-release") + cmd := exec.CommandContext(t.Context(), "git", "worktree", "add", "--detach", worktree, ref) + cmd.Dir = env.repoRoot + out, err := cmd.CombinedOutput() + require.NoError(t, err, "check out previous release %s:\n%s", ref, string(out)) + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, "git", "worktree", "remove", "--force", worktree) + cmd.Dir = env.repoRoot + if out, err := cmd.CombinedOutput(); err != nil { + t.Logf("remove previous-release worktree: %v\n%s", err, string(out)) + } + }) + return filepath.Join(worktree, "go") } -// runInvokeE2E runs the invoke e2e slice from treeGoDir against the controller -// currently serving in the cluster. treeGoDir is the `go/` module dir of the -// matching-version tree (repo root for HEAD, the worktree for the prior -// release). It port-forwards the controller for KAGENT_URL — re-established per -// state, so it survives the controller being reinstalled between states — and -// relies on KAGENT_LOCAL_HOST (kind gateway IP, set by the make target) for the -// agent→host mock-LLM callback. label identifies the state in messages. -// -// It self-skips when the harness isn't set up (no KAGENT_LOCAL_HOST) or the -// test tree isn't available, so a bare `go test -run TestUpgrade` still runs the -// DB round-trip without needing a worktree. func runInvokeE2E(t *testing.T, env upgradeEnv, treeGoDir, label string) { t.Helper() - if os.Getenv("KAGENT_LOCAL_HOST") == "" { - t.Skipf("[%s] KAGENT_LOCAL_HOST is not set; run via `make run-upgrade-tests` to exercise the invoke e2e slice", label) - } - if treeGoDir == "" { - t.Skipf("[%s] no matching-version test tree available (PREV_E2E_DIR unset)", label) - } - if _, err := os.Stat(filepath.Join(treeGoDir, "go.mod")); err != nil { - t.Skipf("[%s] test tree %q is not usable: %v", label, treeGoDir, err) - } - - port, stop := startPortForward(t, env, controllerServiceName, controllerAPIPort) + requireInvokeEnvironment(t, treeGoDir, label) + port, stop := startPortForward(t, env, controllerServiceName, controllerGRPCPort) defer stop() ctx, cancel := context.WithTimeout(t.Context(), 10*time.Minute) defer cancel() - cmd := exec.CommandContext(ctx, "go", "test", "./core/test/e2e", - "-run", invokeE2ETest, "-count=1", "-v") + "-run", invokeE2ETest, "-count=1", "-timeout=9m", "-v") cmd.Dir = treeGoDir - // KAGENT_LOCAL_HOST is inherited from the environment (set by the make - // target); KAGENT_URL points the e2e client at the port-forwarded controller. - cmd.Env = append(os.Environ(), fmt.Sprintf("KAGENT_URL=http://127.0.0.1:%d", port)) + cmd.Env = upgradeE2EEnv(t, env, port) out, err := cmd.CombinedOutput() require.NoErrorf(t, err, "[%s] invoke e2e slice (%s) failed:\n%s", label, treeGoDir, string(out)) - t.Logf("[%s] invoke e2e slice passed", label) + require.Contains(t, string(out), "--- PASS: TestAgentInstanceInteraction", + "[%s] selected invoke test did not run:\n%s", label, string(out)) +} + +func upgradeE2EEnv(t *testing.T, env upgradeEnv, port int) []string { + t.Helper() + + kubeconfig := kubectl(t, env, time.Minute, "config", "view", "--raw", "--flatten", "--minify") + kubeconfigPath := filepath.Join(t.TempDir(), "kubeconfig") + require.NoError(t, os.WriteFile(kubeconfigPath, []byte(kubeconfig), 0o600)) + return append(os.Environ(), + fmt.Sprintf("KAGENT_E2E_GRPC_TARGET=127.0.0.1:%d", port), + "KUBECONFIG="+kubeconfigPath, + ) +} + +func requireInvokeEnvironment(t *testing.T, treeGoDir, label string) { + t.Helper() + + if os.Getenv("KAGENT_LOCAL_HOST") == "" { + t.Skipf("[%s] KAGENT_LOCAL_HOST is not set; run via make run-upgrade-tests", label) + } + if _, err := os.Stat(filepath.Join(treeGoDir, "go.mod")); err != nil { + t.Fatalf("[%s] test tree %q is not usable: %v", label, treeGoDir, err) + } } diff --git a/go/core/test/upgrade/rolling_upgrade_test.go b/go/core/test/upgrade/rolling_upgrade_test.go index 4ad6f58f0..a437ce0d4 100644 --- a/go/core/test/upgrade/rolling_upgrade_test.go +++ b/go/core/test/upgrade/rolling_upgrade_test.go @@ -19,24 +19,25 @@ func TestRollingUpgradeCompatibility(t *testing.T) { env := loadUpgradeEnv(t) targetCoreVersion := latestCoreMigrationVersion(t) seed := fmt.Sprintf("%d", time.Now().UnixNano()) - baselineAgentID := "rolling-baseline-agent-" + seed - compatAgentID := "rolling-compat-agent-" + seed - compatUserID := "rolling-compat-user-" + seed + baselineToolID := "rolling-baseline-tool-" + seed + compatToolID := "rolling-compat-tool-" + seed + toolServerName := "rolling-toolserver-" + seed + groupKind := "upgrade.rolling/v1/Canary" t.Logf("rolling upgrade test: %s -> %s (registry=%s, kubeContext=%s)", env.upgradeFromVersion, env.version, env.dockerRegistry, env.kubeContext) waitForReadyPods(t, env, postgresSelector, 3*time.Minute) - waitForPostgresAgentTable(t, env, 3*time.Minute) + waitForPostgresSchema(t, env, 3*time.Minute) + if !hasGooseMigrationTable(t, env) { + t.Skip("the baseline release does not use Goose") + } // Run even when there is no migration delta: a rolling upgrade rolls the new // image regardless of migrations, so the deploy can still break for // non-schema reasons (a crashing new image, readiness, old pods against // new-code-created resources). When the target build does add migrations, the // same flow additionally exercises the old-code/new-schema window below. - baselineState := pgMigrationState(t, env) - require.False(t, baselineState.dirty, "baseline Postgres migrations are dirty") - // Keep multiple old controller pods around during the rollout. With a single // replica the old-code/new-schema window can be too small to observe reliably. kubectl(t, env, 2*time.Minute, @@ -55,7 +56,9 @@ func TestRollingUpgradeCompatibility(t *testing.T) { // Seed with the baseline schema before the target controller applies new // migrations. The compatibility canary below verifies this row is still // readable while old pods are alive against the target schema. - pgExec(t, env, fmt.Sprintf("INSERT INTO agent (id, type) VALUES (%s, 'Deployment')", pgQuote(baselineAgentID))) + pgExec(t, env, fmt.Sprintf( + "INSERT INTO tool (id, server_name, group_kind, description) VALUES (%s, %s, %s, 'rolling baseline tool')", + pgQuote(baselineToolID), pgQuote(toolServerName), pgQuote(groupKind))) ctx, cancel := context.WithTimeout(t.Context(), 10*time.Minute) defer cancel() @@ -89,9 +92,7 @@ func TestRollingUpgradeCompatibility(t *testing.T) { if err != nil { return false } - return state.version == targetCoreVersion && - !state.dirty && - anyPodsReady(t, env, oldPods) + return state.version == targetCoreVersion && anyPodsReady(t, env, oldPods) }, 6*time.Minute, 500*time.Millisecond, "target schema was not observed while old controller pods were still ready") require.NoError(t, helmErr, "helm upgrade failed before target schema was observed:\n%s", helmOut) @@ -99,18 +100,17 @@ func TestRollingUpgradeCompatibility(t *testing.T) { // not prove every previous-release code path works, but they catch migrations // that break basic old read/write assumptions during a rolling deployment. require.Equal(t, 1, - pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM agent WHERE id = %s AND type = 'Deployment'", pgQuote(baselineAgentID))), + pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM tool WHERE id = %s AND server_name = %s AND group_kind = %s", + pgQuote(baselineToolID), pgQuote(toolServerName), pgQuote(groupKind))), "old-shape read failed after target schema was applied", ) - pgExec(t, env, fmt.Sprintf("INSERT INTO agent (id, type) VALUES (%s, 'Deployment')", pgQuote(compatAgentID))) - pgExec(t, env, fmt.Sprintf("INSERT INTO feedback (user_id, feedback_text) VALUES (%s, 'rolling compatibility feedback')", pgQuote(compatUserID))) - require.Equal(t, 1, - pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM agent WHERE id = %s", pgQuote(compatAgentID))), - "old-shape agent write did not survive against target schema", - ) + pgExec(t, env, fmt.Sprintf( + "INSERT INTO tool (id, server_name, group_kind, description) VALUES (%s, %s, %s, 'rolling compatibility tool')", + pgQuote(compatToolID), pgQuote(toolServerName), pgQuote(groupKind))) require.Equal(t, 1, - pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM feedback WHERE user_id = %s", pgQuote(compatUserID))), - "old-shape feedback write did not survive against target schema", + pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM tool WHERE id = %s AND server_name = %s AND group_kind = %s", + pgQuote(compatToolID), pgQuote(toolServerName), pgQuote(groupKind))), + "old-shape tool write did not survive against target schema", ) result := <-done @@ -122,7 +122,6 @@ func TestRollingUpgradeCompatibility(t *testing.T) { "--timeout=3m", ) finalState := pgMigrationState(t, env) - require.False(t, finalState.dirty, "post-rollout Postgres migrations are dirty") require.Equal(t, targetCoreVersion, finalState.version, "final migration version") } diff --git a/go/core/test/upgrade/roundtrip_test.go b/go/core/test/upgrade/roundtrip_test.go index 8af9cd8da..7b84c453b 100644 --- a/go/core/test/upgrade/roundtrip_test.go +++ b/go/core/test/upgrade/roundtrip_test.go @@ -3,48 +3,28 @@ package upgrade import ( "bufio" "context" - "database/sql" - "errors" "fmt" "os/exec" "regexp" + "slices" "strconv" "strings" "testing" "time" - "github.com/golang-migrate/migrate/v4" - migratepgx "github.com/golang-migrate/migrate/v4/database/pgx/v5" - "github.com/golang-migrate/migrate/v4/source/iofs" - _ "github.com/jackc/pgx/v5/stdlib" migrations "github.com/kagent-dev/kagent/go/core/pkg/migrations" + "github.com/pressly/goose/v3" "github.com/stretchr/testify/require" ) const postgresServiceName = "kagent-postgresql" -// migrationTrack mirrors one migration source: the FS subdirectory it owns -// and its golang-migrate tracking table. Registration order is core, then -// vector; rollback reverses it (vector, then core) so a track is never reversed -// while a later-registered track still depends on its schema. -type migrationTrack struct { - name string - dir string - trackingTable string -} - -var migrationTracks = []migrationTrack{ - {name: "core", dir: "core", trackingTable: "schema_migrations"}, - {name: "vector", dir: "vector", trackingTable: "vector_schema_migrations"}, -} - -// pgTrackVersion returns the current applied version of a golang-migrate -// tracking table, or 0 when the table does not exist (e.g. a disabled track). +// pgTrackVersion returns the current applied migration version. func pgTrackVersion(t *testing.T, env upgradeEnv, table string) int { t.Helper() raw := pgQuery(t, env, fmt.Sprintf( - "SELECT CASE WHEN to_regclass('public.%s') IS NULL THEN 0 ELSE (SELECT COALESCE(MAX(version), 0) FROM public.%s) END", + "SELECT CASE WHEN to_regclass('public.%s') IS NULL THEN 0 ELSE (SELECT COALESCE(MAX(version_id), 0) FROM public.%s WHERE is_applied) END", table, table)) return parseInt(t, raw, table+" version") } @@ -122,75 +102,40 @@ func buildCleanInstallSchema(t *testing.T, env upgradeEnv, dbName string, vector // the time cleanups run, so a normal kubectl call here would always error. t.Cleanup(func() { dropDatabaseBestEffort(env, dbName) }) - localPort, stop := startPortForward(t, env, postgresServiceName, 5432) - defer stop() - - url := fmt.Sprintf("postgres://kagent:kagent@127.0.0.1:%d/%s?sslmode=disable", localPort, dbName) - require.NoError(t, migrations.RunUp(t.Context(), url, migrations.BuiltinSources(vectorEnabled)), - "apply embedded migrations to clean reference database %s", dbName) + applyEmbeddedMigrations(t, env, dbName, vectorEnabled) return pgSchemaDump(t, env, dbName) } -// dropDatabaseBestEffort removes a scratch database, ignoring all errors. It -// uses its own background context so it still runs during test teardown, after -// t.Context() has been canceled, and never fails the test. -func dropDatabaseBestEffort(env upgradeEnv, database string) { - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - podOut, err := exec.CommandContext(ctx, "kubectl", - "--context", env.kubeContext, "get", "pods", - "-n", env.namespace, "-l", postgresSelector, - "-o", "jsonpath={.items[0].metadata.name}", - ).Output() - if err != nil { - return - } - pod := strings.TrimSpace(string(podOut)) - if pod == "" { - return - } - _ = exec.CommandContext(ctx, "kubectl", - "--context", env.kubeContext, "exec", "-n", env.namespace, pod, "-c", postgresContainer, "--", - "psql", "-U", "kagent", "-d", "kagent", "-tAc", - fmt.Sprintf("DROP DATABASE IF EXISTS %s WITH (FORCE)", database), - ).Run() -} - -// migrateTrackTo drives one track to a target version using the embedded -// migration files, standing in for `kagent db migrate goto` until that CLI -// exists. golang-migrate's Migrate moves up or down to the target; a target of -// 0 means roll the track all the way down. -func migrateTrackTo(t *testing.T, url string, track migrationTrack, target int) { +func applyEmbeddedMigrations(t *testing.T, env upgradeEnv, database string, vectorEnabled bool) { t.Helper() - src, err := iofs.New(migrations.FS, track.dir) - require.NoError(t, err, "open embedded %s migrations", track.name) - - db, err := sql.Open("pgx", url) - require.NoError(t, err, "open db for %s track", track.name) + localPort, stop := startPortForward(t, env, postgresServiceName, 5432) + defer stop() - driver, err := migratepgx.WithInstance(db, &migratepgx.Config{MigrationsTable: track.trackingTable}) - require.NoError(t, err, "build migrate driver for %s track", track.name) + url := fmt.Sprintf("postgres://kagent:kagent@127.0.0.1:%d/%s?sslmode=disable", localPort, database) + require.NoError(t, migrations.RunUp(t.Context(), url, migrations.BuiltinSources(vectorEnabled)), + "apply embedded migrations to database %s", database) +} - m, err := migrate.NewWithInstance("iofs", src, "pgx", driver) - require.NoError(t, err, "build migrator for %s track", track.name) - defer m.Close() +func migrateEmbeddedSourcesTo(t *testing.T, env upgradeEnv, targets map[string]int, vectorEnabled bool) { + t.Helper() - if target == 0 { - err = m.Down() - } else { - err = m.Migrate(uint(target)) - } - if err != nil && !errors.Is(err, migrate.ErrNoChange) { - require.NoError(t, err, "migrate %s track to version %d", track.name, target) + localPort, stop := startPortForward(t, env, postgresServiceName, 5432) + defer stop() + url := fmt.Sprintf("postgres://kagent:kagent@127.0.0.1:%d/kagent?sslmode=disable", localPort) + + for _, source := range slices.Backward(migrations.BuiltinSources(vectorEnabled)) { + target, ok := targets[source.Name] + require.True(t, ok, "missing rollback target for migration source %s", source.Name) + err := migrations.WithProvider(t.Context(), url, source, func(provider *goose.Provider) error { + _, err := provider.DownTo(t.Context(), int64(target)) + return err + }) + require.NoError(t, err, "roll back %s migrations to version %d", source.Name, target) } } -// scaleController scales the controller deployment and, for a scale to zero, -// waits until its pods are gone so a booting pod cannot re-apply migrations -// during a schema reversal (the design's scale-to-zero reversal recipe). func scaleController(t *testing.T, env upgradeEnv, replicas int) { t.Helper() @@ -199,7 +144,6 @@ func scaleController(t *testing.T, env upgradeEnv, replicas int) { "-n", env.namespace, fmt.Sprintf("--replicas=%d", replicas), ) - if replicas == 0 { require.Eventually(t, func() bool { pods, err := podNamesForSelectorE(t, env, controllerSelector) @@ -207,7 +151,6 @@ func scaleController(t *testing.T, env upgradeEnv, replicas int) { }, 2*time.Minute, 2*time.Second, "controller pods did not terminate after scale to zero") return } - kubectl(t, env, 3*time.Minute, "rollout", "status", "deployment/kagent-controller", "-n", env.namespace, @@ -215,11 +158,36 @@ func scaleController(t *testing.T, env upgradeEnv, replicas int) { ) } +// dropDatabaseBestEffort removes a scratch database, ignoring all errors. It +// uses its own background context so it still runs during test teardown, after +// t.Context() has been canceled, and never fails the test. +func dropDatabaseBestEffort(env upgradeEnv, database string) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + podOut, err := exec.CommandContext(ctx, "kubectl", + "--context", env.kubeContext, "get", "pods", + "-n", env.namespace, "-l", postgresSelector, + "-o", "jsonpath={.items[0].metadata.name}", + ).Output() + if err != nil { + return + } + pod := strings.TrimSpace(string(podOut)) + if pod == "" { + return + } + _ = exec.CommandContext(ctx, "kubectl", + "--context", env.kubeContext, "exec", "-n", env.namespace, pod, "-c", postgresContainer, "--", + "psql", "-U", "kagent", "-d", "kagent", "-tAc", + fmt.Sprintf("DROP DATABASE IF EXISTS %s WITH (FORCE)", database), + ).Run() +} + var forwardingPortRE = regexp.MustCompile(`Forwarding from 127\.0\.0\.1:(\d+)`) // startPortForward opens a kubectl port-forward to a service and returns the -// chosen local port plus a stop function. Used so the test process can drive -// golang-migrate against the in-cluster Postgres directly. +// chosen local port and a stop function. func startPortForward(t *testing.T, env upgradeEnv, service string, remotePort int) (int, func()) { t.Helper() diff --git a/go/core/test/upgrade/upgrade_test.go b/go/core/test/upgrade/upgrade_test.go index ef6e3f55f..7ebdc5feb 100644 --- a/go/core/test/upgrade/upgrade_test.go +++ b/go/core/test/upgrade/upgrade_test.go @@ -1,10 +1,4 @@ -// Package upgrade holds the database upgrade/rollback compatibility tests. They -// are deliberately separate from test/e2e: they mutate the cluster they run -// against — installing a prior release, upgrading it in place, and reverse- -// migrating the schema — so they cannot share the e2e suite's cluster and must -// run against a throwaway one (see the run-upgrade-tests / run-rolling-upgrade- -// tests make targets). Each test self-skips unless its RUN_*_TESTS env var is -// set, so a plain `go test ./...` compiles but does not execute them. +// Package upgrade holds the database upgrade compatibility tests. package upgrade import ( @@ -15,7 +9,6 @@ import ( "os" "os/exec" "path/filepath" - "slices" "strconv" "strings" "testing" @@ -33,7 +26,7 @@ const ( controllerContainer = "controller" controllerServiceName = "kagent-controller" - controllerAPIPort = 8083 + controllerGRPCPort = 8084 ) type upgradeEnv struct { @@ -49,7 +42,6 @@ type upgradeEnv struct { type postgresMigrationState struct { version int - dirty bool } func TestUpgrade(t *testing.T) { @@ -59,16 +51,14 @@ func TestUpgrade(t *testing.T) { env := loadUpgradeEnv(t) seed := fmt.Sprintf("%d", time.Now().UnixNano()) - seedAgentID := "upgrade-seed-agent-" + seed - seedUserID := "upgrade-seed-user-" + seed - seedSessionID := "upgrade-seed-session-" + seed - seedEventID := "upgrade-seed-event-" + seed - seedTaskID := "upgrade-seed-task-" + seed - seedPushID := "upgrade-seed-push-" + seed seedToolID := "upgrade-seed-tool-" + seed seedToolServerName := "upgrade-seed-toolserver-" + seed seedGroupKind := "upgrade.seed/v1/Canary" seedCanaryCounts := map[string]int{} + seedCanaryQueries := map[string]string{ + "tool": fmt.Sprintf("SELECT count(*) FROM tool WHERE id = %s AND server_name = %s AND group_kind = %s", pgQuote(seedToolID), pgQuote(seedToolServerName), pgQuote(seedGroupKind)), + "toolserver": fmt.Sprintf("SELECT count(*) FROM toolserver WHERE name = %s AND group_kind = %s", pgQuote(seedToolServerName), pgQuote(seedGroupKind)), + } // The controller image embeds the migration files. Comparing the DB // state to this version proves the upgraded pod actually applied the // migration set shipped in the target build. @@ -76,74 +66,76 @@ func TestUpgrade(t *testing.T) { t.Logf("upgrade test: %s -> %s (registry=%s, kubeContext=%s)", env.upgradeFromVersion, env.version, env.dockerRegistry, env.kubeContext) + waitForReadyPods(t, env, postgresSelector, 3*time.Minute) + waitForPostgresSchema(t, env, 3*time.Minute) + if !hasGooseMigrationTable(t, env) { + t.Skip("the baseline release does not use Goose") + } var pgBaselineState postgresMigrationState var baselineVectorVersion int - // cleanTargetSchema is the previous release's freshly-installed schema. The - // rollback round-trip below asserts the reversed database matches it exactly. - var cleanTargetSchema string - // cleanHeadSchema is an independent clean install of the current build's - // migrations. The post-upgrade database must match it exactly. - var cleanHeadSchema string + var cleanPreviousSchema string if !t.Run("seed baseline data before upgrade", func(t *testing.T) { - waitForReadyPods(t, env, postgresSelector, 3*time.Minute) - waitForPostgresAgentTable(t, env, 3*time.Minute) - - // A dirty baseline means a previous migration failed; continuing would - // make any post-upgrade failure ambiguous rather than a regression signal. pgBaselineState = pgMigrationState(t, env) - require.False(t, pgBaselineState.dirty, "baseline Postgres migrations are dirty") baselineVectorVersion = pgTrackVersion(t, env, "vector_schema_migrations") - t.Logf("baseline Postgres schema_migrations version: %d dirty=%t vector=%d (target=%d)", - pgBaselineState.version, pgBaselineState.dirty, baselineVectorVersion, targetCoreVersion) - - // Capture the clean target schema before any seeding or upgrade. The - // freshly-installed previous release is, by definition, a clean target - // install, so its schema is the reversal reference. schema-only dumps - // exclude row data, so seeding afterward does not perturb it. - cleanTargetSchema = pgSchemaDump(t, env, "kagent") + cleanPreviousSchema = pgSchemaDump(t, env, "kagent") + t.Logf("baseline Postgres schema_migrations version: %d vector=%d (target=%d)", + pgBaselineState.version, baselineVectorVersion, targetCoreVersion) // Seed a small cross-section of stable tables. These rows are not // meant to validate every future migration's semantics; they are canaries // for accidental table drops, destructive rewrites, and key/index changes // that lose existing customer data during an upgrade. - pgExec(t, env, fmt.Sprintf("INSERT INTO agent (id, type) VALUES (%s, 'Deployment')", pgQuote(seedAgentID))) - pgExec(t, env, fmt.Sprintf("INSERT INTO session (id, user_id, name, agent_id, source) VALUES (%s, %s, 'upgrade canary session', %s, 'upgrade-test')", - pgQuote(seedSessionID), pgQuote(seedUserID), pgQuote(seedAgentID))) - pgExec(t, env, fmt.Sprintf("INSERT INTO event (id, user_id, session_id, data) VALUES (%s, %s, %s, '{}')", - pgQuote(seedEventID), pgQuote(seedUserID), pgQuote(seedSessionID))) - pgExec(t, env, fmt.Sprintf("INSERT INTO task (id, session_id, data) VALUES (%s, %s, '{}')", - pgQuote(seedTaskID), pgQuote(seedSessionID))) - pgExec(t, env, fmt.Sprintf("INSERT INTO push_notification (id, task_id, data) VALUES (%s, %s, '{}')", - pgQuote(seedPushID), pgQuote(seedTaskID))) - pgExec(t, env, fmt.Sprintf("INSERT INTO feedback (user_id, feedback_text) VALUES (%s, 'pre-upgrade feedback')", pgQuote(seedUserID))) pgExec(t, env, fmt.Sprintf("INSERT INTO tool (id, server_name, group_kind, description) VALUES (%s, %s, %s, 'upgrade canary tool')", pgQuote(seedToolID), pgQuote(seedToolServerName), pgQuote(seedGroupKind))) pgExec(t, env, fmt.Sprintf("INSERT INTO toolserver (name, group_kind, description) VALUES (%s, %s, 'upgrade canary toolserver')", pgQuote(seedToolServerName), pgQuote(seedGroupKind))) - seedAgents := pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM agent WHERE id = %s", pgQuote(seedAgentID))) - require.GreaterOrEqual(t, seedAgents, 1, "expected seeded agent row") - t.Logf("seeded agent rows: %d", seedAgents) - - seedCanaryCounts = map[string]int{ - "agent": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM agent WHERE id = %s", pgQuote(seedAgentID))), - "session": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM session WHERE id = %s AND user_id = %s", pgQuote(seedSessionID), pgQuote(seedUserID))), - "event": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM event WHERE id = %s AND user_id = %s", pgQuote(seedEventID), pgQuote(seedUserID))), - "task": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM task WHERE id = %s", pgQuote(seedTaskID))), - "push_notification": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM push_notification WHERE id = %s", pgQuote(seedPushID))), - "feedback": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM feedback WHERE user_id = %s", pgQuote(seedUserID))), - "tool": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM tool WHERE id = %s AND server_name = %s AND group_kind = %s", pgQuote(seedToolID), pgQuote(seedToolServerName), pgQuote(seedGroupKind))), - "toolserver": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM toolserver WHERE name = %s AND group_kind = %s", pgQuote(seedToolServerName), pgQuote(seedGroupKind))), - } - for table, count := range seedCanaryCounts { + for table, query := range seedCanaryQueries { + count := pgQueryInt(t, env, query) require.GreaterOrEqual(t, count, 1, "expected seeded %s canary row", table) + seedCanaryCounts[table] = count } }) { return } + previousGoDir := checkoutPreviousRelease(t, env) + vectorEnabled := baselineVectorVersion > 0 + if !t.Run("apply target migrations", func(t *testing.T) { + applyEmbeddedMigrations(t, env, "kagent", vectorEnabled) + pgPostState := pgMigrationState(t, env) + require.Equal(t, targetCoreVersion, pgPostState.version, + "Postgres migrations did not reach the target embedded migration version") + }) { + return + } + + if !t.Run("previous release serves against target migrations", func(t *testing.T) { + runInvokeE2E(t, env, previousGoDir, "target migrations") + }) { + return + } + + if !t.Run("previous release restarts against migrated schema", func(t *testing.T) { + kubectl(t, env, time.Minute, + "rollout", "restart", "deployment/kagent-controller", + "-n", env.namespace, + ) + kubectl(t, env, 3*time.Minute, + "rollout", "status", "deployment/kagent-controller", + "-n", env.namespace, + "--timeout=3m", + ) + pod := newestPodNameForSelector(t, env, controllerSelector) + restarts := podContainerRestartCount(t, env, pod, controllerContainer) + require.Zero(t, restarts, + "previous-release controller pod %s crash-looped against the migrated schema", pod) + }) { + return + } + if !t.Run("upgrade with helm", func(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 10*time.Minute) defer cancel() @@ -189,169 +181,100 @@ func TestUpgrade(t *testing.T) { } t.Run("verify seeded data survived migrations", func(t *testing.T) { - // These checks are migration plumbing checks: the version cannot regress, - // the migration table must be clean, and the upgraded controller must - // have reached the latest core migration embedded in this test build. pgPostState := pgMigrationState(t, env) - require.False(t, pgPostState.dirty, "post-upgrade Postgres migrations are dirty") require.GreaterOrEqual(t, pgPostState.version, pgBaselineState.version, "Postgres migration version regressed") require.Equal(t, targetCoreVersion, pgPostState.version, "Postgres migrations did not reach the target embedded migration version") - t.Logf("Postgres schema_migrations version: %d -> %d dirty=%t", - pgBaselineState.version, pgPostState.version, pgPostState.dirty) + t.Logf("Postgres migration version: %d -> %d", + pgBaselineState.version, pgPostState.version) // Keep the schema invariant intentionally broad and cheap: core // tables should still exist before we ask more specific questions about // the seeded rows below. requirePostgresTablesExist(t, env, - "agent", - "session", - "event", - "task", - "push_notification", - "feedback", "tool", "toolserver", ) - postAgents := pgQueryInt(t, env, - fmt.Sprintf("SELECT count(*) FROM agent WHERE id = %s AND workload_type = 'deployment'", pgQuote(seedAgentID))) - require.GreaterOrEqual(t, postAgents, 1, - "seeded agent row missing or not backfilled to workload_type='deployment' after upgrade") - - postFeedback := pgQueryInt(t, env, - fmt.Sprintf("SELECT count(*) FROM feedback WHERE user_id = %s", pgQuote(seedUserID))) - require.GreaterOrEqual(t, postFeedback, 1, "seeded feedback row did not survive the upgrade migrations") - - postCanaryCounts := map[string]int{ - "agent": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM agent WHERE id = %s", pgQuote(seedAgentID))), - "session": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM session WHERE id = %s AND user_id = %s", pgQuote(seedSessionID), pgQuote(seedUserID))), - "event": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM event WHERE id = %s AND user_id = %s", pgQuote(seedEventID), pgQuote(seedUserID))), - "task": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM task WHERE id = %s", pgQuote(seedTaskID))), - "push_notification": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM push_notification WHERE id = %s", pgQuote(seedPushID))), - "feedback": postFeedback, - "tool": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM tool WHERE id = %s AND server_name = %s AND group_kind = %s", pgQuote(seedToolID), pgQuote(seedToolServerName), pgQuote(seedGroupKind))), - "toolserver": pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM toolserver WHERE name = %s AND group_kind = %s", pgQuote(seedToolServerName), pgQuote(seedGroupKind))), - } for table, before := range seedCanaryCounts { // The generic canaries only assert non-regression. Future migrations // that intentionally transform data should still add targeted // assertions for their expected post-upgrade shape. - require.GreaterOrEqual(t, postCanaryCounts[table], before, + require.GreaterOrEqual(t, pgQueryInt(t, env, seedCanaryQueries[table]), before, "%s canary row count decreased across upgrade", table) } }) - vectorEnabled := baselineVectorVersion > 0 - if !t.Run("verify upgraded schema matches a clean install", func(t *testing.T) { // Build an independent clean install of the current build's migrations // and require the upgraded database to be structurally identical. This // catches upgrade paths that leave residue a fresh install would not. - cleanHeadSchema = buildCleanInstallSchema(t, env, "clean_head_"+seed, vectorEnabled) + cleanHeadSchema := buildCleanInstallSchema(t, env, "clean_head_"+seed, vectorEnabled) upgradedSchema := pgSchemaDump(t, env, "kagent") - - if cleanHeadSchema == cleanTargetSchema { - t.Log("no schema change between target and HEAD; the round-trip is a structural no-op until a new migration lands") - } require.Equal(t, cleanHeadSchema, upgradedSchema, "upgraded schema diverged from a clean install of the current build") }) { return } - t.Run("post-upgrade invoke (HEAD)", func(t *testing.T) { + if !t.Run("post-upgrade invoke (HEAD)", func(t *testing.T) { // The HEAD controller is serving on the migrated schema. Exercise the // current code's real query paths against it (deploy + invoke an agent), // not just the psql-level checks above. runInvokeE2E(t, env, filepath.Join(env.repoRoot, "go"), "post-upgrade") - }) + }) { + return + } - if !t.Run("previous release boots against the upgraded schema", func(t *testing.T) { - // A deployment-only rollback puts the previous binary back while HEAD's - // schema is still applied, so the binary is now behind its database. It - // must boot and serve rather than crash-loop trying to migrate down — the - // ahead-schema tolerance shipped in 0.9.9 (#1963). Reinstalling the - // previous release downgrades the running controller to the old image with - // the upgraded schema left in place; helm --wait fails here if it does not - // become Ready. + if !t.Run("roll back application to previous release", func(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 10*time.Minute) defer cancel() out, err := installPreviousReleaseCommand(ctx, env).CombinedOutput() require.NoError(t, err, "rolling back to the previous release failed:\n%s", string(out)) - kubectl(t, env, 3*time.Minute, "rollout", "status", "deployment/kagent-controller", "-n", env.namespace, "--timeout=3m", ) pod := newestPodNameForSelector(t, env, controllerSelector) - restarts := podContainerRestartCount(t, env, pod, controllerContainer) - require.Zero(t, restarts, - "previous-release controller pod %s crash-looped against the upgraded schema", pod) - t.Logf("previous-release controller %s restarts=%d against the upgraded schema", pod, restarts) + require.Zero(t, podContainerRestartCount(t, env, pod, controllerContainer), + "previous-release controller pod %s crash-looped against the target schema", pod) }) { return } - t.Run("contraction invoke (previous release)", func(t *testing.T) { - // The previous release is now serving against the ahead (HEAD) schema. - // Run its own invoke e2e slice (from the worktree at its tag) against it: - // old code's real queries against the new schema — the contraction - // property raw SQL cannot reach. - runInvokeE2E(t, env, prevTreeGoDir(), "contraction") - }) + if !t.Run("previous release invokes against target schema", func(t *testing.T) { + runInvokeE2E(t, env, previousGoDir, "application rollback") + }) { + return + } - t.Run("reverse schema to target", func(t *testing.T) { - // Scale the controller to zero so no booting pod re-applies migrations - // while we reverse the schema (the design's scale-to-zero recipe). + if !t.Run("reverse schema to previous release", func(t *testing.T) { scaleController(t, env, 0) - - localPort, stop := startPortForward(t, env, postgresServiceName, 5432) - defer stop() - dbURL := fmt.Sprintf("postgres://kagent:kagent@127.0.0.1:%d/kagent?sslmode=disable", localPort) - - // Reverse each track to the target release's version, in reverse - // registration order (vector before core). This stands in for - // `kagent db migrate goto --release ` until that CLI exists; it - // exercises every down file between HEAD and the target. - targets := map[string]int{ + migrateEmbeddedSourcesTo(t, env, map[string]int{ "core": pgBaselineState.version, "vector": baselineVectorVersion, + }, vectorEnabled) + + require.Equal(t, pgBaselineState.version, pgMigrationState(t, env).version, + "core migrations did not return to the previous-release version") + require.Equal(t, baselineVectorVersion, pgTrackVersion(t, env, "vector_schema_migrations"), + "vector migrations did not return to the previous-release version") + require.Equal(t, cleanPreviousSchema, pgSchemaDump(t, env, "kagent"), + "reversed schema diverged from a clean previous-release install") + for table, before := range seedCanaryCounts { + require.GreaterOrEqual(t, pgQueryInt(t, env, seedCanaryQueries[table]), before, + "%s row count decreased during schema rollback", table) } - for _, track := range slices.Backward(migrationTracks) { - migrateTrackTo(t, dbURL, track, targets[track.name]) - } - - // Migration bookkeeping is back at the target versions and clean. - reverted := pgMigrationState(t, env) - revertedVector := pgTrackVersion(t, env, "vector_schema_migrations") - t.Logf("reversed Postgres schema_migrations version: %d -> %d dirty=%t vector=%d (target=%d)", - targetCoreVersion, reverted.version, reverted.dirty, revertedVector, baselineVectorVersion) - require.False(t, reverted.dirty, "reverted Postgres migrations are dirty") - require.Equal(t, pgBaselineState.version, reverted.version, "core track not reversed to target version") - require.Equal(t, baselineVectorVersion, revertedVector, - "vector track not reversed to target version") - - // Schema matches a clean target install, and the seeded rows survived - // the down migrations. - require.Equal(t, cleanTargetSchema, pgSchemaDump(t, env, "kagent"), - "reversed schema diverged from a clean target install") - require.GreaterOrEqual(t, pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM agent WHERE id = %s", pgQuote(seedAgentID))), 1, - "seeded agent row did not survive the rollback") - require.GreaterOrEqual(t, pgQueryInt(t, env, fmt.Sprintf("SELECT count(*) FROM feedback WHERE user_id = %s", pgQuote(seedUserID))), 1, - "seeded feedback row did not survive the rollback") - }) + }) { + return + } - t.Run("post-rollback invoke (previous release)", func(t *testing.T) { - // Bring the previous controller back up on the reverted schema (the reverse - // step scaled it to zero). The schema now matches the old binary, so it - // boots without migrating. Then run its invoke e2e slice — old code fully - // serving on the rolled-back schema. + t.Run("previous release invokes after schema rollback", func(t *testing.T) { scaleController(t, env, 1) - runInvokeE2E(t, env, prevTreeGoDir(), "post-rollback") + runInvokeE2E(t, env, previousGoDir, "schema rollback") }) } @@ -373,12 +296,6 @@ func helmUpgradeCommand(ctx context.Context, env upgradeEnv) *exec.Cmd { return cmd } -// installPreviousReleaseCommand returns the command that reinstalls the -// previously-released charts over the current deployment — i.e. a rollback of -// the app to the prior version. It reuses the repo's install-previous-release -// target (helm upgrade --install of the published prior kagent-crds and kagent -// from the OCI registry, with --wait), so a controller that cannot start -// against the current schema surfaces as a helm wait timeout. func installPreviousReleaseCommand(ctx context.Context, env upgradeEnv) *exec.Cmd { cmd := exec.CommandContext(ctx, "make", "-C", env.repoRoot, "install-previous-release") cmd.Dir = env.repoRoot @@ -448,13 +365,13 @@ func waitForReadyPods(t *testing.T, env upgradeEnv, selector string, timeout tim ) } -func waitForPostgresAgentTable(t *testing.T, env upgradeEnv, timeout time.Duration) { +func waitForPostgresSchema(t *testing.T, env upgradeEnv, timeout time.Duration) { t.Helper() require.Eventually(t, func() bool { - out, err := pgQueryE(t, env, "SELECT to_regclass('public.agent') IS NOT NULL") + out, err := pgQueryE(t, env, "SELECT to_regclass('public.tool') IS NOT NULL") return err == nil && out == "t" - }, timeout, 5*time.Second, "agent table did not appear in the baseline Postgres schema") + }, timeout, 5*time.Second, "baseline Postgres schema did not appear") } func pgExec(t *testing.T, env upgradeEnv, query string) { @@ -506,28 +423,20 @@ func pgMigrationState(t *testing.T, env upgradeEnv) postgresMigrationState { // pgMigrationStateE is the error-returning core of pgMigrationState, for use // inside require.Eventually conditions (see pgQueryE). func pgMigrationStateE(t *testing.T, env upgradeEnv) (postgresMigrationState, error) { - raw, err := pgQueryE(t, env, "SELECT CASE WHEN to_regclass('public.schema_migrations') IS NULL THEN '0,false' ELSE (SELECT concat(COALESCE(MAX(version), 0), ',', COALESCE(bool_or(dirty), false)) FROM public.schema_migrations) END") + raw, err := pgQueryE(t, env, "SELECT CASE WHEN to_regclass('public.schema_migrations') IS NULL THEN 0 ELSE (SELECT COALESCE(MAX(version_id), 0) FROM public.schema_migrations WHERE is_applied) END") if err != nil { return postgresMigrationState{}, err } - parts := strings.Split(raw, ",") - if len(parts) != 2 { - return postgresMigrationState{}, fmt.Errorf("parse schema_migrations state %q", raw) - } - version, err := strconv.Atoi(strings.TrimSpace(parts[0])) + version, err := strconv.Atoi(strings.TrimSpace(raw)) if err != nil { - return postgresMigrationState{}, fmt.Errorf("parse schema_migrations version %q: %w", parts[0], err) - } - var dirty bool - switch strings.TrimSpace(parts[1]) { - case "t", "true": - dirty = true - case "f", "false": - dirty = false - default: - return postgresMigrationState{}, fmt.Errorf("parse schema_migrations dirty %q", parts[1]) + return postgresMigrationState{}, fmt.Errorf("parse Goose migration version %q: %w", raw, err) } - return postgresMigrationState{version: version, dirty: dirty}, nil + return postgresMigrationState{version: version}, nil +} + +func hasGooseMigrationTable(t *testing.T, env upgradeEnv) bool { + t.Helper() + return pgQuery(t, env, "SELECT EXISTS(SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'schema_migrations' AND column_name = 'version_id')") == "t" } func requirePostgresTablesExist(t *testing.T, env upgradeEnv, tables ...string) { @@ -646,7 +555,7 @@ func latestCoreMigrationVersion(t *testing.T) int { maxVersion := 0 for _, entry := range entries { name := entry.Name() - if entry.IsDir() || !strings.HasSuffix(name, ".up.sql") { + if entry.IsDir() || !strings.HasSuffix(name, ".sql") { continue } versionPart, _, ok := strings.Cut(name, "_") diff --git a/go/go.mod b/go/go.mod index 9d4b464d7..2920f2a62 100644 --- a/go/go.mod +++ b/go/go.mod @@ -23,7 +23,6 @@ require ( github.com/go-logr/logr v1.4.4 github.com/go-logr/zapr v1.3.0 github.com/golang-jwt/jwt/v5 v5.3.1 - github.com/golang-migrate/migrate/v4 v4.19.1 github.com/google/go-containerregistry v0.21.9 github.com/google/jsonschema-go v0.4.3 // api dependencies @@ -43,6 +42,7 @@ require ( github.com/pelletier/go-toml/v2 v2.4.3 github.com/pgvector/pgvector-go v0.4.1 github.com/pgvector/pgvector-go/pgx v0.4.1 + github.com/pressly/goose/v3 v3.27.3 github.com/prometheus/client_golang v1.24.1 github.com/spf13/afero v1.15.0 github.com/spf13/cobra v1.10.2 @@ -272,7 +272,6 @@ require ( github.com/huandu/xstrings v1.5.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/invopop/jsonschema v0.14.0 // indirect - github.com/jackc/pgerrcode v0.0.0-20250907135507-afb5586c32a6 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect @@ -317,6 +316,7 @@ require ( github.com/mattn/go-isatty v0.0.24 // indirect github.com/mattn/go-localereader v0.0.1 // indirect github.com/mattn/go-runewidth v0.0.28 // indirect + github.com/mfridman/interpolate v0.0.2 // indirect github.com/mgechev/revive v1.15.0 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect @@ -378,6 +378,7 @@ require ( github.com/securego/gosec/v2 v2.26.1 // indirect github.com/segmentio/asm v1.2.1 // indirect github.com/segmentio/encoding v0.5.4 // indirect + github.com/sethvargo/go-retry v0.4.0 // indirect github.com/shirou/gopsutil/v4 v4.26.7 // indirect github.com/shopspring/decimal v1.4.0 // indirect github.com/sirupsen/logrus v1.10.1 // indirect diff --git a/go/go.sum b/go/go.sum index b12b8050c..afdc847e7 100644 --- a/go/go.sum +++ b/go/go.sum @@ -306,8 +306,6 @@ github.com/denis-tingaikin/go-header v0.5.0/go.mod h1:mMenU5bWrok6Wl2UsZjy+1okeg github.com/desertbit/timer v0.0.0-20180107155436-c41aec40b27f h1:U5y3Y5UE0w7amNe7Z5G/twsBW0KEalRQXZzf8ufSh9I= github.com/desertbit/timer v0.0.0-20180107155436-c41aec40b27f/go.mod h1:xH/i4TFMt8koVQZ6WFms69WAsDWr2XsYL3Hkl7jkoLE= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= -github.com/dhui/dktest v0.4.6 h1:+DPKyScKSEp3VLtbMDHcUq6V5Lm5zfZZVb0Sk7Ahom4= -github.com/dhui/dktest v0.4.6/go.mod h1:JHTSYDtKkvFNFHJKqCzVzqXecyv+tKt8EzceOmQOgbU= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/dlclark/regexp2 v1.12.0 h1:0j4c5qQmnC6XOWNjP3PIXURXN2gWx76rd3KvgdPkCz8= @@ -316,8 +314,6 @@ github.com/dnaeon/go-vcr v1.2.0 h1:zHCHvJYTMh1N7xnV7zf1m1GPBF9Ad0Jk/whtQ1663qI= github.com/dnaeon/go-vcr v1.2.0/go.mod h1:R4UdLID7HZT3taECzJs4YgbbH6PIGXB6W/sc5OLb6RQ= github.com/docker/cli v29.7.2+incompatible h1:dlkwallR8XqfeVnA2ELEhdwvb4lsSwuB4IgsG8Q9cLY= github.com/docker/cli v29.7.2+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= -github.com/docker/docker v28.3.3+incompatible h1:Dypm25kh4rmk49v1eiVbsAtpAsYURjYkaKubwuBdxEI= -github.com/docker/docker v28.3.3+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/docker-credential-helpers v0.9.8 h1:bIREROb7So6PRlq6KTtdS9MPEjC29OQRkFNlvK2OX8Q= github.com/docker/docker-credential-helpers v0.9.8/go.mod h1:v1S+hepowrQXITkEfw6o4+BMbGot02wiKpzWhGUZK6c= github.com/docker/go-connections v0.8.1 h1:JibmG5hULs5qXSr/cp/w3Pw5fZuStt4MOHMUExb29/M= @@ -498,12 +494,8 @@ github.com/gogo/googleapis v1.1.0/go.mod h1:gf4bu3Q80BeJ6H1S1vYPm8/ELATdvryBaNFG github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.0/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= -github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= -github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= -github.com/golang-migrate/migrate/v4 v4.19.1 h1:OCyb44lFuQfYXYLx1SCxPZQGU7mcaZ7gH9yH4jSFbBA= -github.com/golang-migrate/migrate/v4 v4.19.1/go.mod h1:CTcgfjxhaUtsLipnLoQRWCrjYXycRz/g5+RWDuYgPrE= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= @@ -666,8 +658,6 @@ github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLf github.com/influxdata/influxdb1-client v0.0.0-20191209144304-8bf82d3c094d/go.mod h1:qj24IKcXYK6Iy9ceXlo3Tc+vtHo9lIhSX5JddghvEPo= github.com/invopop/jsonschema v0.14.0 h1:MHQqLhvpNUZfw+hM3AZDYK7jxO8FZoQeQM77g8iyZjg= github.com/invopop/jsonschema v0.14.0/go.mod h1:ygm6C2EaVNMBDPpaPlnOA2pFAxBnxGjFlMZABxm9n2I= -github.com/jackc/pgerrcode v0.0.0-20250907135507-afb5586c32a6 h1:D/V0gu4zQ3cL2WKeVNVM4r2gLxGGf6McLwgXzRTo2RQ= -github.com/jackc/pgerrcode v0.0.0-20250907135507-afb5586c32a6/go.mod h1:a/s9Lp5W7n/DD0VrVoyJ00FbP2ytTPDVOivvn2bMlds= github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= @@ -819,6 +809,8 @@ github.com/mattn/go-sqlite3 v1.14.24/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxU github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/mdelapenya/tlscert v0.2.0 h1:7H81W6Z/4weDvZBNOfQte5GpIMo0lGYEeWbkGp5LJHI= github.com/mdelapenya/tlscert v0.2.0/go.mod h1:O4njj3ELLnJjGdkN7M/vIVCpZ+Cf0L6muqOG4tLSl8o= +github.com/mfridman/interpolate v0.0.2 h1:pnuTK7MQIxxFz1Gr+rjSIx9u7qVjf5VOoM/u6BbAxPY= +github.com/mfridman/interpolate v0.0.2/go.mod h1:p+7uk6oE07mpE/Ik1b8EckO0O4ZXiGAfshKBWLUM9Xg= github.com/mgechev/revive v1.15.0 h1:vJ0HzSBzfNyPbHKolgiFjHxLek9KUijhqh42yGoqZ8Q= github.com/mgechev/revive v1.15.0/go.mod h1:LlAKO3QQe9OJ0pVZzI2GPa8CbXGZ/9lNpCGvK4T/a8A= github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg= @@ -871,8 +863,6 @@ github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee h1:W5t00kpgFd github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/moricho/tparallel v0.3.2 h1:odr8aZVFA3NZrNybggMkYO3rgPRcqjeQUlBBFVxKHTI= github.com/moricho/tparallel v0.3.2/go.mod h1:OQ+K3b4Ln3l2TZveGCywybl68glfLEwFGqvnjok8b+U= -github.com/morikuni/aec v1.0.0 h1:nP9CBfwrvYnBRgY6qfDQkygYDmYwOilePFkwzv4dU8A= -github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 h1:ZK8zHtRHOkbHy6Mmr5D264iyp3TiX5OmNcI5cIARiQI= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6/go.mod h1:CJlz5H+gyd6CUWT45Oy4q24RdLyn7Md9Vj2/ldJBSIo= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= @@ -910,8 +900,8 @@ github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:v github.com/ollama/ollama v0.32.15 h1:lnCycypBjS9SoMNeM6FivlYeDRn7mP/zfLG0uJXwmZ4= github.com/ollama/ollama v0.32.15/go.mod h1:Kekx/+OtFZHmqbkVH/QUUDVcMQS+1pg1dcz4Qy7TGn4= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= +github.com/onsi/ginkgo v1.7.0 h1:WSHQ+IS43OoUrWtD1/bbclrwK8TTH5hzp+umCiuxHgs= github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= -github.com/onsi/ginkgo v1.16.4 h1:29JGrr5oVBm5ulCWet69zQkzWipVXIol6ygQUe/EzNc= github.com/onsi/ginkgo/v2 v2.31.0 h1:GtuJos5DFUV9EerYJo8RhYxosYNGvOdDE5haKq6Grfs= github.com/onsi/ginkgo/v2 v2.31.0/go.mod h1:+aXOY+vzZ5mu2iI2HpTZUPmM//oQfsNFX6gU9kNcA44= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= @@ -970,6 +960,8 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI= github.com/power-devops/perfstat v0.0.0-20260805114148-88456608a4f6 h1:jL3a8soXdzuTCcRnKhOmtcsVOObdDTFf4O2B403HPRU= github.com/power-devops/perfstat v0.0.0-20260805114148-88456608a4f6/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE= +github.com/pressly/goose/v3 v3.27.3 h1:pIglVHjw99r4e/hDHHwbl9vfOsDMqUokfkXo6+n/RxA= +github.com/pressly/goose/v3 v3.27.3/go.mod h1:Dag+xpV6o20HR2LFY1j0q6MDwc3f7vPUFDA77R+0yGY= github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= github.com/prometheus/client_golang v0.9.3-0.20190127221311-3c4408c8b829/go.mod h1:p2iRAGwDERtqlqzRXnrOVns+ignqQo//hLXqYxZYVNs= github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo= @@ -1060,6 +1052,8 @@ github.com/segmentio/encoding v0.5.4 h1:OW1VRern8Nw6ITAtwSZ7Idrl3MXCFwXHPgqESYfv github.com/segmentio/encoding v0.5.4/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= +github.com/sethvargo/go-retry v0.4.0 h1:9qy1OoIAxBL+gBYnkTnTnWle5wlfsXQlwRzIbbpdqPw= +github.com/sethvargo/go-retry v0.4.0/go.mod h1:tvsjdKG6xfiCx4LSiUZ06kcv38xvdVQwv8R6/VnnVWg= github.com/shirou/gopsutil/v4 v4.26.7 h1:IXzpHz/dkMRYAhKkOXr1HB6SuzWU3eoyyeWe7g3bNZc= github.com/shirou/gopsutil/v4 v4.26.7/go.mod h1:5O9FjBiXoTDFatIWjZZosqj4pV0DRtLx598xGbBehzM= github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp81k=