Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions apps/backend/cmd/kandev/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories
// inside their own container).
if githubSvc != nil {
taskSvc.SetRemoteBranchLister(githubBranchListerAdapter{svc: githubSvc})
taskSvc.SetPRTaskResolver(githubSvc)
}

// Initialize Automation service
Expand Down
6 changes: 6 additions & 0 deletions apps/backend/internal/github/service_pr_watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,12 @@ func (s *Service) ListTaskPRs(ctx context.Context, taskIDs []string) (map[string
return s.store.ListTaskPRsByTaskIDs(ctx, taskIDs)
}

// FindTaskIDsByPRNumber returns the IDs of tasks in a workspace associated with
// the given PR number. Used by task search to surface a task by its PR number.
func (s *Service) FindTaskIDsByPRNumber(ctx context.Context, workspaceID string, prNumber int) ([]string, error) {
return s.store.ListTaskIDsByPRNumber(ctx, workspaceID, prNumber)
}

// ListWorkspaceTaskPRs returns all PR associations for a workspace, grouped by
// task_id. Multi-repo tasks may have more than one PR per task. It returns
// cached data immediately and triggers background refresh for stale entries.
Expand Down
19 changes: 19 additions & 0 deletions apps/backend/internal/github/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ func (s *Store) initSchema() error {
if err := s.backfillPRWatchesRepositoryID(); err != nil {
return fmt.Errorf("backfill github_pr_watches.repository_id: %w", err)
}
// pr_number is the 3rd column of UNIQUE(task_id, repository_id, pr_number),
// so SQLite can't use that index for the PR-number task search. Add a
// dedicated leading-key index so lookups by PR number stay index-backed.
_, _ = s.db.Exec(`CREATE INDEX IF NOT EXISTS idx_github_task_prs_pr_number ON github_task_prs (pr_number)`)
return nil
}

Expand Down Expand Up @@ -744,6 +748,21 @@ func (s *Store) ListTaskPRsByWorkspaceID(ctx context.Context, workspaceID string
return groupTaskPRsByTask(prs), nil
}

// ListTaskIDsByPRNumber returns the IDs of tasks in a workspace that have a PR
// association with the given PR number. Workspace-scoped via the JOIN on tasks
// so a PR number shared across workspaces never leaks results. A task with
// multiple PR rows for the same number (multi-repo) is returned once.
func (s *Store) ListTaskIDsByPRNumber(ctx context.Context, workspaceID string, prNumber int) ([]string, error) {
var ids []string
if err := s.ro.SelectContext(ctx, &ids,
`SELECT DISTINCT gtp.task_id FROM github_task_prs gtp
INNER JOIN tasks t ON gtp.task_id = t.id
WHERE t.workspace_id = ? AND gtp.pr_number = ?`, workspaceID, prNumber); err != nil {
return nil, err
}
return ids, nil
}
Comment thread
zeval marked this conversation as resolved.

func groupTaskPRsByTask(prs []TaskPR) map[string][]*TaskPR {
result := make(map[string][]*TaskPR)
for i := range prs {
Expand Down
85 changes: 85 additions & 0 deletions apps/backend/internal/github/store_pr_lookup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package github

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

"github.com/jmoiron/sqlx"

"github.com/kandev/kandev/internal/db"
)

// newTestStoreWithWorkspaces builds a store whose `tasks` table carries
// workspace_id, so the workspace-scoped PR-number lookup can be exercised.
func newTestStoreWithWorkspaces(t *testing.T) *Store {
Comment thread
zeval marked this conversation as resolved.
Outdated
Comment thread
zeval marked this conversation as resolved.
Outdated
t.Helper()
tmp := t.TempDir()
dbConn, err := db.OpenSQLite(filepath.Join(tmp, "github.db"))
if err != nil {
t.Fatalf("open db: %v", err)
}
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
t.Cleanup(func() { _ = sqlxDB.Close() })
if _, err := sqlxDB.Exec(`CREATE TABLE tasks (id TEXT PRIMARY KEY, workspace_id TEXT, archived_at DATETIME)`); err != nil {
t.Fatalf("create tasks table: %v", err)
}
store, err := NewStore(sqlxDB, sqlxDB)
if err != nil {
t.Fatalf("new store: %v", err)
}
return store
}

func TestListTaskIDsByPRNumber(t *testing.T) {
store := newTestStoreWithWorkspaces(t)
ctx := context.Background()
now := time.Now().UTC()

// task-1 (ws-1) has PR #1243, task-2 (ws-1) has PR #99,
// task-3 (ws-2) also has PR #1243 — must not leak into ws-1.
exec := store.db
for _, q := range []string{
`INSERT INTO tasks (id, workspace_id) VALUES ('task-1', 'ws-1')`,
`INSERT INTO tasks (id, workspace_id) VALUES ('task-2', 'ws-1')`,
`INSERT INTO tasks (id, workspace_id) VALUES ('task-3', 'ws-2')`,
} {
if _, err := exec.Exec(q); err != nil {
t.Fatalf("seed task: %v", err)
}
}
mkPR := func(id, taskID string, num int) *TaskPR {
return &TaskPR{
ID: id, TaskID: taskID, RepositoryID: "repo-" + id,
Owner: "kdlbs", Repo: "kandev", PRNumber: num,
PRURL: "https://github.com/kdlbs/kandev/pull/1", PRTitle: "x",
HeadBranch: "feat/x", BaseBranch: "main", State: "open", CreatedAt: now,
}
}
for _, pr := range []*TaskPR{
mkPR("p1", "task-1", 1243),
mkPR("p2", "task-2", 99),
mkPR("p3", "task-3", 1243),
} {
if err := store.CreateTaskPR(ctx, pr); err != nil {
t.Fatalf("create PR: %v", err)
}
}

got, err := store.ListTaskIDsByPRNumber(ctx, "ws-1", 1243)
if err != nil {
t.Fatalf("lookup: %v", err)
}
if len(got) != 1 || got[0] != "task-1" {
t.Errorf("expected [task-1] for ws-1 PR #1243, got %v", got)
}

none, err := store.ListTaskIDsByPRNumber(ctx, "ws-1", 7777)
if err != nil {
t.Fatalf("lookup unknown: %v", err)
}
if len(none) != 0 {
t.Errorf("expected no tasks for unknown PR number, got %v", none)
}
}
14 changes: 14 additions & 0 deletions apps/backend/internal/task/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ type WorkflowStepGetter interface {
GetNextStepByPosition(ctx context.Context, workflowID string, currentPosition int) (*wfmodels.WorkflowStep, error)
}

// PRTaskResolver resolves which tasks are associated with a GitHub PR number.
// Implemented by the github service; injected so the task service can surface a
// task by its PR number in search without coupling to the github schema.
type PRTaskResolver interface {
FindTaskIDsByPRNumber(ctx context.Context, workspaceID string, prNumber int) ([]string, error)
}

// StartStepResolver resolves the starting step for a workflow.
type StartStepResolver interface {
ResolveStartStep(ctx context.Context, workflowID string) (string, error)
Expand Down Expand Up @@ -152,6 +159,7 @@ type Service struct {
workflowStepCreator WorkflowStepCreator
workflowStepGetter WorkflowStepGetter
startStepResolver StartStepResolver
prTaskResolver PRTaskResolver
quickChatDir string // Directory for quick-chat workspaces (e.g., ~/.kandev/quick-chat)
branchFetcher *branchFetcher
envDestroyer EnvironmentDestroyer
Expand Down Expand Up @@ -222,6 +230,12 @@ func (s *Service) SetStartStepResolver(resolver StartStepResolver) {
s.startStepResolver = resolver
}

// SetPRTaskResolver wires the GitHub PR→task resolver for PR-number search.
// Optional — when unset, search by PR number is a no-op.
func (s *Service) SetPRTaskResolver(resolver PRTaskResolver) {
s.prTaskResolver = resolver
}

// SetQuickChatDir sets the directory for quick-chat workspaces.
// When set, task cleanup deletes the session directory under this path for all tasks.
func (s *Service) SetQuickChatDir(dir string) {
Expand Down
197 changes: 197 additions & 0 deletions apps/backend/internal/task/service/service_pr_search_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
package service

import (
"context"
"testing"

"github.com/kandev/kandev/internal/task/models"
v1 "github.com/kandev/kandev/pkg/api/v1"
)

// fakePRResolver is a stand-in for the github service in PR-number search tests.
type fakePRResolver struct {
byPR map[int][]string
err error
called bool
}

func (f *fakePRResolver) FindTaskIDsByPRNumber(_ context.Context, _ string, prNumber int) ([]string, error) {
f.called = true
if f.err != nil {
return nil, f.err
}
return f.byPR[prNumber], nil
}

func seedPRSearchTasks(t *testing.T, repo interface {
CreateWorkspace(context.Context, *models.Workspace) error
CreateWorkflow(context.Context, *models.Workflow) error
CreateTask(context.Context, *models.Task) error
}) {
t.Helper()
ctx := context.Background()
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "WS"}); err != nil {
t.Fatalf("create workspace: %v", err)
}
if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "WF"}); err != nil {
t.Fatalf("create workflow: %v", err)
}
tasks := []*models.Task{
{ID: "task-pr", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "Unrelated work", State: v1.TaskStateCreated, Priority: "medium"},
{ID: "task-title", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "Bug in PR 1243 handler", State: v1.TaskStateCreated, Priority: "medium"},
}
for _, tk := range tasks {
if err := repo.CreateTask(ctx, tk); err != nil {
t.Fatalf("create task %s: %v", tk.ID, err)
}
}
}

func taskIDSet(tasks []*models.Task) map[string]bool {
set := make(map[string]bool, len(tasks))
for _, t := range tasks {
set[t.ID] = true
}
return set
}

func TestListTasksByWorkspace_PRNumberSurfacesTask(t *testing.T) {
svc, _, repo := createTestService(t)
seedPRSearchTasks(t, repo)
svc.SetPRTaskResolver(&fakePRResolver{byPR: map[int][]string{1243: {"task-pr"}}})
ctx := context.Background()

// "#1243" — task-pr has no "1243" in its title, only the PR association.
tasks, total, err := svc.ListTasksByWorkspace(ctx, "ws-1", "", "", "#1243", 1, 5, true, false, false, false)
if err != nil {
t.Fatalf("search: %v", err)
}
got := taskIDSet(tasks)
if !got["task-pr"] {
t.Errorf("expected task-pr surfaced by PR number, got %v", got)
}
if total < 1 {
t.Errorf("expected total >= 1, got %d", total)
}
}

func TestListTasksByWorkspace_PRNumberDedupes(t *testing.T) {
svc, _, repo := createTestService(t)
seedPRSearchTasks(t, repo)
// Resolver returns task-title, which ALSO matches the LIKE search for "1243".
svc.SetPRTaskResolver(&fakePRResolver{byPR: map[int][]string{1243: {"task-title"}}})
ctx := context.Background()

tasks, _, err := svc.ListTasksByWorkspace(ctx, "ws-1", "", "", "1243", 1, 5, true, false, false, false)
if err != nil {
t.Fatalf("search: %v", err)
}
count := 0
for _, tk := range tasks {
if tk.ID == "task-title" {
count++
}
}
if count != 1 {
t.Errorf("expected task-title exactly once, got %d", count)
}
}

func TestListTasksByWorkspace_NonNumericQuerySkipsResolver(t *testing.T) {
svc, _, repo := createTestService(t)
seedPRSearchTasks(t, repo)
resolver := &fakePRResolver{byPR: map[int][]string{1243: {"task-pr"}}}
svc.SetPRTaskResolver(resolver)
ctx := context.Background()

tasks, _, err := svc.ListTasksByWorkspace(ctx, "ws-1", "", "", "unrelated", 1, 5, true, false, false, false)
if err != nil {
t.Fatalf("search: %v", err)
}
if resolver.called {
t.Error("resolver should not be consulted for a non-numeric query")
}
// task-pr matches the title LIKE search, task-title does not.
if got := taskIDSet(tasks); !got["task-pr"] {
t.Errorf("expected title match task-pr, got %v", got)
}
}

func TestListTasksByWorkspace_NilResolverNoPanic(t *testing.T) {
svc, _, repo := createTestService(t)
seedPRSearchTasks(t, repo)
ctx := context.Background()

// No resolver wired — PR-number search must be a safe no-op.
tasks, _, err := svc.ListTasksByWorkspace(ctx, "ws-1", "", "", "#1243", 1, 5, true, false, false, false)
if err != nil {
t.Fatalf("search: %v", err)
}
// task-pr is only reachable via the resolver, so it must be absent.
if got := taskIDSet(tasks); got["task-pr"] {
t.Errorf("did not expect task-pr without a resolver, got %v", got)
}
}

func TestListTasksByWorkspace_PRMatchRespectsArchivedFilter(t *testing.T) {
svc, _, repo := createTestService(t)
seedPRSearchTasks(t, repo)
if err := repo.ArchiveTask(context.Background(), "task-pr"); err != nil {
t.Fatalf("archive: %v", err)
}
svc.SetPRTaskResolver(&fakePRResolver{byPR: map[int][]string{1243: {"task-pr"}}})
ctx := context.Background()

// includeArchived=false → archived PR task excluded.
excluded, _, err := svc.ListTasksByWorkspace(ctx, "ws-1", "", "", "#1243", 1, 5, false, false, false, false)
if err != nil {
t.Fatalf("search excluded: %v", err)
}
if taskIDSet(excluded)["task-pr"] {
t.Error("archived task-pr should be excluded when includeArchived=false")
}

// includeArchived=true → archived PR task included.
included, _, err := svc.ListTasksByWorkspace(ctx, "ws-1", "", "", "#1243", 1, 5, true, false, false, false)
if err != nil {
t.Fatalf("search included: %v", err)
}
if !taskIDSet(included)["task-pr"] {
t.Error("archived task-pr should be included when includeArchived=true")
}
}

func TestListTasksByWorkspace_PRMatchSkippedOnLaterPagesAndScopedSearches(t *testing.T) {
ctx := context.Background()
cases := []struct {
name string
page int
workflowID string
repoID string
}{
{name: "page 2", page: 2},
{name: "workflow-scoped", page: 1, workflowID: "wf-1"},
{name: "repository-scoped", page: 1, repoID: "repo-x"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
svc, _, repo := createTestService(t)
seedPRSearchTasks(t, repo)
resolver := &fakePRResolver{byPR: map[int][]string{1243: {"task-pr"}}}
svc.SetPRTaskResolver(resolver)

tasks, _, err := svc.ListTasksByWorkspace(ctx, "ws-1", tc.workflowID, tc.repoID, "#1243", tc.page, 5, true, false, false, false)
if err != nil {
t.Fatalf("search: %v", err)
}
if resolver.called {
t.Error("resolver should not be consulted for later pages or scoped searches")
}
if taskIDSet(tasks)["task-pr"] {
t.Errorf("task-pr should not be augmented for %s, got %v", tc.name, taskIDSet(tasks))
}
})
}
}

var _ PRTaskResolver = (*fakePRResolver)(nil)
Loading
Loading