From c26e5cf8a36bba127d7c6dfc402a011ddf6e6b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 15 Aug 2026 14:10:47 +0100 Subject: [PATCH] fix: preserve repository branch templates (#2611) (#2684) --- .../task/repository/sqlite/base_migrations.go | 7 +- ...branch_template_migration_postgres_test.go | 89 +++++++++++++++++ ...worktree_branch_template_migration_test.go | 95 +++++++++++++++++++ .../plan.md | 79 +++++++++++++++ .../task-01-preserve-branch-template.md | 70 ++++++++++++++ docs/specs/INDEX.md | 1 + .../workspaces/worktree-branch-templates.md | 64 +++++++++++++ 7 files changed, 403 insertions(+), 2 deletions(-) create mode 100644 apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_postgres_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_test.go create mode 100644 docs/plans/worktree-branch-template-persistence/plan.md create mode 100644 docs/plans/worktree-branch-template-persistence/task-01-preserve-branch-template.md create mode 100644 docs/specs/workspaces/worktree-branch-templates.md diff --git a/apps/backend/internal/task/repository/sqlite/base_migrations.go b/apps/backend/internal/task/repository/sqlite/base_migrations.go index 0f5b342e01..e0b111aa60 100644 --- a/apps/backend/internal/task/repository/sqlite/base_migrations.go +++ b/apps/backend/internal/task/repository/sqlite/base_migrations.go @@ -215,8 +215,11 @@ func (r *Repository) runMigrations() error { SET provider_host = 'https://github.com' WHERE LOWER(TRIM(provider)) = 'github' AND TRIM(COALESCE(provider_host, '')) = ''`) - r.migrate.Apply("repositories.worktree_branch_template", `ALTER TABLE repositories ADD COLUMN worktree_branch_template TEXT DEFAULT 'feature/{title}-{suffix}'`) - r.migrate.Apply("repositories.worktree_branch_template.backfill", `UPDATE repositories SET worktree_branch_template = COALESCE(NULLIF(TRIM(worktree_branch_prefix), ''), 'feature/') || '{title}-{suffix}'`) + r.migrate.Apply("repositories.worktree_branch_template", `ALTER TABLE repositories ADD COLUMN worktree_branch_template TEXT DEFAULT ''`) + r.migrate.Apply("repositories.worktree_branch_template.backfill", ` + UPDATE repositories + SET worktree_branch_template = COALESCE(NULLIF(TRIM(worktree_branch_prefix), ''), 'feature/') || '{title}-{suffix}' + WHERE TRIM(COALESCE(worktree_branch_template, '')) = ''`) r.migrate.Apply("task_plans.implementation_started_at", `ALTER TABLE task_plans ADD COLUMN implementation_started_at TIMESTAMP`) r.migrate.Apply("task_plans.implementation_started_session_id", `ALTER TABLE task_plans ADD COLUMN implementation_started_session_id TEXT`) r.migrate.Apply("task_plans.implementation_started_by", `ALTER TABLE task_plans ADD COLUMN implementation_started_by TEXT`) diff --git a/apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_postgres_test.go b/apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_postgres_test.go new file mode 100644 index 0000000000..af3731af14 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_postgres_test.go @@ -0,0 +1,89 @@ +package sqlite + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/testutil" +) + +func TestPostgresWorktreeBranchTemplateMigration(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("init postgres schema: %v", err) + } + ctx := context.Background() + seedWorkspaceForWorktreeTemplateMigration(t, repo, "ws-pg-worktree-template") + + custom := &models.Repository{ + ID: "repo-pg-worktree-template-custom", + WorkspaceID: "ws-pg-worktree-template", + Name: "custom-template", + SourceType: "local", + WorktreeBranchPrefix: "feature/", + WorktreeBranchTemplate: "bugfix/{ticket}-{title}", + } + if err := repo.CreateRepository(ctx, custom); err != nil { + t.Fatalf("create custom repository: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("replay custom template migration: %v", err) + } + assertPostgresWorktreeBranchTemplate(t, db, custom.ID, custom.WorktreeBranchTemplate) + + legacyRepositories := []struct { + id string + prefix string + wantTemplate string + }{ + {id: "repo-pg-worktree-template-fix", prefix: " fix/ ", wantTemplate: "fix/{title}-{suffix}"}, + {id: "repo-pg-worktree-template-default", prefix: " ", wantTemplate: "feature/{title}-{suffix}"}, + } + for _, legacy := range legacyRepositories { + if err := repo.CreateRepository(ctx, &models.Repository{ + ID: legacy.id, + WorkspaceID: "ws-pg-worktree-template", + Name: legacy.id, + SourceType: "local", + WorktreeBranchPrefix: legacy.prefix, + }); err != nil { + t.Fatalf("create legacy repository %q: %v", legacy.id, err) + } + } + + if _, err := db.Exec(`ALTER TABLE repositories DROP COLUMN worktree_branch_template`); err != nil { + t.Fatalf("remove template column from legacy schema: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("migrate legacy postgres schema: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("replay legacy postgres migration: %v", err) + } + + for _, legacy := range legacyRepositories { + assertPostgresWorktreeBranchTemplate(t, db, legacy.id, legacy.wantTemplate) + } +} + +func seedWorkspaceForWorktreeTemplateMigration(t *testing.T, repo *Repository, id string) { + t.Helper() + if err := repo.CreateWorkspace(context.Background(), &models.Workspace{ID: id, Name: id}); err != nil { + t.Fatalf("seed workspace %s: %v", id, err) + } +} + +func assertPostgresWorktreeBranchTemplate(t *testing.T, db interface { + Get(dest interface{}, query string, args ...interface{}) error +}, id, want string) { + t.Helper() + var got string + if err := db.Get(&got, `SELECT worktree_branch_template FROM repositories WHERE id = $1`, id); err != nil { + t.Fatalf("read postgres template %q: %v", id, err) + } + if got != want { + t.Fatalf("postgres template %q = %q, want %q", id, got, want) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_test.go b/apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_test.go new file mode 100644 index 0000000000..e4a7d601a1 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_test.go @@ -0,0 +1,95 @@ +package sqlite + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +func TestWorktreeBranchTemplateMigrationPreservesCustomValue(t *testing.T) { + repo := newRepoForEntityTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "ws-worktree-template-replay") + + custom := &models.Repository{ + ID: "repo-worktree-template-replay", + WorkspaceID: "ws-worktree-template-replay", + Name: "custom-template", + SourceType: "local", + WorktreeBranchPrefix: "feature/", + WorktreeBranchTemplate: "bugfix/{ticket}-{title}", + } + if err := repo.CreateRepository(ctx, custom); err != nil { + t.Fatalf("create repository: %v", err) + } + + if err := repo.runMigrations(); err != nil { + t.Fatalf("replay migrations: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("second replay migrations: %v", err) + } + + got, err := repo.GetRepository(ctx, custom.ID) + if err != nil { + t.Fatalf("get repository after replay: %v", err) + } + if got.WorktreeBranchTemplate != custom.WorktreeBranchTemplate { + t.Fatalf("replayed template = %q, want %q", got.WorktreeBranchTemplate, custom.WorktreeBranchTemplate) + } +} + +func TestWorktreeBranchTemplateMigrationBackfillsLegacyPrefix(t *testing.T) { + repo := newRepoForEntityTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "ws-worktree-template-legacy") + + legacyRepositories := []struct { + id string + prefix string + wantTemplate string + }{ + { + id: "repo-worktree-template-fix", + prefix: " fix/ ", + wantTemplate: "fix/{title}-{suffix}", + }, + { + id: "repo-worktree-template-default", + prefix: " ", + wantTemplate: "feature/{title}-{suffix}", + }, + } + for _, legacy := range legacyRepositories { + if err := repo.CreateRepository(ctx, &models.Repository{ + ID: legacy.id, + WorkspaceID: "ws-worktree-template-legacy", + Name: legacy.id, + SourceType: "local", + WorktreeBranchPrefix: legacy.prefix, + }); err != nil { + t.Fatalf("create legacy repository %q: %v", legacy.id, err) + } + } + + if _, err := repo.db.Exec(`ALTER TABLE repositories DROP COLUMN worktree_branch_template`); err != nil { + t.Fatalf("remove template column from legacy schema: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("migrate legacy schema: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("replay legacy migration: %v", err) + } + + for _, legacy := range legacyRepositories { + var got string + if err := repo.db.QueryRow(`SELECT worktree_branch_template FROM repositories WHERE id = ?`, legacy.id).Scan(&got); err != nil { + t.Fatalf("read migrated template %q: %v", legacy.id, err) + } + if got != legacy.wantTemplate { + t.Errorf("migrated template %q = %q, want %q", legacy.id, got, legacy.wantTemplate) + } + } +} diff --git a/docs/plans/worktree-branch-template-persistence/plan.md b/docs/plans/worktree-branch-template-persistence/plan.md new file mode 100644 index 0000000000..4682390286 --- /dev/null +++ b/docs/plans/worktree-branch-template-persistence/plan.md @@ -0,0 +1,79 @@ +--- +spec: docs/specs/workspaces/worktree-branch-templates.md +created: 2026-08-15 +status: complete +--- + +# Implementation Plan: Preserve Worktree Branch Templates + +## Overview + +Issue [#2611](https://github.com/kdlbs/kandev/issues/2611) occurs because the +template backfill runs on every schema replay. The replay replaces each saved +template with a value derived from the legacy prefix. The repair limits the +backfill to empty values and adds SQLite and Postgres migration tests. + +## Confirmed Root Cause + +`MigrateLogger.Apply` runs successful data statements during every backend startup. +The `repositories.worktree_branch_template.backfill` statement has no condition, so +each startup updates every repository row. + +The column migration also gives legacy rows a non-empty default. A safe repair must +use an empty migration default before it conditionally fills legacy rows. + +## Backend + +### Repository schema migration + +- Update `apps/backend/internal/task/repository/sqlite/base_migrations.go`. +- Add `worktree_branch_template` to a legacy table with an empty default. +- Backfill only rows whose template is null, empty, or whitespace. +- Derive the value from the trimmed legacy prefix. +- Use `feature/` when the legacy prefix is empty. +- Keep the fresh-schema default in `base_schema.go` unchanged. + +This sequence preserves old branch naming during the first upgrade. Later schema +replays leave user values unchanged. + +## Tests + +- **What:** A custom template survives repeated schema migration calls on SQLite. + **File:** `apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_test.go`. + **How:** Save a repository with a custom value, replay `runMigrations`, and read + the row from the real SQLite store. +- **What:** A legacy SQLite row derives its template once from its prefix. + **File:** `apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_test.go`. + **How:** Remove the template column, seed non-empty and empty prefix cases, run + the migration twice, and compare both stored values. +- **What:** Postgres has the same legacy upgrade and replay behavior. + **File:** `apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_postgres_test.go`. + **How:** Use `testutil.OpenIsolatedPostgres`, repeat the legacy and custom-value + cases, and gate the test with `KANDEV_TEST_POSTGRES_DSN`. +- **What:** Existing repository creation and update behavior stays unchanged. + **File:** `apps/backend/internal/task/service/service_test.go`. + **How:** Run the existing default-template and update-template service tests. + +## Verification Results + +- RED: the focused SQLite migration command failed on + `TestWorktreeBranchTemplateMigrationPreservesCustomValue` because replay + replaced the custom template with `feature/{title}-{suffix}`. +- GREEN: `cd apps/backend && go test -tags fts5 ./internal/task/repository/sqlite -run 'Test(WorktreeBranchTemplateMigration|PostgresWorktreeBranchTemplateMigration)' -count=1` passed its two SQLite tests; the Postgres test was skipped because `KANDEV_TEST_POSTGRES_DSN` is unset. +- Service checks: `cd apps/backend && go test -tags fts5 ./internal/task/service -run 'TestService_(CreateRepository_DefaultWorktreeBranchTemplate|UpdateRepository_WorktreeBranchTemplate)' -count=1` passed both tests. +- `git diff --check` passed. +- Fixup: the custom-template regression now replays `runMigrations()` twice; + the migration and service checks passed again after this test-only change. + +## Implementation Waves And Parallel Candidates + +Wave 1 (sequential): + +- [x] [task-01-preserve-branch-template](task-01-preserve-branch-template.md) + +There are no parallel-safe tasks. The migration and its tests share one schema +boundary. + +## Open Questions + +None. diff --git a/docs/plans/worktree-branch-template-persistence/task-01-preserve-branch-template.md b/docs/plans/worktree-branch-template-persistence/task-01-preserve-branch-template.md new file mode 100644 index 0000000000..15edad9eb2 --- /dev/null +++ b/docs/plans/worktree-branch-template-persistence/task-01-preserve-branch-template.md @@ -0,0 +1,70 @@ +--- +id: "01-preserve-branch-template" +title: "Preserve saved worktree branch templates" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/workspaces/worktree-branch-templates.md" +--- + +# Task 01: Preserve Saved Worktree Branch Templates + +## Acceptance + +- A non-empty `worktree_branch_template` survives each startup schema replay. +- A legacy row receives `{title}-{suffix}` exactly once. +- An empty legacy prefix produces `feature/{title}-{suffix}`. +- SQLite and Postgres implement the same behavior. +- Repository creation and update contracts remain unchanged. + +## Verification + +- `cd apps/backend && go test -tags fts5 ./internal/task/repository/sqlite -run 'Test(WorktreeBranchTemplateMigration|PostgresWorktreeBranchTemplateMigration)' -count=1` +- `cd apps/backend && go test -tags fts5 ./internal/task/service -run 'TestService_(CreateRepository_DefaultWorktreeBranchTemplate|UpdateRepository_WorktreeBranchTemplate)' -count=1` + +## Files likely touched + +- `apps/backend/internal/task/repository/sqlite/base_migrations.go` +- `apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_test.go` +- `apps/backend/internal/task/repository/sqlite/worktree_branch_template_migration_postgres_test.go` +- `docs/specs/workspaces/worktree-branch-templates.md` +- `docs/specs/INDEX.md` +- `docs/plans/worktree-branch-template-persistence/plan.md` +- `docs/plans/worktree-branch-template-persistence/task-01-preserve-branch-template.md` + +## Dependencies + +None. + +## Parallelism + +Sequential. The production migration and both dialect tests share one schema +boundary. + +## Inputs + +- The persistence guarantees and scenarios in the linked specification. +- The configurable branch-name contract in + `docs/decisions/0032-configurable-worktree-branch-names.md`. +- The schema replay rules in + `docs/decisions/0027-replayable-schema-migrations.md`. +- The confirmed issue #2611 reproduction. A custom value became + `feature/{title}-{suffix}` after one `runMigrations` call. + +## Output contract + +Report the changed files, each command and result, remaining risks, and synchronized +task and plan statuses in the primary session. + +## Results + +- RED: the focused SQLite migration command reproduced the overwrite of a + custom template during schema replay. +- GREEN: SQLite migration regressions passed. +- Postgres migration regression is implemented and skipped locally because + `KANDEV_TEST_POSTGRES_DSN` is unset. +- Existing repository create and update service checks passed. +- `git diff --check` passed. +- Fixup added a second replay to the custom-template regression; both focused + migration and service commands passed again. diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index 82d7f10efd..069df38595 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -154,6 +154,7 @@ Per-workspace credentials and triage triggers for external services. | [creation](workspaces/creation.md) | building | | [deletion](workspaces/deletion.md) | shipped | | [local-repositories](workspaces/local-repositories.md) | shipped | +| [worktree-branch-templates](workspaces/worktree-branch-templates.md) | building | | [repository-secrets](workspaces/repository-secrets.md) | shipped | | [secret-scope-transfer](workspaces/secret-scope-transfer.md) | shipped | diff --git a/docs/specs/workspaces/worktree-branch-templates.md b/docs/specs/workspaces/worktree-branch-templates.md new file mode 100644 index 0000000000..a84c6fa587 --- /dev/null +++ b/docs/specs/workspaces/worktree-branch-templates.md @@ -0,0 +1,64 @@ +--- +status: building +created: 2026-08-15 +owner: kandev +--- + +# Worktree Branch Templates + +## Why + +Teams use repository-specific branch names for tickets and other local rules. +Kandev must preserve each saved template across backend restarts and redeployments. + +## What + +- Each workspace repository has one worktree branch template. +- A saved valid template replaces the template for that repository. +- A backend restart or schema replay does not change a non-empty saved template. +- A new repository uses `feature/{title}-{suffix}` when the request omits the template. +- An upgrade from the legacy prefix field derives the template once from that prefix. +- The legacy upgrade uses `feature/` when the stored prefix is empty. + +The template syntax and placeholders follow +[ADR 0032](../../decisions/0032-configurable-worktree-branch-names.md). + +## Data Model + +The `repositories` table owns these fields: + +| Field | Contract | +| --- | --- | +| `worktree_branch_template` | The current template for the repository. | +| `worktree_branch_prefix` | The legacy prefix used only to upgrade a repository that has no template field. | + +## API Surface + +The existing repository create, update, and read contracts use the +`worktree_branch_template` field. This repair does not change these contracts. + +## Persistence Guarantees + +The repository store keeps a saved template across process restarts and deployment +replacements. Startup schema work can initialize a legacy row, but it cannot replace +a non-empty template. + +## Scenarios + +- **GIVEN** a repository with a custom template, **WHEN** Kandev restarts, **THEN** + the repository still has the exact custom template. +- **GIVEN** a legacy repository with prefix `fix/` and no template field, **WHEN** + Kandev upgrades the database, **THEN** the template becomes + `fix/{title}-{suffix}`. +- **GIVEN** a legacy repository with an empty prefix and no template field, + **WHEN** Kandev upgrades the database, **THEN** the template becomes + `feature/{title}-{suffix}`. +- **GIVEN** a migrated repository whose template was later changed, **WHEN** Kandev + replays startup schema work, **THEN** the changed template remains unchanged. + +## Out of Scope + +- Recovery of a custom template that an earlier Kandev version already replaced. +- Changes to the template syntax, placeholders, validation, default value, or UI. +- Replacement of the current schema migration system. +