feat(tasks): search tasks by PR number in command panel (#1268)
* feat(tasks): search tasks by PR number in command panel Typing a PR number (e.g. 1243 or #1243) in the Cmd+K panel now surfaces the task that PR is associated with, alongside the existing title/description/repo search. The github store gains a workspace-scoped ListTaskIDsByPRNumber lookup over github_task_prs; the task service resolves it through an injected PRTaskResolver interface (github service wired in cmd/kandev) and merges PR matches into search results, deduped and respecting the archived/ephemeral/config filters. Keeping the lookup at the service layer avoids coupling the task repository to the github schema, so existing search queries and tests are untouched. * fix(tasks): scope PR-number search to page 1 and unfiltered queries Address review feedback on PR-number search: - Only augment results on the first page of an unscoped search. Running on every page re-prepended the PR match and truncated a real result off each subsequent page; running under a workflow/repository filter leaked tasks that don't satisfy the filter. - Add idx_github_task_prs_pr_number so the lookup is index-backed instead of a full scan (pr_number is the 3rd column of the existing UNIQUE constraint, unusable as a leading key). - Log instead of silently dropping a task when GetTask fails in the PR-match loop. * test(github): dedup PR-lookup test setup into shared newTestStore Add workspace_id to newTestStore's tasks table and drop the near-identical newTestStoreWithWorkspaces helper, per review feedback. * perf(tasks): batch-fetch PR-matched tasks instead of N+1 GetTask Add a repository GetTasksByIDs and use it in augmentWithPRMatches so PR-number search loads all matched tasks in one query instead of one GetTask per id, addressing the N+1 noted in review. Fetch+filter is extracted into a fetchPRMatchedTasks helper that preserves resolver order and dedup.
This commit is contained in:
@@ -106,6 +106,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
|
||||
|
||||
@@ -354,6 +354,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.
|
||||
|
||||
@@ -207,6 +207,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
|
||||
}
|
||||
|
||||
@@ -814,6 +818,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
|
||||
}
|
||||
|
||||
func groupTaskPRsByTask(prs []TaskPR) map[string][]*TaskPR {
|
||||
result := make(map[string][]*TaskPR)
|
||||
for i := range prs {
|
||||
|
||||
@@ -21,7 +21,7 @@ func newTestStore(t *testing.T) *Store {
|
||||
}
|
||||
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
|
||||
t.Cleanup(func() { _ = sqlxDB.Close() })
|
||||
if _, err := sqlxDB.Exec(`CREATE TABLE tasks (id TEXT PRIMARY KEY, archived_at DATETIME)`); err != nil {
|
||||
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)
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestListTaskIDsByPRNumber(t *testing.T) {
|
||||
// newTestStore's tasks table carries workspace_id, which the
|
||||
// workspace-scoped PR-number lookup needs.
|
||||
store := newTestStore(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)
|
||||
}
|
||||
}
|
||||
@@ -394,6 +394,15 @@ func (m *mockRepository) GetTask(ctx context.Context, id string) (*models.Task,
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
func (m *mockRepository) GetTasksByIDs(ctx context.Context, ids []string) ([]*models.Task, error) {
|
||||
var out []*models.Task
|
||||
for _, id := range ids {
|
||||
if task, ok := m.tasks[id]; ok {
|
||||
out = append(out, task)
|
||||
}
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
func (m *mockRepository) UpdateTask(ctx context.Context, task *models.Task) error { return nil }
|
||||
func (m *mockRepository) DeleteTask(ctx context.Context, id string) error { return nil }
|
||||
func (m *mockRepository) ListTasks(ctx context.Context, workflowID string) ([]*models.Task, error) {
|
||||
|
||||
@@ -52,6 +52,9 @@ func (m *mockRepository) CreateTask(ctx context.Context, task *models.Task) erro
|
||||
func (m *mockRepository) GetTask(ctx context.Context, id string) (*models.Task, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (m *mockRepository) GetTasksByIDs(ctx context.Context, ids []string) ([]*models.Task, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (m *mockRepository) UpdateTask(ctx context.Context, task *models.Task) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@ type WorkspaceRepository interface {
|
||||
type TaskRepository interface {
|
||||
CreateTask(ctx context.Context, task *models.Task) error
|
||||
GetTask(ctx context.Context, id string) (*models.Task, error)
|
||||
GetTasksByIDs(ctx context.Context, ids []string) ([]*models.Task, error)
|
||||
UpdateTask(ctx context.Context, task *models.Task) error
|
||||
DeleteTask(ctx context.Context, id string) error
|
||||
ListTasks(ctx context.Context, workflowID string) ([]*models.Task, error)
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
@@ -653,6 +654,29 @@ func (r *Repository) scanTasks(rows *sql.Rows) ([]*models.Task, error) {
|
||||
return result, rows.Err()
|
||||
}
|
||||
|
||||
// GetTasksByIDs fetches multiple tasks in a single query. Missing IDs are
|
||||
// silently omitted; result order is not guaranteed, so callers that need a
|
||||
// specific order should reorder by ID themselves.
|
||||
func (r *Repository) GetTasksByIDs(ctx context.Context, ids []string) ([]*models.Task, error) {
|
||||
if len(ids) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
placeholders := make([]string, len(ids))
|
||||
args := make([]interface{}, len(ids))
|
||||
for i, id := range ids {
|
||||
placeholders[i] = "?"
|
||||
args[i] = id
|
||||
}
|
||||
query := fmt.Sprintf(`SELECT %s FROM tasks t WHERE t.id IN (%s)`,
|
||||
taskSelectColumns("t"), strings.Join(placeholders, ","))
|
||||
rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(query), args...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer func() { _ = rows.Close() }()
|
||||
return r.scanTasks(rows)
|
||||
}
|
||||
|
||||
// ArchiveTask sets the archived_at timestamp on a task
|
||||
func (r *Repository) ArchiveTask(ctx context.Context, id string) error {
|
||||
now := time.Now().UTC()
|
||||
|
||||
@@ -201,6 +201,40 @@ func TestSQLiteRepository_ListTasks(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSQLiteRepository_GetTasksByIDs(t *testing.T) {
|
||||
repo, cleanup := createTestSQLiteRepo(t)
|
||||
defer cleanup()
|
||||
ctx := context.Background()
|
||||
|
||||
_ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"})
|
||||
_ = repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "WF"})
|
||||
for _, id := range []string{"task-1", "task-2", "task-3"} {
|
||||
_ = repo.CreateTask(ctx, &models.Task{ID: id, WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: id})
|
||||
}
|
||||
|
||||
// Empty input returns no tasks and no error.
|
||||
none, err := repo.GetTasksByIDs(ctx, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTasksByIDs(nil): %v", err)
|
||||
}
|
||||
if len(none) != 0 {
|
||||
t.Errorf("expected 0 tasks for empty input, got %d", len(none))
|
||||
}
|
||||
|
||||
// Existing + missing IDs: only the existing ones come back.
|
||||
got, err := repo.GetTasksByIDs(ctx, []string{"task-1", "task-3", "missing"})
|
||||
if err != nil {
|
||||
t.Fatalf("GetTasksByIDs: %v", err)
|
||||
}
|
||||
ids := map[string]bool{}
|
||||
for _, tk := range got {
|
||||
ids[tk.ID] = true
|
||||
}
|
||||
if len(got) != 2 || !ids["task-1"] || !ids["task-3"] {
|
||||
t.Errorf("expected [task-1 task-3], got %v", ids)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSQLiteRepository_ListTasksByWorkflowStep(t *testing.T) {
|
||||
repo, cleanup := createTestSQLiteRepo(t)
|
||||
defer cleanup()
|
||||
|
||||
@@ -19,6 +19,15 @@ func (f *fakeTaskLookup) GetTask(ctx context.Context, id string) (*models.Task,
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
func (f *fakeTaskLookup) GetTasksByIDs(ctx context.Context, ids []string) ([]*models.Task, error) {
|
||||
var out []*models.Task
|
||||
for _, id := range ids {
|
||||
if t, ok := f.tasks[id]; ok {
|
||||
out = append(out, t)
|
||||
}
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func newGraph(tasks ...*models.Task) *fakeTaskLookup {
|
||||
m := make(map[string]*models.Task, len(tasks))
|
||||
|
||||
@@ -225,6 +225,18 @@ func (f *fakeTaskRepo) GetTask(_ context.Context, id string) (*models.Task, erro
|
||||
return f.tasks[id], nil
|
||||
}
|
||||
|
||||
func (f *fakeTaskRepo) GetTasksByIDs(_ context.Context, ids []string) ([]*models.Task, error) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
var out []*models.Task
|
||||
for _, id := range ids {
|
||||
if t, ok := f.tasks[id]; ok {
|
||||
out = append(out, t)
|
||||
}
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func (f *fakeTaskRepo) ListChildren(_ context.Context, parentID string) ([]*models.Task, error) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
@@ -291,6 +303,9 @@ type phase4TaskRepo struct {
|
||||
func (r *phase4TaskRepo) GetTask(ctx context.Context, id string) (*models.Task, error) {
|
||||
return r.base.GetTask(ctx, id)
|
||||
}
|
||||
func (r *phase4TaskRepo) GetTasksByIDs(ctx context.Context, ids []string) ([]*models.Task, error) {
|
||||
return r.base.GetTasksByIDs(ctx, ids)
|
||||
}
|
||||
func (r *phase4TaskRepo) ListChildren(ctx context.Context, parentID string) ([]*models.Task, error) {
|
||||
return r.base.ListChildren(ctx, parentID)
|
||||
}
|
||||
|
||||
@@ -104,6 +104,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)
|
||||
@@ -177,6 +184,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
|
||||
@@ -264,6 +272,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) {
|
||||
|
||||
@@ -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)
|
||||
@@ -1161,6 +1161,19 @@ func (s *Service) ListTasksByWorkspace(ctx context.Context, workspaceID, workflo
|
||||
return nil, 0, err
|
||||
}
|
||||
|
||||
tasks, total = s.augmentWithPRMatches(ctx, tasks, total, prSearchOptions{
|
||||
workspaceID: workspaceID,
|
||||
workflowID: workflowID,
|
||||
repositoryID: repositoryID,
|
||||
query: query,
|
||||
page: page,
|
||||
pageSize: pageSize,
|
||||
includeArchived: includeArchived,
|
||||
includeEphemeral: includeEphemeral,
|
||||
onlyEphemeral: onlyEphemeral,
|
||||
excludeConfig: excludeConfig,
|
||||
})
|
||||
|
||||
if err := s.loadTaskRepositoriesBatch(ctx, tasks); err != nil {
|
||||
s.logger.Error("failed to batch-load task repositories", zap.Error(err))
|
||||
}
|
||||
@@ -1168,6 +1181,145 @@ func (s *Service) ListTasksByWorkspace(ctx context.Context, workspaceID, workflo
|
||||
return tasks, total, nil
|
||||
}
|
||||
|
||||
type prSearchOptions struct {
|
||||
workspaceID string
|
||||
workflowID string
|
||||
repositoryID string
|
||||
query string
|
||||
page int
|
||||
pageSize int
|
||||
includeArchived bool
|
||||
includeEphemeral bool
|
||||
onlyEphemeral bool
|
||||
excludeConfig bool
|
||||
}
|
||||
|
||||
// parsePRQuery extracts a positive PR number from a search query, accepting an
|
||||
// optional leading '#'. Returns (0, false) when the query is not a PR number.
|
||||
func parsePRQuery(query string) (int, bool) {
|
||||
trimmed := strings.TrimPrefix(strings.TrimSpace(query), "#")
|
||||
if trimmed == "" {
|
||||
return 0, false
|
||||
}
|
||||
n, err := strconv.Atoi(trimmed)
|
||||
if err != nil || n <= 0 {
|
||||
return 0, false
|
||||
}
|
||||
return n, true
|
||||
}
|
||||
|
||||
// isConfigTask reports whether a task is a config-mode task (mirrors the SQL
|
||||
// `json_extract(metadata, '$.config_mode') IS NOT 1` filter). JSON-decoded
|
||||
// numbers arrive as float64, so accept both numeric 1 and bool true.
|
||||
func isConfigTask(task *models.Task) bool {
|
||||
switch v := task.Metadata["config_mode"].(type) {
|
||||
case float64:
|
||||
return v == 1
|
||||
case int:
|
||||
return v == 1
|
||||
case bool:
|
||||
return v
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// augmentWithPRMatches surfaces tasks associated with a PR number when the
|
||||
// search query looks like one. PR matches are prepended (most relevant) and
|
||||
// deduped against the existing results; `total` grows by the net-new count.
|
||||
// Best-effort: a missing resolver or a lookup error leaves results unchanged.
|
||||
//
|
||||
// Augmentation only applies to the first page of an unscoped search. It is
|
||||
// skipped for page > 1 (the prepend+truncate only makes sense against page 1,
|
||||
// otherwise a PR match would re-appear on every page and push out a real
|
||||
// result) and when a workflow or repository filter is set (a PR-matched task
|
||||
// isn't guaranteed to satisfy those filters, and the only caller that searches
|
||||
// by PR number — the Cmd+K command panel — sets neither).
|
||||
func (s *Service) augmentWithPRMatches(ctx context.Context, tasks []*models.Task, total int, opts prSearchOptions) ([]*models.Task, int) {
|
||||
if opts.page > 1 || opts.workflowID != "" || opts.repositoryID != "" {
|
||||
return tasks, total
|
||||
}
|
||||
prNum, ok := parsePRQuery(opts.query)
|
||||
if !ok || s.prTaskResolver == nil {
|
||||
return tasks, total
|
||||
}
|
||||
ids, err := s.prTaskResolver.FindTaskIDsByPRNumber(ctx, opts.workspaceID, prNum)
|
||||
if err != nil {
|
||||
s.logger.Warn("PR-number task lookup failed", zap.Int("pr_number", prNum), zap.Error(err))
|
||||
return tasks, total
|
||||
}
|
||||
|
||||
matched := s.fetchPRMatchedTasks(ctx, ids, tasks, opts)
|
||||
if len(matched) == 0 {
|
||||
return tasks, total
|
||||
}
|
||||
|
||||
merged := make([]*models.Task, 0, len(matched)+len(tasks))
|
||||
merged = append(merged, matched...)
|
||||
merged = append(merged, tasks...)
|
||||
total += len(matched)
|
||||
if len(merged) > opts.pageSize {
|
||||
merged = merged[:opts.pageSize]
|
||||
}
|
||||
return merged, total
|
||||
}
|
||||
|
||||
// fetchPRMatchedTasks batch-loads the resolver's task IDs that aren't already in
|
||||
// `existing`, applies the same visibility filters as the repository search, and
|
||||
// returns the survivors in resolver order. The resolver returns distinct IDs,
|
||||
// so excluding the already-present ones is enough to keep the result deduped.
|
||||
func (s *Service) fetchPRMatchedTasks(ctx context.Context, ids []string, existing []*models.Task, opts prSearchOptions) []*models.Task {
|
||||
seen := make(map[string]struct{}, len(existing))
|
||||
for _, t := range existing {
|
||||
seen[t.ID] = struct{}{}
|
||||
}
|
||||
var fetchIDs []string
|
||||
for _, id := range ids {
|
||||
if _, ok := seen[id]; !ok {
|
||||
fetchIDs = append(fetchIDs, id)
|
||||
}
|
||||
}
|
||||
if len(fetchIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
fetched, err := s.tasks.GetTasksByIDs(ctx, fetchIDs)
|
||||
if err != nil {
|
||||
s.logger.Warn("PR-match task fetch failed", zap.Error(err))
|
||||
return nil
|
||||
}
|
||||
byID := make(map[string]*models.Task, len(fetched))
|
||||
for _, t := range fetched {
|
||||
byID[t.ID] = t
|
||||
}
|
||||
var matched []*models.Task
|
||||
for _, id := range fetchIDs {
|
||||
task := byID[id]
|
||||
if task == nil || s.prMatchFilteredOut(task, opts) {
|
||||
continue
|
||||
}
|
||||
matched = append(matched, task)
|
||||
}
|
||||
return matched
|
||||
}
|
||||
|
||||
// prMatchFilteredOut applies the same visibility filters the repository search
|
||||
// uses, so a PR-matched task respects includeArchived / ephemeral / config flags.
|
||||
func (s *Service) prMatchFilteredOut(task *models.Task, opts prSearchOptions) bool {
|
||||
if !opts.includeArchived && task.ArchivedAt != nil {
|
||||
return true
|
||||
}
|
||||
if opts.onlyEphemeral && !task.IsEphemeral {
|
||||
return true
|
||||
}
|
||||
if !opts.includeEphemeral && !opts.onlyEphemeral && task.IsEphemeral {
|
||||
return true
|
||||
}
|
||||
if opts.excludeConfig && isConfigTask(task) {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// loadTaskRepositoriesBatch loads repositories for multiple tasks in a single query.
|
||||
func (s *Service) loadTaskRepositoriesBatch(ctx context.Context, tasks []*models.Task) error {
|
||||
if len(tasks) == 0 {
|
||||
|
||||
Reference in New Issue
Block a user