Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions apps/backend/cmd/kandev/e2e_reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"database/sql"
"net/http"
"os"
"strings"
Expand Down Expand Up @@ -96,6 +97,17 @@ func handleE2EReset(
log.Warn("e2e reset: agent settings reset failed", zap.Error(err))
}

// Wipe Mantis configs + issue watches (and their dedup rows) so
// per-workspace Mantis state doesn't bleed across specs. Done before
// task deletion so the Mantis forward poller (once wired in T05)
// can't recreate dedup rows / tasks mid-reset — same race the
// GitHub review watch cleanup below was hardened against.
if err := deleteMantisStateForReset(ctx, repo.DB(), workspaceID); err != nil {
log.Error("e2e reset: mantis cleanup failed", zap.Error(err))
c.JSON(http.StatusInternalServerError, gin.H{errKey: err.Error()})
return
}

// Wipe GitHub review watches (and their dedup rows + owned tasks) so
// review-watch specs stay isolated. seedData/backend are worker-scoped,
// so a watch an earlier test created stays enabled; the global review
Expand Down Expand Up @@ -185,6 +197,25 @@ func deleteAutomationsForReset(
return automationSvc.Store().DeleteAutomationsByWorkspace(ctx, workspaceID)
}

// deleteMantisStateForReset wipes per-workspace Mantis state in the order
// required by the foreign-key topology: dedup rows first (so the FK to
// mantis_issue_watches doesn't fire after the parent is gone), then watches,
// then the workspace config. Callers must invoke this BEFORE the task
// deletion phase so the Mantis forward poller can't recreate rows mid-reset
// — see apps/backend/CLAUDE.md "E2E reset invariant".
func deleteMantisStateForReset(ctx context.Context, db *sql.DB, workspaceID string) error {
for _, q := range []string{
`DELETE FROM mantis_issue_watch_tasks WHERE issue_watch_id IN (SELECT id FROM mantis_issue_watches WHERE workspace_id = ?)`,
`DELETE FROM mantis_issue_watches WHERE workspace_id = ?`,
`DELETE FROM mantis_configs WHERE workspace_id = ?`,
} {
if _, err := db.ExecContext(ctx, q, workspaceID); err != nil {
return err
}
}
return nil
}

type e2eHiddenWorkflowRequest struct {
WorkspaceID string `json:"workspace_id"`
Name string `json:"name"`
Expand Down
152 changes: 152 additions & 0 deletions apps/backend/cmd/kandev/e2e_reset_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
package main

import (
"context"
"path/filepath"
"testing"
"time"

"github.com/google/uuid"
"github.com/jmoiron/sqlx"

"github.com/kandev/kandev/internal/db"
sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite"
)

// TestDeleteMantisStateForReset_OnlyTargetWorkspace locks in the workspace
// isolation contract: the cleanup deletes Mantis state for one workspace and
// leaves every other workspace untouched. Catches accidental over-broad
// DELETEs (e.g. dropping the WHERE clause) before they leak into specs where
// two workspaces share a Playwright worker's database.
func TestDeleteMantisStateForReset_OnlyTargetWorkspace(t *testing.T) {
repo, sqlxDB := newE2EResetTestRepo(t)
_ = repo

target := uuid.New().String()
bystander := uuid.New().String()
seedMantisFixtures(t, sqlxDB, target)
seedMantisFixtures(t, sqlxDB, bystander)

if err := deleteMantisStateForReset(context.Background(), sqlxDB.DB, target); err != nil {
t.Fatalf("deleteMantisStateForReset: %v", err)
}

if got := countMantisConfigs(t, sqlxDB, target); got != 0 {
t.Errorf("target mantis_configs still has %d rows, want 0", got)
}
if got := countMantisWatches(t, sqlxDB, target); got != 0 {
t.Errorf("target mantis_issue_watches still has %d rows, want 0", got)
}
if got := countMantisWatchTasks(t, sqlxDB, target); got != 0 {
t.Errorf("target mantis_issue_watch_tasks still has %d rows, want 0", got)
}

if got := countMantisConfigs(t, sqlxDB, bystander); got != 1 {
t.Errorf("bystander mantis_configs has %d rows, want 1 (cleanup leaked across workspaces)", got)
}
if got := countMantisWatches(t, sqlxDB, bystander); got != 1 {
t.Errorf("bystander mantis_issue_watches has %d rows, want 1", got)
}
if got := countMantisWatchTasks(t, sqlxDB, bystander); got != 1 {
t.Errorf("bystander mantis_issue_watch_tasks has %d rows, want 1", got)
}
}

// TestDeleteMantisStateForReset_Idempotent locks in the no-op-safe contract:
// running the cleanup against a workspace that has no Mantis rows must not
// error. Reset endpoints are hit between every spec, so the empty-state path
// is hot.
func TestDeleteMantisStateForReset_Idempotent(t *testing.T) {
_, sqlxDB := newE2EResetTestRepo(t)
ws := uuid.New().String()
if err := deleteMantisStateForReset(context.Background(), sqlxDB.DB, ws); err != nil {
t.Fatalf("first call: %v", err)
}
if err := deleteMantisStateForReset(context.Background(), sqlxDB.DB, ws); err != nil {
t.Fatalf("second call: %v", err)
}
}

// newE2EResetTestRepo opens an on-disk SQLite repo so initSchema runs through
// the same code path the live binary uses on boot.
func newE2EResetTestRepo(t *testing.T) (*sqliterepo.Repository, *sqlx.DB) {
t.Helper()
tmpDir := t.TempDir()
dbConn, err := db.OpenSQLite(filepath.Join(tmpDir, "test.db"))
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
repo, err := sqliterepo.NewWithDB(sqlxDB, sqlxDB, nil)
if err != nil {
t.Fatalf("new repo: %v", err)
}
t.Cleanup(func() { _ = sqlxDB.Close() })
return repo, sqlxDB
Comment on lines +79 to +85

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Move cleanup registration above NewWithDB fatal path.

If NewWithDB(...) fails, t.Fatalf exits before cleanup is registered, leaving the DB open on that path.

Suggested fix
  sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
+ t.Cleanup(func() { _ = sqlxDB.Close() })
  repo, err := sqliterepo.NewWithDB(sqlxDB, sqlxDB, nil)
  if err != nil {
    t.Fatalf("new repo: %v", err)
  }
- t.Cleanup(func() { _ = sqlxDB.Close() })
  return repo, sqlxDB

As per coding guidelines, “Register t.Cleanup immediately after creating resources that need teardown ... before any t.Fatal/t.Fatalf path to prevent resource leaks.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
repo, err := sqliterepo.NewWithDB(sqlxDB, sqlxDB, nil)
if err != nil {
t.Fatalf("new repo: %v", err)
}
t.Cleanup(func() { _ = sqlxDB.Close() })
return repo, sqlxDB
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
t.Cleanup(func() { _ = sqlxDB.Close() })
repo, err := sqliterepo.NewWithDB(sqlxDB, sqlxDB, nil)
if err != nil {
t.Fatalf("new repo: %v", err)
}
return repo, sqlxDB
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/backend/cmd/kandev/e2e_reset_test.go` around lines 79 - 85, The
t.Cleanup registration for closing sqlxDB is placed after the NewWithDB call
that could fail with t.Fatalf, which means cleanup will never be registered if
NewWithDB returns an error, leaving the database connection open. Move the
t.Cleanup call immediately after the sqlx.NewDb call (before NewWithDB is
invoked) so that the cleanup is registered before any potential fatal error
path, ensuring the database connection is always closed even if NewWithDB fails.

Source: Coding guidelines

Comment on lines +79 to +85

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Register t.Cleanup immediately after creating sqlxDB; otherwise a NewWithDB failure path exits via t.Fatalf before cleanup is registered and can leak the DB handle.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/cmd/kandev/e2e_reset_test.go, line 79:

<comment>Register `t.Cleanup` immediately after creating `sqlxDB`; otherwise a `NewWithDB` failure path exits via `t.Fatalf` before cleanup is registered and can leak the DB handle.</comment>

<file context>
@@ -0,0 +1,152 @@
+	if err != nil {
+		t.Fatalf("open sqlite: %v", err)
+	}
+	sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
+	repo, err := sqliterepo.NewWithDB(sqlxDB, sqlxDB, nil)
+	if err != nil {
</file context>
Suggested change
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
repo, err := sqliterepo.NewWithDB(sqlxDB, sqlxDB, nil)
if err != nil {
t.Fatalf("new repo: %v", err)
}
t.Cleanup(func() { _ = sqlxDB.Close() })
return repo, sqlxDB
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
t.Cleanup(func() { _ = sqlxDB.Close() })
repo, err := sqliterepo.NewWithDB(sqlxDB, sqlxDB, nil)
if err != nil {
t.Fatalf("new repo: %v", err)
}
return repo, sqlxDB

}

// seedMantisFixtures writes one row into each of the three Mantis tables for
// the given workspace, with a deterministic watch ID chain so the watch_tasks
// row's FK to mantis_issue_watches is satisfied.
func seedMantisFixtures(t *testing.T, db *sqlx.DB, workspaceID string) {
t.Helper()
now := time.Now().UTC()
if _, err := db.Exec(`
INSERT INTO mantis_configs (
workspace_id, base_url, username, auth_method, default_project_id,
last_ok, last_error, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`,
workspaceID, "https://example.mantis", "user", "api_token", 0, 0, "", now, now,
); err != nil {
t.Fatalf("seed mantis_configs: %v", err)
}
watchID := uuid.New().String()
if _, err := db.Exec(`
INSERT INTO mantis_issue_watches (
id, workspace_id, workflow_id, workflow_step_id, filter,
agent_profile_id, executor_profile_id, prompt, enabled,
poll_interval_seconds, last_error, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
watchID, workspaceID, "wf", "step", "{}", "", "", "", 1, 300, "", now, now,
); err != nil {
t.Fatalf("seed mantis_issue_watches: %v", err)
}
if _, err := db.Exec(`
INSERT INTO mantis_issue_watch_tasks (
id, issue_watch_id, issue_id, issue_url, task_id, created_at
) VALUES (?, ?, ?, ?, ?, ?)`,
uuid.New().String(), watchID, "42", "https://example.mantis/view.php?id=42", "", now,
); err != nil {
t.Fatalf("seed mantis_issue_watch_tasks: %v", err)
}
}

func countMantisConfigs(t *testing.T, db *sqlx.DB, workspaceID string) int {
t.Helper()
var n int
if err := db.Get(&n, `SELECT COUNT(*) FROM mantis_configs WHERE workspace_id = ?`, workspaceID); err != nil {
t.Fatalf("count mantis_configs: %v", err)
}
return n
}

func countMantisWatches(t *testing.T, db *sqlx.DB, workspaceID string) int {
t.Helper()
var n int
if err := db.Get(&n, `SELECT COUNT(*) FROM mantis_issue_watches WHERE workspace_id = ?`, workspaceID); err != nil {
t.Fatalf("count mantis_issue_watches: %v", err)
}
return n
}

func countMantisWatchTasks(t *testing.T, db *sqlx.DB, workspaceID string) int {
t.Helper()
var n int
if err := db.Get(&n, `
SELECT COUNT(*) FROM mantis_issue_watch_tasks
WHERE issue_watch_id IN (SELECT id FROM mantis_issue_watches WHERE workspace_id = ?)
`, workspaceID); err != nil {
t.Fatalf("count mantis_issue_watch_tasks: %v", err)
}
return n
}
Loading