diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index 4da6627e5c..b570ce502b 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -524,14 +524,22 @@ func registerRoutes(p routeParams) { if p.services.GitHub != nil { p.services.GitHub.SetCascadeTaskDeleter(handoffSvc) } + // repoLookup validates a watcher's optional repository binding (workspace + // ownership + default-branch fill) on create/update. Shared across the three + // repo-less watchers; one concrete adapter satisfies each package's + // structurally-identical RepositoryLookup interface. + repoLookup := &repositoryLookupAdapter{svc: p.taskSvc} if p.services.Jira != nil { p.services.Jira.SetTaskDeleter(handoffSvc) + p.services.Jira.SetRepositoryLookup(repoLookup) } if p.services.Linear != nil { p.services.Linear.SetTaskDeleter(handoffSvc) + p.services.Linear.SetRepositoryLookup(repoLookup) } if p.services.Sentry != nil { p.services.Sentry.SetTaskDeleter(handoffSvc) + p.services.Sentry.SetRepositoryLookup(repoLookup) } p.orchestratorSvc.SetWorkspaceMaterializer(handoffSvc) // Phase 8 prompt enrichment — wire the office scheduler's diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index a869994ce8..7c720aa1d3 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -477,6 +477,9 @@ func startAgentInfrastructure( // reconciler when its agent type left the registry) self-heal on the // next poll instead of looping on "profile not found" forever. orchestratorSvc.SetProfileLookup(&profileLookupAdapter{store: repos.AgentSettings}) + // Watcher dispatch self-heals a binding whose repository was soft-deleted + // after the watch was configured, instead of creating an orphan task row. + orchestratorSvc.SetRepositoryChecker(&repositoryLookupAdapter{svc: services.Task}) // Wire the watcher-dependency enumerator into the agent settings // controller so the profile-delete UI can surface "this will also diff --git a/apps/backend/internal/backendapp/turn_adapters.go b/apps/backend/internal/backendapp/turn_adapters.go index 92e9b16644..9c5a9daa9b 100644 --- a/apps/backend/internal/backendapp/turn_adapters.go +++ b/apps/backend/internal/backendapp/turn_adapters.go @@ -107,3 +107,38 @@ func (a *taskDeleterAdapter) DeleteTask(ctx context.Context, taskID string) erro } return err } + +// repositoryLookupAdapter satisfies the linear/jira/sentry RepositoryLookup +// interface over the task service. It is the validation seam for a watcher's +// optional repository binding. The task service's GetRepository filters +// soft-deleted rows and errors on a miss, so a missing or deleted repository +// maps to ok=false and watcher create/update rejects the binding. +type repositoryLookupAdapter struct { + svc *taskservice.Service +} + +func (a *repositoryLookupAdapter) GetRepository(ctx context.Context, id string) (string, string, bool) { + repo, err := a.svc.GetRepository(ctx, id) + if err != nil || repo == nil { + return "", "", false + } + return repo.WorkspaceID, repo.DefaultBranch, true +} + +// RepositoryExists satisfies orchestrator.RepositoryChecker. It uses the +// workspace listing (which excludes soft-deleted repos) so a definitive +// "absent" is distinguishable from a transient error: a non-nil err lets the +// dispatch pre-flight fail open, while (false, nil) means the bound repository +// was removed and the watcher should self-heal. +func (a *repositoryLookupAdapter) RepositoryExists(ctx context.Context, workspaceID, repositoryID string) (bool, error) { + repos, err := a.svc.ListRepositories(ctx, workspaceID) + if err != nil { + return false, err + } + for _, repo := range repos { + if repo.ID == repositoryID { + return true, nil + } + } + return false, nil +} diff --git a/apps/backend/internal/common/securityutil/git.go b/apps/backend/internal/common/securityutil/git.go index bcc93b8521..c90cbb87ca 100644 --- a/apps/backend/internal/common/securityutil/git.go +++ b/apps/backend/internal/common/securityutil/git.go @@ -32,6 +32,25 @@ func IsValidBranchName(branch string) bool { return validBranchNameRegex.MatchString(branch) } +// IsValidBaseBranchRef validates a base-branch ref using the IsValidBranchName +// allowlist, transparently stripping an "origin/" prefix (the regex disallows +// "/" as the first character). Shared so service-tier base-branch rejection +// uses the same allowlist as task base-branch overrides and the agentctl-side +// sanitiser. +func IsValidBaseBranchRef(ref string) bool { + rest := ref + if stripped, ok := strings.CutPrefix(ref, "origin/"); ok { + rest = stripped + } + // git check-ref-format also rejects a trailing "/" and consecutive "//", + // which the IsValidBranchName regex (its char class includes "/") permits. + // Reject them here so "main/" / "origin/main/" / "a//b" don't pass. + if strings.HasSuffix(rest, "/") || strings.Contains(rest, "//") { + return false + } + return IsValidBranchName(rest) +} + // IsKnownSafeGitFlag returns true if the argument is a known safe git flag used by this codebase. // This prevents argument injection where user input could introduce malicious flags. // Only flags actually used by the Kandev codebase are whitelisted. diff --git a/apps/backend/internal/common/securityutil/git_test.go b/apps/backend/internal/common/securityutil/git_test.go new file mode 100644 index 0000000000..35d0316de4 --- /dev/null +++ b/apps/backend/internal/common/securityutil/git_test.go @@ -0,0 +1,28 @@ +package securityutil + +import "testing" + +func TestIsValidBaseBranchRef(t *testing.T) { + cases := map[string]bool{ + "main": true, + "release/v2": true, + "feature/foo-bar": true, + "origin/main": true, + "origin/release/v2": true, + "": false, + "main/": false, // trailing slash + "origin/main/": false, // trailing slash after origin strip + "a//b": false, // consecutive slashes + "bad..ref": false, // path traversal + "feature.lock": false, // .lock suffix + "bad branch": false, // space + "bad;rm -rf": false, // shell metacharacters + "-flag": false, // leading dash (regex requires alphanumeric first) + "/leading": false, // leading slash + } + for ref, want := range cases { + if got := IsValidBaseBranchRef(ref); got != want { + t.Errorf("IsValidBaseBranchRef(%q) = %v, want %v", ref, got, want) + } + } +} diff --git a/apps/backend/internal/jira/models.go b/apps/backend/internal/jira/models.go index 3c90106252..2ab3f11bc4 100644 --- a/apps/backend/internal/jira/models.go +++ b/apps/backend/internal/jira/models.go @@ -156,14 +156,24 @@ const DefaultIssueWatchPollInterval = 300 // the JQL on a schedule and emits a NewJiraIssueEvent for each matching ticket // the orchestrator hasn't already turned into a Kandev task. // -// Unlike the GitHub equivalent, JIRA issues have no repository affinity — the -// target workflow step's defaults determine where the resulting task runs, so -// there's no `repos` column. +// A watch may optionally bind a single repository (RepositoryID + BaseBranch); +// when set, watcher-created tasks launch in an isolated worktree of that repo. +// Empty = unbound, in which case the target workflow step's defaults determine +// where the resulting task runs (the historical repo-less behaviour). type IssueWatch struct { - ID string `json:"id" db:"id"` - WorkspaceID string `json:"workspaceId" db:"workspace_id"` - WorkflowID string `json:"workflowId" db:"workflow_id"` - WorkflowStepID string `json:"workflowStepId" db:"workflow_step_id"` + ID string `json:"id" db:"id"` + WorkspaceID string `json:"workspaceId" db:"workspace_id"` + WorkflowID string `json:"workflowId" db:"workflow_id"` + WorkflowStepID string `json:"workflowStepId" db:"workflow_step_id"` + // RepositoryID optionally binds watcher-created tasks to a repository so the + // agent launches in an isolated worktree instead of a blank scratch checkout. + // Empty = unbound. When set, the resulting task carries a single + // (repository_id, base_branch) pair. + RepositoryID string `json:"repositoryId" db:"repository_id"` + // BaseBranch is the branch the per-task worktree is cut from. Empty defaults + // to the repository's default branch (resolved at create/update time). + // Meaningful only when RepositoryID is set. + BaseBranch string `json:"baseBranch" db:"base_branch"` JQL string `json:"jql" db:"jql"` AgentProfileID string `json:"agentProfileId" db:"agent_profile_id"` ExecutorProfileID string `json:"executorProfileId" db:"executor_profile_id"` @@ -200,10 +210,15 @@ type IssueWatchTask struct { // ticket matching a watch that has no existing dedup row. The orchestrator // consumes this to create (and optionally auto-start) a Kandev task. type NewJiraIssueEvent struct { - IssueWatchID string `json:"issueWatchId"` - WorkspaceID string `json:"workspaceId"` - WorkflowID string `json:"workflowId"` - WorkflowStepID string `json:"workflowStepId"` + IssueWatchID string `json:"issueWatchId"` + WorkspaceID string `json:"workspaceId"` + WorkflowID string `json:"workflowId"` + WorkflowStepID string `json:"workflowStepId"` + // RepositoryID / BaseBranch carry the watch's optional repository binding so + // the orchestrator source can populate IssueTaskRequest.Repositories without + // reloading the watch row. Empty RepositoryID = unbound (repo-less task). + RepositoryID string `json:"repositoryId,omitempty"` + BaseBranch string `json:"baseBranch,omitempty"` AgentProfileID string `json:"agentProfileId"` ExecutorProfileID string `json:"executorProfileId"` Prompt string `json:"prompt"` @@ -219,6 +234,8 @@ type CreateIssueWatchRequest struct { WorkspaceID string `json:"workspaceId"` WorkflowID string `json:"workflowId"` WorkflowStepID string `json:"workflowStepId"` + RepositoryID string `json:"repositoryId"` + BaseBranch string `json:"baseBranch"` JQL string `json:"jql"` AgentProfileID string `json:"agentProfileId"` ExecutorProfileID string `json:"executorProfileId"` @@ -235,6 +252,8 @@ type CreateIssueWatchRequest struct { type UpdateIssueWatchRequest struct { WorkflowID *string `json:"workflowId,omitempty"` WorkflowStepID *string `json:"workflowStepId,omitempty"` + RepositoryID *string `json:"repositoryId,omitempty"` + BaseBranch *string `json:"baseBranch,omitempty"` JQL *string `json:"jql,omitempty"` AgentProfileID *string `json:"agentProfileId,omitempty"` ExecutorProfileID *string `json:"executorProfileId,omitempty"` diff --git a/apps/backend/internal/jira/service.go b/apps/backend/internal/jira/service.go index cb5f6f90e1..5ffe956351 100644 --- a/apps/backend/internal/jira/service.go +++ b/apps/backend/internal/jira/service.go @@ -11,6 +11,7 @@ import ( "go.uber.org/zap" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/common/securityutil" "github.com/kandev/kandev/internal/events" "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/watchreset" @@ -25,6 +26,15 @@ type SecretStore interface { Exists(ctx context.Context, id string) (bool, error) } +// RepositoryLookup is the subset of the task service used to validate a watch's +// optional repository binding (workspace ownership + default-branch fill). +// Wired post-construction via SetRepositoryLookup to avoid an import cycle with +// the task service. ok is false when the repository does not exist or has been +// soft-deleted. +type RepositoryLookup interface { + GetRepository(ctx context.Context, id string) (workspaceID, defaultBranch string, ok bool) +} + // Service orchestrates Jira config storage, the cached client, and the // fetch/transition operations used by the WebSocket + HTTP handlers. type Service struct { @@ -41,6 +51,10 @@ type Service struct { // the import cycle that would result from passing the concrete // *task.HandoffService into NewService. taskDeleter watchreset.TaskDeleter + // repoLookup validates an optional repository binding on create/update. + // Wired post-construction via SetRepositoryLookup. When nil (e.g. unit + // tests), the binding is accepted as-is and default-branch fill is skipped. + repoLookup RepositoryLookup // mockClient is non-nil only when Provide built the service with a MockClient // (KANDEV_MOCK_JIRA=true). Exposed via MockClient() so the e2e control routes // can drive the same instance the clientFn returns. @@ -56,6 +70,21 @@ func (s *Service) SetTaskDeleter(td watchreset.TaskDeleter) { s.taskDeleter = td } +// SetRepositoryLookup wires the repository validator used by CreateIssueWatch / +// UpdateIssueWatch. Optional — when unset, a repository binding is persisted +// as-is without workspace/default-branch resolution. +func (s *Service) SetRepositoryLookup(rl RepositoryLookup) { + s.mu.Lock() + defer s.mu.Unlock() + s.repoLookup = rl +} + +func (s *Service) getRepositoryLookup() RepositoryLookup { + s.mu.Lock() + defer s.mu.Unlock() + return s.repoLookup +} + // MockClient returns the shared mock client when the service was built in mock // mode, or nil for production builds. func (s *Service) MockClient() *MockClient { @@ -517,10 +546,16 @@ func (s *Service) CreateIssueWatch(ctx context.Context, req *CreateIssueWatchReq if err := validateIssueWatchCreate(req); err != nil { return nil, err } + repositoryID, baseBranch, err := s.resolveRepositoryBinding(ctx, req.WorkspaceID, req.RepositoryID, req.BaseBranch) + if err != nil { + return nil, err + } w := &IssueWatch{ WorkspaceID: req.WorkspaceID, WorkflowID: req.WorkflowID, WorkflowStepID: req.WorkflowStepID, + RepositoryID: repositoryID, + BaseBranch: baseBranch, JQL: strings.TrimSpace(req.JQL), AgentProfileID: req.AgentProfileID, ExecutorProfileID: req.ExecutorProfileID, @@ -567,6 +602,7 @@ func (s *Service) UpdateIssueWatch(ctx context.Context, id string, req *UpdateIs if err != nil { return nil, err } + prevRepositoryID, prevBaseBranch := w.RepositoryID, w.BaseBranch applyIssueWatchPatch(w, req) // Empty-string PATCH writes (`{"workflowId": ""}` etc.) bypass the nil // guard in applyIssueWatchPatch — Go's JSON decoder returns a non-nil @@ -584,6 +620,18 @@ func (s *Service) UpdateIssueWatch(ctx context.Context, id string, req *UpdateIs if err := validateMaxInflightTasks(w.MaxInflightTasks); err != nil { return nil, err } + // Only validate/resolve the binding when its value actually changed. The + // dialog re-sends repositoryId/baseBranch on every PATCH, and an unchanged + // binding whose repo was since soft-deleted must not block edits to other + // fields (prompt, JQL, …). + if w.RepositoryID != prevRepositoryID || w.BaseBranch != prevBaseBranch { + repositoryID, baseBranch, err := s.resolveRepositoryBinding(ctx, w.WorkspaceID, w.RepositoryID, w.BaseBranch) + if err != nil { + return nil, err + } + w.RepositoryID = repositoryID + w.BaseBranch = baseBranch + } if err := s.store.UpdateIssueWatch(ctx, w); err != nil { return nil, err } @@ -700,6 +748,8 @@ func (s *Service) publishNewJiraIssueEvent(ctx context.Context, w *IssueWatch, t WorkspaceID: w.WorkspaceID, WorkflowID: w.WorkflowID, WorkflowStepID: w.WorkflowStepID, + RepositoryID: w.RepositoryID, + BaseBranch: w.BaseBranch, AgentProfileID: w.AgentProfileID, ExecutorProfileID: w.ExecutorProfileID, Prompt: w.Prompt, @@ -726,6 +776,41 @@ const ( MaxIssueWatchPollInterval = 3600 ) +// resolveRepositoryBinding validates the watch's optional repository binding +// against its workspace and fills an empty base branch with the repository's +// default branch. An empty repositoryID clears the binding (and forces an empty +// base branch), preserving the historical repo-less behaviour. When no +// RepositoryLookup is wired (unit tests, early boot), the binding is accepted +// as-is and the default-branch fill is skipped. +func (s *Service) resolveRepositoryBinding(ctx context.Context, workspaceID, repositoryID, baseBranch string) (string, string, error) { + repositoryID = strings.TrimSpace(repositoryID) + baseBranch = strings.TrimSpace(baseBranch) + if repositoryID == "" { + return "", "", nil + } + // Reject a non-empty base branch that isn't a safe git ref before it can be + // persisted and copied into watcher-created tasks (then fail at worktree + // launch). Empty defers to the repo's default branch below. + if baseBranch != "" && !securityutil.IsValidBaseBranchRef(baseBranch) { + return "", "", fmt.Errorf("%w: base branch %q is not a valid git ref", ErrInvalidConfig, baseBranch) + } + rl := s.getRepositoryLookup() + if rl == nil { + return repositoryID, baseBranch, nil + } + repoWorkspace, defaultBranch, ok := rl.GetRepository(ctx, repositoryID) + if !ok { + return "", "", fmt.Errorf("%w: repository %q not found", ErrInvalidConfig, repositoryID) + } + if repoWorkspace != workspaceID { + return "", "", fmt.Errorf("%w: repository %q does not belong to this workspace", ErrInvalidConfig, repositoryID) + } + if baseBranch == "" { + baseBranch = defaultBranch + } + return repositoryID, baseBranch, nil +} + func validateIssueWatchCreate(req *CreateIssueWatchRequest) error { if req.WorkspaceID == "" { return fmt.Errorf("%w: workspaceId required", ErrInvalidConfig) @@ -776,6 +861,20 @@ func applyIssueWatchPatch(w *IssueWatch, req *UpdateIssueWatchRequest) { if req.WorkflowStepID != nil { w.WorkflowStepID = *req.WorkflowStepID } + // RepositoryID / BaseBranch are applied here; UpdateIssueWatch then runs them + // through resolveRepositoryBinding (workspace check + default-branch fill, or + // clear when empty). An empty RepositoryID unbinds the watch. Switching to a + // different repository without an explicit base branch resets the branch so + // the new repo's default is used instead of carrying the old repo's branch. + if req.RepositoryID != nil { + if *req.RepositoryID != w.RepositoryID && req.BaseBranch == nil { + w.BaseBranch = "" + } + w.RepositoryID = *req.RepositoryID + } + if req.BaseBranch != nil { + w.BaseBranch = *req.BaseBranch + } if req.JQL != nil { w.JQL = strings.TrimSpace(*req.JQL) } diff --git a/apps/backend/internal/jira/service_issue_watch_repository_test.go b/apps/backend/internal/jira/service_issue_watch_repository_test.go new file mode 100644 index 0000000000..22a0763c9a --- /dev/null +++ b/apps/backend/internal/jira/service_issue_watch_repository_test.go @@ -0,0 +1,193 @@ +package jira + +import ( + "context" + "errors" + "testing" +) + +// fakeRepoLookup is a static RepositoryLookup for repository-binding tests. +type fakeRepoLookup struct { + workspaceID string + defaultBranch string + ok bool +} + +func (f fakeRepoLookup) GetRepository(_ context.Context, _ string) (string, string, bool) { + return f.workspaceID, f.defaultBranch, f.ok +} + +func TestService_CreateIssueWatch_RepositoryBinding(t *testing.T) { + ctx := context.Background() + baseReq := func() *CreateIssueWatchRequest { + return &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf", + WorkflowStepID: "step", + JQL: `project = PROJ`, + } + } + + t.Run("default branch filled from repo", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "repo-1" || w.BaseBranch != "main" { + t.Fatalf("expected repo-1@main, got repo=%q branch=%q", w.RepositoryID, w.BaseBranch) + } + }) + + t.Run("explicit branch preserved", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + req.BaseBranch = "release/v2" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.BaseBranch != "release/v2" { + t.Fatalf("explicit branch overwritten: %q", w.BaseBranch) + } + }) + + t.Run("cross-workspace repo rejected", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-other", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + if _, err := f.svc.CreateIssueWatch(ctx, req); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for cross-workspace repo, got %v", err) + } + }) + + t.Run("missing repo rejected", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{ok: false}) + req := baseReq() + req.RepositoryID = "ghost" + if _, err := f.svc.CreateIssueWatch(ctx, req); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for missing repo, got %v", err) + } + }) + + t.Run("unbound skips lookup and clears branch", func(t *testing.T) { + f := newSvcFixture(t) // no RepositoryLookup wired + req := baseReq() + req.BaseBranch = "ignored-without-repo" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "" || w.BaseBranch != "" { + t.Fatalf("unbound watch must stay empty, got repo=%q branch=%q", w.RepositoryID, w.BaseBranch) + } + }) +} + +func TestService_UpdateIssueWatch_RepositoryBinding(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + + w, err := f.svc.CreateIssueWatch(ctx, &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf", + WorkflowStepID: "step", + JQL: "project = PROJ", + }) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "" { + t.Fatalf("expected unbound on create, got %q", w.RepositoryID) + } + + repoID := "repo-1" + updated, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repoID}) + if err != nil { + t.Fatalf("update bind: %v", err) + } + if updated.RepositoryID != "repo-1" || updated.BaseBranch != "main" { + t.Fatalf("expected repo-1@main after update, got repo=%q branch=%q", updated.RepositoryID, updated.BaseBranch) + } + + empty := "" + cleared, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &empty}) + if err != nil { + t.Fatalf("update unbind: %v", err) + } + if cleared.RepositoryID != "" || cleared.BaseBranch != "" { + t.Fatalf("expected cleared after update, got repo=%q branch=%q", cleared.RepositoryID, cleared.BaseBranch) + } +} + +func TestService_UpdateIssueWatch_RebindAndDeletedRepo(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + + w, err := f.svc.CreateIssueWatch(ctx, &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf", WorkflowStepID: "step", JQL: "project = PROJ", + }) + if err != nil { + t.Fatalf("create: %v", err) + } + repo1 := "repo-1" + if _, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repo1}); err != nil { + t.Fatalf("bind: %v", err) + } + + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "develop", ok: true}) + repo2 := "repo-2" + rebound, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repo2}) + if err != nil { + t.Fatalf("rebind: %v", err) + } + if rebound.RepositoryID != "repo-2" || rebound.BaseBranch != "develop" { + t.Fatalf("rebind should reset branch to new default, got repo=%q branch=%q", rebound.RepositoryID, rebound.BaseBranch) + } + + f.svc.SetRepositoryLookup(fakeRepoLookup{ok: false}) + prompt := "updated prompt" + edited, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{Prompt: &prompt}) + if err != nil { + t.Fatalf("unrelated edit blocked by deleted bound repo: %v", err) + } + if edited.Prompt != "updated prompt" || edited.RepositoryID != "repo-2" || edited.BaseBranch != "develop" { + t.Fatalf("expected prompt updated + binding preserved, got prompt=%q repo=%q branch=%q", edited.Prompt, edited.RepositoryID, edited.BaseBranch) + } +} + +func TestService_IssueWatch_RejectsInvalidBaseBranch(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + base := func() *CreateIssueWatchRequest { + return &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf", WorkflowStepID: "step", + JQL: "project = PROJ", RepositoryID: "repo-1", + } + } + + bad := base() + bad.BaseBranch = "bad..ref" + if _, err := f.svc.CreateIssueWatch(ctx, bad); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for invalid base branch on create, got %v", err) + } + + w, err := f.svc.CreateIssueWatch(ctx, base()) + if err != nil { + t.Fatalf("create: %v", err) + } + badRef := "bad..ref" + if _, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{BaseBranch: &badRef}); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for invalid base branch on update, got %v", err) + } +} diff --git a/apps/backend/internal/jira/store.go b/apps/backend/internal/jira/store.go index 5b72311596..d04d4b5e04 100644 --- a/apps/backend/internal/jira/store.go +++ b/apps/backend/internal/jira/store.go @@ -61,6 +61,11 @@ const createTablesSQL = ` workspace_id TEXT NOT NULL, workflow_id TEXT NOT NULL, workflow_step_id TEXT NOT NULL, + -- Optional repository binding. Empty = unbound (repo-less task, the + -- historical behaviour). When set, watcher-created tasks launch in an + -- isolated worktree of this repo cut from base_branch. + repository_id TEXT NOT NULL DEFAULT '', + base_branch TEXT NOT NULL DEFAULT '', jql TEXT NOT NULL, agent_profile_id TEXT NOT NULL DEFAULT '', executor_profile_id TEXT NOT NULL DEFAULT '', @@ -113,6 +118,35 @@ func (s *Store) initSchema() error { if err := s.addIssueWatchLastErrorColumns(); err != nil { return err } + if err := s.addIssueWatchRepositoryColumns(); err != nil { + return err + } + return nil +} + +// addIssueWatchRepositoryColumns brings older databases up to the current +// schema by appending repository_id / base_branch to jira_issue_watches when +// missing. Both backfill to ” (unbound), so existing repo-less watches keep +// their behaviour. Idempotent — column lookup before each ALTER avoids the +// "duplicate column name" error on fresh installs that already have them. +func (s *Store) addIssueWatchRepositoryColumns() error { + cols, err := s.tableColumns("jira_issue_watches") + if err != nil { + return err + } + if len(cols) == 0 { + return nil + } + if _, ok := cols["repository_id"]; !ok { + if _, err := s.db.Exec(`ALTER TABLE jira_issue_watches ADD COLUMN repository_id TEXT NOT NULL DEFAULT ''`); err != nil { + return fmt.Errorf("add repository_id column: %w", err) + } + } + if _, ok := cols["base_branch"]; !ok { + if _, err := s.db.Exec(`ALTER TABLE jira_issue_watches ADD COLUMN base_branch TEXT NOT NULL DEFAULT ''`); err != nil { + return fmt.Errorf("add base_branch column: %w", err) + } + } return nil } @@ -397,7 +431,8 @@ func (s *Store) UpdateAuthHealth(ctx context.Context, ok bool, errMsg string, ch // issueWatchInsertColumns / issueWatchSelectColumns split insert vs read so // the SELECT path can wrap nullable last_error in COALESCE (older databases // pre-self-heal migration may return NULL). -const issueWatchInsertColumns = `id, workspace_id, workflow_id, workflow_step_id, jql, +const issueWatchInsertColumns = `id, workspace_id, workflow_id, workflow_step_id, + repository_id, base_branch, jql, agent_profile_id, executor_profile_id, prompt, enabled, poll_interval_seconds, max_inflight_tasks, last_polled_at, last_error, last_error_at, @@ -413,7 +448,8 @@ func nullableInt(v *int) interface{} { return *v } -const issueWatchSelectColumns = `id, workspace_id, workflow_id, workflow_step_id, jql, +const issueWatchSelectColumns = `id, workspace_id, workflow_id, workflow_step_id, + COALESCE(repository_id, '') AS repository_id, COALESCE(base_branch, '') AS base_branch, jql, agent_profile_id, executor_profile_id, prompt, enabled, poll_interval_seconds, max_inflight_tasks, last_polled_at, COALESCE(last_error, '') AS last_error, last_error_at, @@ -433,8 +469,9 @@ func (s *Store) CreateIssueWatch(ctx context.Context, w *IssueWatch) error { } _, err := s.db.ExecContext(ctx, ` INSERT INTO jira_issue_watches (`+issueWatchInsertColumns+`) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, - w.ID, w.WorkspaceID, w.WorkflowID, w.WorkflowStepID, w.JQL, + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + w.ID, w.WorkspaceID, w.WorkflowID, w.WorkflowStepID, + w.RepositoryID, w.BaseBranch, w.JQL, w.AgentProfileID, w.ExecutorProfileID, w.Prompt, w.Enabled, w.PollIntervalSeconds, nullableInt(w.MaxInflightTasks), w.LastPolledAt, w.LastError, w.LastErrorAt, @@ -505,12 +542,14 @@ func (s *Store) UpdateIssueWatch(ctx context.Context, w *IssueWatch) error { w.PollIntervalSeconds = DefaultIssueWatchPollInterval } _, err := s.db.ExecContext(ctx, ` - UPDATE jira_issue_watches SET workflow_id = ?, workflow_step_id = ?, jql = ?, + UPDATE jira_issue_watches SET workflow_id = ?, workflow_step_id = ?, + repository_id = ?, base_branch = ?, jql = ?, agent_profile_id = ?, executor_profile_id = ?, prompt = ?, enabled = ?, poll_interval_seconds = ?, max_inflight_tasks = ?, last_polled_at = ?, updated_at = ? WHERE id = ?`, - w.WorkflowID, w.WorkflowStepID, w.JQL, + w.WorkflowID, w.WorkflowStepID, + w.RepositoryID, w.BaseBranch, w.JQL, w.AgentProfileID, w.ExecutorProfileID, w.Prompt, w.Enabled, w.PollIntervalSeconds, nullableInt(w.MaxInflightTasks), w.LastPolledAt, w.UpdatedAt, w.ID) diff --git a/apps/backend/internal/jira/store_issue_watch_repository_test.go b/apps/backend/internal/jira/store_issue_watch_repository_test.go new file mode 100644 index 0000000000..be561d44ad --- /dev/null +++ b/apps/backend/internal/jira/store_issue_watch_repository_test.go @@ -0,0 +1,131 @@ +package jira + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" +) + +func TestStore_IssueWatch_RepositoryBindingRoundTrip(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + + w := newTestIssueWatch("ws-1") + w.RepositoryID = "repo-123" + w.BaseBranch = "develop" + if err := store.CreateIssueWatch(ctx, w); err != nil { + t.Fatalf("create: %v", err) + } + + got, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.RepositoryID != "repo-123" || got.BaseBranch != "develop" { + t.Fatalf("repository binding lost on round-trip: repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } + + // Update clears the binding (unbind path). + got.RepositoryID = "" + got.BaseBranch = "" + if err := store.UpdateIssueWatch(ctx, got); err != nil { + t.Fatalf("update: %v", err) + } + after, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get after update: %v", err) + } + if after.RepositoryID != "" || after.BaseBranch != "" { + t.Fatalf("expected binding cleared, got repo=%q branch=%q", after.RepositoryID, after.BaseBranch) + } +} + +// TestStore_IssueWatch_DefaultsUnbound pins the backward-compat invariant: a +// watch created without a repository binding reads back as unbound (empty), +// preserving the repo-less behaviour. +func TestStore_IssueWatch_DefaultsUnbound(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + + w := newTestIssueWatch("ws-1") + if err := store.CreateIssueWatch(ctx, w); err != nil { + t.Fatalf("create: %v", err) + } + got, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.RepositoryID != "" || got.BaseBranch != "" { + t.Fatalf("unbound watch should have empty binding, got repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } +} + +// preRepoIssueWatchSchema is the jira_issue_watches DDL from before the +// repository-binding columns were introduced (but after the max_inflight / +// last_error migrations). Used to exercise addIssueWatchRepositoryColumns on an +// older database. +const preRepoIssueWatchSchema = ` + CREATE TABLE jira_issue_watches ( + id TEXT PRIMARY KEY, + workspace_id TEXT NOT NULL, + workflow_id TEXT NOT NULL, + workflow_step_id TEXT NOT NULL, + jql TEXT NOT NULL, + agent_profile_id TEXT NOT NULL DEFAULT '', + executor_profile_id TEXT NOT NULL DEFAULT '', + prompt TEXT NOT NULL DEFAULT '', + enabled BOOLEAN NOT NULL DEFAULT 1, + poll_interval_seconds INTEGER NOT NULL DEFAULT 300, + max_inflight_tasks INTEGER DEFAULT 5, + last_polled_at DATETIME, + last_error TEXT NOT NULL DEFAULT '', + last_error_at DATETIME, + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + );` + +func TestStore_IssueWatch_AddRepositoryColumns_Migration(t *testing.T) { + ctx := context.Background() + raw, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open: %v", err) + } + raw.SetMaxOpenConns(1) + t.Cleanup(func() { _ = raw.Close() }) + + if _, err := raw.Exec(preRepoIssueWatchSchema); err != nil { + t.Fatalf("legacy schema: %v", err) + } + now := time.Now().UTC() + id := uuid.New().String() + if _, err := raw.Exec(`INSERT INTO jira_issue_watches + (id, workspace_id, workflow_id, workflow_step_id, jql, created_at, updated_at) + VALUES (?, 'ws-1', 'wf-1', 'step-1', 'project = PROJ', ?, ?)`, id, now, now); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + + store, err := NewStore(raw, raw) + if err != nil { + t.Fatalf("NewStore on legacy table: %v", err) + } + + got, err := store.GetIssueWatch(ctx, id) + if err != nil { + t.Fatalf("get migrated row: %v", err) + } + if got == nil { + t.Fatal("expected migrated row, got nil") + } + if got.RepositoryID != "" || got.BaseBranch != "" { + t.Fatalf("existing row should backfill to unbound, got repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } + + // Idempotent: running initSchema again must not fail on duplicate columns. + if err := store.initSchema(); err != nil { + t.Fatalf("second initSchema (idempotency): %v", err) + } +} diff --git a/apps/backend/internal/linear/models.go b/apps/backend/internal/linear/models.go index 4beb7d7dcf..bcafdbaa92 100644 --- a/apps/backend/internal/linear/models.go +++ b/apps/backend/internal/linear/models.go @@ -188,10 +188,20 @@ const DefaultIssueWatchPollInterval = 300 // As with Jira, Linear issues have no repository affinity — the target // workflow step's defaults determine where the resulting task runs. type IssueWatch struct { - ID string `json:"id" db:"id"` - WorkspaceID string `json:"workspaceId" db:"workspace_id"` - WorkflowID string `json:"workflowId" db:"workflow_id"` - WorkflowStepID string `json:"workflowStepId" db:"workflow_step_id"` + ID string `json:"id" db:"id"` + WorkspaceID string `json:"workspaceId" db:"workspace_id"` + WorkflowID string `json:"workflowId" db:"workflow_id"` + WorkflowStepID string `json:"workflowStepId" db:"workflow_step_id"` + // RepositoryID optionally binds watcher-created tasks to a repository so the + // agent launches in an isolated worktree of that repo instead of a blank + // scratch checkout. Empty = unbound, which preserves the historical + // repo-less behaviour. When set, the resulting task carries a single + // (repository_id, base_branch) pair. + RepositoryID string `json:"repositoryId" db:"repository_id"` + // BaseBranch is the branch the per-task worktree is cut from. Empty defaults + // to the repository's default branch (resolved at create/update time). + // Meaningful only when RepositoryID is set. + BaseBranch string `json:"baseBranch" db:"base_branch"` Filter SearchFilter `json:"filter"` AgentProfileID string `json:"agentProfileId" db:"agent_profile_id"` ExecutorProfileID string `json:"executorProfileId" db:"executor_profile_id"` @@ -230,10 +240,15 @@ type IssueWatchTask struct { // issue matching a watch that has no existing dedup row. The orchestrator // consumes this to create (and optionally auto-start) a Kandev task. type NewLinearIssueEvent struct { - IssueWatchID string `json:"issueWatchId"` - WorkspaceID string `json:"workspaceId"` - WorkflowID string `json:"workflowId"` - WorkflowStepID string `json:"workflowStepId"` + IssueWatchID string `json:"issueWatchId"` + WorkspaceID string `json:"workspaceId"` + WorkflowID string `json:"workflowId"` + WorkflowStepID string `json:"workflowStepId"` + // RepositoryID / BaseBranch carry the watch's optional repository binding so + // the orchestrator source can populate IssueTaskRequest.Repositories without + // reloading the watch row. Empty RepositoryID = unbound (repo-less task). + RepositoryID string `json:"repositoryId,omitempty"` + BaseBranch string `json:"baseBranch,omitempty"` AgentProfileID string `json:"agentProfileId"` ExecutorProfileID string `json:"executorProfileId"` Prompt string `json:"prompt"` @@ -249,6 +264,8 @@ type CreateIssueWatchRequest struct { WorkspaceID string `json:"workspaceId"` WorkflowID string `json:"workflowId"` WorkflowStepID string `json:"workflowStepId"` + RepositoryID string `json:"repositoryId"` + BaseBranch string `json:"baseBranch"` Filter SearchFilter `json:"filter"` AgentProfileID string `json:"agentProfileId"` ExecutorProfileID string `json:"executorProfileId"` @@ -265,6 +282,8 @@ type CreateIssueWatchRequest struct { type UpdateIssueWatchRequest struct { WorkflowID *string `json:"workflowId,omitempty"` WorkflowStepID *string `json:"workflowStepId,omitempty"` + RepositoryID *string `json:"repositoryId,omitempty"` + BaseBranch *string `json:"baseBranch,omitempty"` Filter *SearchFilter `json:"filter,omitempty"` AgentProfileID *string `json:"agentProfileId,omitempty"` ExecutorProfileID *string `json:"executorProfileId,omitempty"` diff --git a/apps/backend/internal/linear/service.go b/apps/backend/internal/linear/service.go index 4495a393dd..403759935e 100644 --- a/apps/backend/internal/linear/service.go +++ b/apps/backend/internal/linear/service.go @@ -22,6 +22,15 @@ type SecretStore interface { Exists(ctx context.Context, id string) (bool, error) } +// RepositoryLookup is the subset of the task service used to validate a watch's +// optional repository binding (workspace ownership + default-branch fill). +// Wired post-construction via SetRepositoryLookup to avoid an import cycle with +// the task service. ok is false when the repository does not exist or has been +// soft-deleted. +type RepositoryLookup interface { + GetRepository(ctx context.Context, id string) (workspaceID, defaultBranch string, ok bool) +} + // Service orchestrates Linear config storage, the cached client, and the // fetch/transition operations used by the WebSocket + HTTP handlers. type Service struct { @@ -37,6 +46,10 @@ type Service struct { // Wired post-construction via SetTaskDeleter to avoid an import cycle // with the task service. taskDeleter watchreset.TaskDeleter + // repoLookup validates an optional repository binding on create/update. + // Wired post-construction via SetRepositoryLookup. When nil (e.g. unit + // tests), the binding is accepted as-is and default-branch fill is skipped. + repoLookup RepositoryLookup // mockClient is non-nil only when Provide built the service with a MockClient // (KANDEV_MOCK_LINEAR=true). Exposed via MockClient() so the e2e control // routes can drive the same instance the clientFn returns. @@ -52,6 +65,21 @@ func (s *Service) SetTaskDeleter(td watchreset.TaskDeleter) { s.taskDeleter = td } +// SetRepositoryLookup wires the repository validator used by CreateIssueWatch / +// UpdateIssueWatch. Optional — when unset, a repository binding is persisted +// as-is without workspace/default-branch resolution. +func (s *Service) SetRepositoryLookup(rl RepositoryLookup) { + s.mu.Lock() + defer s.mu.Unlock() + s.repoLookup = rl +} + +func (s *Service) getRepositoryLookup() RepositoryLookup { + s.mu.Lock() + defer s.mu.Unlock() + return s.repoLookup +} + // MockClient returns the shared mock client when the service was built in mock // mode, or nil for production builds. func (s *Service) MockClient() *MockClient { diff --git a/apps/backend/internal/linear/service_issue_watch.go b/apps/backend/internal/linear/service_issue_watch.go index 93c648fd69..724d79b7e3 100644 --- a/apps/backend/internal/linear/service_issue_watch.go +++ b/apps/backend/internal/linear/service_issue_watch.go @@ -10,6 +10,7 @@ import ( "go.uber.org/zap" + "github.com/kandev/kandev/internal/common/securityutil" "github.com/kandev/kandev/internal/events" "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/watchreset" @@ -32,10 +33,16 @@ func (s *Service) CreateIssueWatch(ctx context.Context, req *CreateIssueWatchReq if err := validateIssueWatchCreate(req); err != nil { return nil, err } + repositoryID, baseBranch, err := s.resolveRepositoryBinding(ctx, req.WorkspaceID, req.RepositoryID, req.BaseBranch) + if err != nil { + return nil, err + } w := &IssueWatch{ WorkspaceID: req.WorkspaceID, WorkflowID: req.WorkflowID, WorkflowStepID: req.WorkflowStepID, + RepositoryID: repositoryID, + BaseBranch: baseBranch, Filter: normalizeFilter(req.Filter), AgentProfileID: req.AgentProfileID, ExecutorProfileID: req.ExecutorProfileID, @@ -82,6 +89,7 @@ func (s *Service) UpdateIssueWatch(ctx context.Context, id string, req *UpdateIs if err != nil { return nil, err } + prevRepositoryID, prevBaseBranch := w.RepositoryID, w.BaseBranch applyIssueWatchPatch(w, req) if filterIsEmpty(w.Filter) { return nil, fmt.Errorf("%w: filter must specify at least one of query, teamKey, stateIds, assigned, priorities, labelIds, creatorId, or estimate range", ErrInvalidConfig) @@ -98,6 +106,18 @@ func (s *Service) UpdateIssueWatch(ctx context.Context, id string, req *UpdateIs if err := validateMaxInflightTasks(w.MaxInflightTasks); err != nil { return nil, err } + // Only validate/resolve the binding when its value actually changed. The + // dialog re-sends repositoryId/baseBranch on every PATCH, and an unchanged + // binding whose repo was since soft-deleted must not block edits to other + // fields (prompt, filter, …). + if w.RepositoryID != prevRepositoryID || w.BaseBranch != prevBaseBranch { + repositoryID, baseBranch, err := s.resolveRepositoryBinding(ctx, w.WorkspaceID, w.RepositoryID, w.BaseBranch) + if err != nil { + return nil, err + } + w.RepositoryID = repositoryID + w.BaseBranch = baseBranch + } if err := s.store.UpdateIssueWatch(ctx, w); err != nil { return nil, err } @@ -215,6 +235,8 @@ func (s *Service) publishNewLinearIssueEvent(ctx context.Context, w *IssueWatch, WorkspaceID: w.WorkspaceID, WorkflowID: w.WorkflowID, WorkflowStepID: w.WorkflowStepID, + RepositoryID: w.RepositoryID, + BaseBranch: w.BaseBranch, AgentProfileID: w.AgentProfileID, ExecutorProfileID: w.ExecutorProfileID, Prompt: w.Prompt, @@ -239,6 +261,41 @@ const ( MaxIssueWatchPollInterval = 3600 ) +// resolveRepositoryBinding validates the watch's optional repository binding +// against its workspace and fills an empty base branch with the repository's +// default branch. An empty repositoryID clears the binding (and forces an empty +// base branch), preserving the historical repo-less behaviour. When no +// RepositoryLookup is wired (unit tests, early boot), the binding is accepted +// as-is and the default-branch fill is skipped. +func (s *Service) resolveRepositoryBinding(ctx context.Context, workspaceID, repositoryID, baseBranch string) (string, string, error) { + repositoryID = strings.TrimSpace(repositoryID) + baseBranch = strings.TrimSpace(baseBranch) + if repositoryID == "" { + return "", "", nil + } + // Reject a non-empty base branch that isn't a safe git ref before it can be + // persisted and copied into watcher-created tasks (then fail at worktree + // launch). Empty defers to the repo's default branch below. + if baseBranch != "" && !securityutil.IsValidBaseBranchRef(baseBranch) { + return "", "", fmt.Errorf("%w: base branch %q is not a valid git ref", ErrInvalidConfig, baseBranch) + } + rl := s.getRepositoryLookup() + if rl == nil { + return repositoryID, baseBranch, nil + } + repoWorkspace, defaultBranch, ok := rl.GetRepository(ctx, repositoryID) + if !ok { + return "", "", fmt.Errorf("%w: repository %q not found", ErrInvalidConfig, repositoryID) + } + if repoWorkspace != workspaceID { + return "", "", fmt.Errorf("%w: repository %q does not belong to this workspace", ErrInvalidConfig, repositoryID) + } + if baseBranch == "" { + baseBranch = defaultBranch + } + return repositoryID, baseBranch, nil +} + func validateIssueWatchCreate(req *CreateIssueWatchRequest) error { if req.WorkspaceID == "" { return fmt.Errorf("%w: workspaceId required", ErrInvalidConfig) @@ -371,6 +428,20 @@ func applyIssueWatchPatch(w *IssueWatch, req *UpdateIssueWatchRequest) { if req.WorkflowStepID != nil { w.WorkflowStepID = *req.WorkflowStepID } + // RepositoryID / BaseBranch are applied here; UpdateIssueWatch then runs them + // through resolveRepositoryBinding (workspace check + default-branch fill, or + // clear when empty). An empty RepositoryID unbinds the watch. Switching to a + // different repository without an explicit base branch resets the branch so + // the new repo's default is used instead of carrying the old repo's branch. + if req.RepositoryID != nil { + if *req.RepositoryID != w.RepositoryID && req.BaseBranch == nil { + w.BaseBranch = "" + } + w.RepositoryID = *req.RepositoryID + } + if req.BaseBranch != nil { + w.BaseBranch = *req.BaseBranch + } if req.Filter != nil { w.Filter = normalizeFilter(*req.Filter) } diff --git a/apps/backend/internal/linear/service_issue_watch_test.go b/apps/backend/internal/linear/service_issue_watch_test.go index b6f0f1032e..398c16462d 100644 --- a/apps/backend/internal/linear/service_issue_watch_test.go +++ b/apps/backend/internal/linear/service_issue_watch_test.go @@ -36,6 +36,91 @@ func TestService_CreateIssueWatch_AcceptsRichFilters(t *testing.T) { } } +// fakeRepoLookup is a static RepositoryLookup for repository-binding tests. +type fakeRepoLookup struct { + workspaceID string + defaultBranch string + ok bool +} + +func (f fakeRepoLookup) GetRepository(_ context.Context, _ string) (string, string, bool) { + return f.workspaceID, f.defaultBranch, f.ok +} + +func TestService_CreateIssueWatch_RepositoryBinding(t *testing.T) { + ctx := context.Background() + baseReq := func() *CreateIssueWatchRequest { + return &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf", + WorkflowStepID: "step", + Filter: SearchFilter{TeamKey: "ENG"}, + } + } + + t.Run("default branch filled from repo", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "repo-1" || w.BaseBranch != "main" { + t.Fatalf("expected repo-1@main, got repo=%q branch=%q", w.RepositoryID, w.BaseBranch) + } + }) + + t.Run("explicit branch preserved", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + req.BaseBranch = "release/v2" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.BaseBranch != "release/v2" { + t.Fatalf("explicit branch overwritten: %q", w.BaseBranch) + } + }) + + t.Run("cross-workspace repo rejected", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-other", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + if _, err := f.svc.CreateIssueWatch(ctx, req); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for cross-workspace repo, got %v", err) + } + }) + + t.Run("missing repo rejected", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{ok: false}) + req := baseReq() + req.RepositoryID = "ghost" + if _, err := f.svc.CreateIssueWatch(ctx, req); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for missing repo, got %v", err) + } + }) + + t.Run("unbound skips lookup and clears branch", func(t *testing.T) { + f := newSvcFixture(t) // no RepositoryLookup wired + req := baseReq() + req.BaseBranch = "ignored-without-repo" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "" || w.BaseBranch != "" { + t.Fatalf("unbound watch must stay empty, got repo=%q branch=%q", w.RepositoryID, w.BaseBranch) + } + }) +} + // withSearchResults returns a fakeClient that always returns the given issues // for SearchIssues, ignoring the filter. func (c *fakeClient) withSearchResults(issues []LinearIssue) *fakeClient { @@ -371,3 +456,111 @@ func TestService_CheckIssueWatch_StampsLastPolledOnError(t *testing.T) { t.Error("expected last_polled_at stamped even on search failure (liveness signal)") } } + +func TestService_UpdateIssueWatch_RepositoryBinding(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + + w, err := f.svc.CreateIssueWatch(ctx, &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf", + WorkflowStepID: "step", + Filter: SearchFilter{TeamKey: "ENG"}, + }) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "" { + t.Fatalf("expected unbound on create, got %q", w.RepositoryID) + } + + // Bind an existing watch via update — the edit path must persist it. + repoID := "repo-1" + updated, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repoID}) + if err != nil { + t.Fatalf("update bind: %v", err) + } + if updated.RepositoryID != "repo-1" || updated.BaseBranch != "main" { + t.Fatalf("expected repo-1@main after update, got repo=%q branch=%q", updated.RepositoryID, updated.BaseBranch) + } + + // Unbind via update (empty string). + empty := "" + cleared, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &empty}) + if err != nil { + t.Fatalf("update unbind: %v", err) + } + if cleared.RepositoryID != "" || cleared.BaseBranch != "" { + t.Fatalf("expected cleared after update, got repo=%q branch=%q", cleared.RepositoryID, cleared.BaseBranch) + } +} + +func TestService_UpdateIssueWatch_RebindAndDeletedRepo(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + + w, err := f.svc.CreateIssueWatch(ctx, &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf", WorkflowStepID: "step", + Filter: SearchFilter{TeamKey: "ENG"}, + }) + if err != nil { + t.Fatalf("create: %v", err) + } + repo1 := "repo-1" + if _, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repo1}); err != nil { + t.Fatalf("bind: %v", err) + } + + // Rebind to a different repo with no explicit branch → branch resets to the + // new repo's default ("develop"), not the previous repo's "main". + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "develop", ok: true}) + repo2 := "repo-2" + rebound, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repo2}) + if err != nil { + t.Fatalf("rebind: %v", err) + } + if rebound.RepositoryID != "repo-2" || rebound.BaseBranch != "develop" { + t.Fatalf("rebind should reset branch to new default, got repo=%q branch=%q", rebound.RepositoryID, rebound.BaseBranch) + } + + // Bound repo is now soft-deleted (lookup fails). Editing an unrelated field + // must still succeed — the binding isn't part of this PATCH. + f.svc.SetRepositoryLookup(fakeRepoLookup{ok: false}) + prompt := "updated prompt" + edited, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{Prompt: &prompt}) + if err != nil { + t.Fatalf("unrelated edit blocked by deleted bound repo: %v", err) + } + if edited.Prompt != "updated prompt" || edited.RepositoryID != "repo-2" || edited.BaseBranch != "develop" { + t.Fatalf("expected prompt updated + binding preserved, got prompt=%q repo=%q branch=%q", edited.Prompt, edited.RepositoryID, edited.BaseBranch) + } +} + +func TestService_IssueWatch_RejectsInvalidBaseBranch(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + base := func() *CreateIssueWatchRequest { + return &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf", WorkflowStepID: "step", + Filter: SearchFilter{TeamKey: "ENG"}, RepositoryID: "repo-1", + } + } + + bad := base() + bad.BaseBranch = "bad..ref" + if _, err := f.svc.CreateIssueWatch(ctx, bad); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for invalid base branch on create, got %v", err) + } + + w, err := f.svc.CreateIssueWatch(ctx, base()) // valid: empty branch -> default + if err != nil { + t.Fatalf("create: %v", err) + } + badRef := "bad..ref" + if _, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{BaseBranch: &badRef}); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for invalid base branch on update, got %v", err) + } +} diff --git a/apps/backend/internal/linear/store.go b/apps/backend/internal/linear/store.go index f90dd596d6..d671dcc4b2 100644 --- a/apps/backend/internal/linear/store.go +++ b/apps/backend/internal/linear/store.go @@ -57,6 +57,11 @@ const createTablesSQL = ` workspace_id TEXT NOT NULL, workflow_id TEXT NOT NULL, workflow_step_id TEXT NOT NULL, + -- Optional repository binding. Empty = unbound (repo-less task, the + -- historical behaviour). When set, watcher-created tasks launch in an + -- isolated worktree of this repo cut from base_branch. + repository_id TEXT NOT NULL DEFAULT '', + base_branch TEXT NOT NULL DEFAULT '', filter_json TEXT NOT NULL DEFAULT '{}', agent_profile_id TEXT NOT NULL DEFAULT '', executor_profile_id TEXT NOT NULL DEFAULT '', @@ -104,6 +109,36 @@ func (s *Store) initSchema() error { if err := s.addIssueWatchLastErrorColumns(); err != nil { return err } + if err := s.addIssueWatchRepositoryColumns(); err != nil { + return err + } + return nil +} + +// addIssueWatchRepositoryColumns brings older databases up to the current +// schema by appending repository_id / base_branch to linear_issue_watches when +// missing. Both backfill to ” (unbound), so existing repo-less watches keep +// their behaviour. Fresh installs hit the column-already-present branch since +// createTablesSQL declares both columns. Idempotent — column lookup before each +// ALTER avoids the "duplicate column name" error. +func (s *Store) addIssueWatchRepositoryColumns() error { + cols, err := s.tableColumns("linear_issue_watches") + if err != nil { + return err + } + if len(cols) == 0 { + return nil + } + if _, ok := cols["repository_id"]; !ok { + if _, err := s.db.Exec(`ALTER TABLE linear_issue_watches ADD COLUMN repository_id TEXT NOT NULL DEFAULT ''`); err != nil { + return fmt.Errorf("add repository_id column: %w", err) + } + } + if _, ok := cols["base_branch"]; !ok { + if _, err := s.db.Exec(`ALTER TABLE linear_issue_watches ADD COLUMN base_branch TEXT NOT NULL DEFAULT ''`); err != nil { + return fmt.Errorf("add base_branch column: %w", err) + } + } return nil } diff --git a/apps/backend/internal/linear/store_issue_watch.go b/apps/backend/internal/linear/store_issue_watch.go index ae9b4b58c0..a3d6149ccc 100644 --- a/apps/backend/internal/linear/store_issue_watch.go +++ b/apps/backend/internal/linear/store_issue_watch.go @@ -19,6 +19,8 @@ type issueWatchRow struct { WorkspaceID string `db:"workspace_id"` WorkflowID string `db:"workflow_id"` WorkflowStepID string `db:"workflow_step_id"` + RepositoryID string `db:"repository_id"` + BaseBranch string `db:"base_branch"` FilterJSON string `db:"filter_json"` AgentProfileID string `db:"agent_profile_id"` ExecutorProfileID string `db:"executor_profile_id"` @@ -50,6 +52,8 @@ func (r *issueWatchRow) toIssueWatch() (*IssueWatch, error) { WorkspaceID: r.WorkspaceID, WorkflowID: r.WorkflowID, WorkflowStepID: r.WorkflowStepID, + RepositoryID: r.RepositoryID, + BaseBranch: r.BaseBranch, Filter: filter, AgentProfileID: r.AgentProfileID, ExecutorProfileID: r.ExecutorProfileID, @@ -77,13 +81,15 @@ func encodeFilter(f SearchFilter) (string, error) { // SELECTs use issueWatchSelectColumns which wraps the nullable last_error in // COALESCE so older databases (pre-self-heal migration) read back as empty // strings rather than NULL. -const issueWatchInsertColumns = `id, workspace_id, workflow_id, workflow_step_id, filter_json, +const issueWatchInsertColumns = `id, workspace_id, workflow_id, workflow_step_id, + repository_id, base_branch, filter_json, agent_profile_id, executor_profile_id, prompt, enabled, poll_interval_seconds, max_inflight_tasks, last_polled_at, last_error, last_error_at, created_at, updated_at` -const issueWatchSelectColumns = `id, workspace_id, workflow_id, workflow_step_id, filter_json, +const issueWatchSelectColumns = `id, workspace_id, workflow_id, workflow_step_id, + COALESCE(repository_id, '') AS repository_id, COALESCE(base_branch, '') AS base_branch, filter_json, agent_profile_id, executor_profile_id, prompt, enabled, poll_interval_seconds, max_inflight_tasks, last_polled_at, COALESCE(last_error, '') AS last_error, last_error_at, @@ -107,8 +113,9 @@ func (s *Store) CreateIssueWatch(ctx context.Context, w *IssueWatch) error { } _, err = s.db.ExecContext(ctx, ` INSERT INTO linear_issue_watches (`+issueWatchInsertColumns+`) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, - w.ID, w.WorkspaceID, w.WorkflowID, w.WorkflowStepID, filterJSON, + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + w.ID, w.WorkspaceID, w.WorkflowID, w.WorkflowStepID, + w.RepositoryID, w.BaseBranch, filterJSON, w.AgentProfileID, w.ExecutorProfileID, w.Prompt, w.Enabled, w.PollIntervalSeconds, nullableInt(w.MaxInflightTasks), w.LastPolledAt, w.LastError, w.LastErrorAt, @@ -201,12 +208,14 @@ func (s *Store) UpdateIssueWatch(ctx context.Context, w *IssueWatch) error { return err } _, err = s.db.ExecContext(ctx, ` - UPDATE linear_issue_watches SET workflow_id = ?, workflow_step_id = ?, filter_json = ?, + UPDATE linear_issue_watches SET workflow_id = ?, workflow_step_id = ?, + repository_id = ?, base_branch = ?, filter_json = ?, agent_profile_id = ?, executor_profile_id = ?, prompt = ?, enabled = ?, poll_interval_seconds = ?, max_inflight_tasks = ?, last_polled_at = ?, updated_at = ? WHERE id = ?`, - w.WorkflowID, w.WorkflowStepID, filterJSON, + w.WorkflowID, w.WorkflowStepID, + w.RepositoryID, w.BaseBranch, filterJSON, w.AgentProfileID, w.ExecutorProfileID, w.Prompt, w.Enabled, w.PollIntervalSeconds, nullableInt(w.MaxInflightTasks), w.LastPolledAt, w.UpdatedAt, w.ID) diff --git a/apps/backend/internal/linear/store_issue_watch_repository_test.go b/apps/backend/internal/linear/store_issue_watch_repository_test.go new file mode 100644 index 0000000000..887c25e845 --- /dev/null +++ b/apps/backend/internal/linear/store_issue_watch_repository_test.go @@ -0,0 +1,133 @@ +package linear + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" +) + +func TestStore_IssueWatch_RepositoryBindingRoundTrip(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + + w := newTestIssueWatch("ws-1") + w.RepositoryID = "repo-123" + w.BaseBranch = "develop" + if err := store.CreateIssueWatch(ctx, w); err != nil { + t.Fatalf("create: %v", err) + } + + got, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.RepositoryID != "repo-123" || got.BaseBranch != "develop" { + t.Fatalf("repository binding lost on round-trip: repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } + + // Update clears the binding (unbind path). + got.RepositoryID = "" + got.BaseBranch = "" + if err := store.UpdateIssueWatch(ctx, got); err != nil { + t.Fatalf("update: %v", err) + } + after, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get after update: %v", err) + } + if after.RepositoryID != "" || after.BaseBranch != "" { + t.Fatalf("expected binding cleared, got repo=%q branch=%q", after.RepositoryID, after.BaseBranch) + } +} + +// TestStore_IssueWatch_DefaultsUnbound pins the backward-compat invariant: a +// watch created without a repository binding reads back as unbound (empty), +// preserving the repo-less behaviour. +func TestStore_IssueWatch_DefaultsUnbound(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + + w := newTestIssueWatch("ws-1") + if err := store.CreateIssueWatch(ctx, w); err != nil { + t.Fatalf("create: %v", err) + } + got, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.RepositoryID != "" || got.BaseBranch != "" { + t.Fatalf("unbound watch should have empty binding, got repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } +} + +// preRepoIssueWatchSchema is the linear_issue_watches DDL from before the +// repository-binding columns were introduced (but after the max_inflight / +// last_error migrations). Used to exercise addIssueWatchRepositoryColumns on an +// older database. +const preRepoIssueWatchSchema = ` + CREATE TABLE linear_issue_watches ( + id TEXT PRIMARY KEY, + workspace_id TEXT NOT NULL, + workflow_id TEXT NOT NULL, + workflow_step_id TEXT NOT NULL, + filter_json TEXT NOT NULL DEFAULT '{}', + agent_profile_id TEXT NOT NULL DEFAULT '', + executor_profile_id TEXT NOT NULL DEFAULT '', + prompt TEXT NOT NULL DEFAULT '', + enabled BOOLEAN NOT NULL DEFAULT 1, + poll_interval_seconds INTEGER NOT NULL DEFAULT 300, + max_inflight_tasks INTEGER DEFAULT 5, + last_polled_at DATETIME, + last_error TEXT NOT NULL DEFAULT '', + last_error_at DATETIME, + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + );` + +func TestStore_IssueWatch_AddRepositoryColumns_Migration(t *testing.T) { + ctx := context.Background() + raw, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open: %v", err) + } + raw.SetMaxOpenConns(1) + t.Cleanup(func() { _ = raw.Close() }) + + // Seed an old-schema table with one existing row. + if _, err := raw.Exec(preRepoIssueWatchSchema); err != nil { + t.Fatalf("legacy schema: %v", err) + } + now := time.Now().UTC() + id := uuid.New().String() + if _, err := raw.Exec(`INSERT INTO linear_issue_watches + (id, workspace_id, workflow_id, workflow_step_id, filter_json, created_at, updated_at) + VALUES (?, 'ws-1', 'wf-1', 'step-1', '{}', ?, ?)`, id, now, now); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + + // NewStore runs initSchema, which must add the new columns idempotently. + store, err := NewStore(raw, raw) + if err != nil { + t.Fatalf("NewStore on legacy table: %v", err) + } + + got, err := store.GetIssueWatch(ctx, id) + if err != nil { + t.Fatalf("get migrated row: %v", err) + } + if got == nil { + t.Fatal("expected migrated row, got nil") + } + if got.RepositoryID != "" || got.BaseBranch != "" { + t.Fatalf("existing row should backfill to unbound, got repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } + + // Idempotent: running initSchema again must not fail on duplicate columns. + if err := store.initSchema(); err != nil { + t.Fatalf("second initSchema (idempotency): %v", err) + } +} diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index 90c64146bd..42d5dccf17 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -299,6 +299,9 @@ type Service struct { // dispatch pre-flight. Set via SetProfileLookup from main; nil-safe so // the legacy code path (and tests without profile wiring) keep working. profileLookup ProfileLookup + // repoChecker answers "does this bound repository still exist?" for the + // dispatch pre-flight. Set via SetRepositoryChecker from main; nil-safe. + repoChecker RepositoryChecker // modelInfoLookup resolves optional model metadata from models.dev. Nil-safe; // ACP context-window events remain authoritative when they include a size. modelInfoMu sync.RWMutex diff --git a/apps/backend/internal/orchestrator/source_jira.go b/apps/backend/internal/orchestrator/source_jira.go index 8acc64942f..5d08968c23 100644 --- a/apps/backend/internal/orchestrator/source_jira.go +++ b/apps/backend/internal/orchestrator/source_jira.go @@ -59,7 +59,7 @@ func (s *JiraWatcherSource) BuildTaskRequest(evt any) (*IssueTaskRequest, error) if !ok || e == nil || e.Issue == nil { return nil, errors.New("jira source: event payload missing or wrong type") } - return &IssueTaskRequest{ + req := &IssueTaskRequest{ WorkspaceID: e.WorkspaceID, WorkflowID: e.WorkflowID, WorkflowStepID: e.WorkflowStepID, @@ -76,7 +76,16 @@ func (s *JiraWatcherSource) BuildTaskRequest(evt any) (*IssueTaskRequest, error) "agent_profile_id": e.AgentProfileID, "executor_profile_id": e.ExecutorProfileID, }, - }, nil + } + // Only a bound watch carries Repositories. An unbound watch leaves the slice + // nil so the launch path falls to the historical blank-scratch behaviour. + if e.RepositoryID != "" { + req.Repositories = []IssueTaskRepository{{ + RepositoryID: e.RepositoryID, + BaseBranch: e.BaseBranch, + }} + } + return req, nil } func (s *JiraWatcherSource) AttachTaskID(ctx context.Context, evt any, taskID string) error { diff --git a/apps/backend/internal/orchestrator/source_jira_test.go b/apps/backend/internal/orchestrator/source_jira_test.go index 0ff4526c52..dd137e0aa3 100644 --- a/apps/backend/internal/orchestrator/source_jira_test.go +++ b/apps/backend/internal/orchestrator/source_jira_test.go @@ -117,6 +117,28 @@ func TestJiraSource_BuildTaskRequest(t *testing.T) { if req.Metadata["agent_profile_id"] != "agent-1" { t.Errorf("missing agent_profile_id metadata") } + // Unbound event (no RepositoryID) must leave Repositories nil so the launch + // path keeps the historical repo-less behaviour. + if req.Repositories != nil { + t.Errorf("unbound event should yield nil Repositories, got %+v", req.Repositories) + } +} + +func TestJiraSource_BuildTaskRequest_RepositoryBinding(t *testing.T) { + src := &JiraWatcherSource{} + evt := sampleJiraEvent() + evt.RepositoryID = "repo-9" + evt.BaseBranch = "develop" + req, err := src.BuildTaskRequest(evt) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(req.Repositories) != 1 { + t.Fatalf("expected one repository, got %d", len(req.Repositories)) + } + if req.Repositories[0].RepositoryID != "repo-9" || req.Repositories[0].BaseBranch != "develop" { + t.Errorf("repository binding wrong: %+v", req.Repositories[0]) + } } func TestJiraSource_BuildTaskRequest_WrongType(t *testing.T) { diff --git a/apps/backend/internal/orchestrator/source_linear.go b/apps/backend/internal/orchestrator/source_linear.go index 8fdf0d902c..63c0411a4b 100644 --- a/apps/backend/internal/orchestrator/source_linear.go +++ b/apps/backend/internal/orchestrator/source_linear.go @@ -62,7 +62,7 @@ func (s *LinearWatcherSource) BuildTaskRequest(evt any) (*IssueTaskRequest, erro if !ok || e == nil || e.Issue == nil { return nil, errors.New("linear source: event payload missing or wrong type") } - return &IssueTaskRequest{ + req := &IssueTaskRequest{ WorkspaceID: e.WorkspaceID, WorkflowID: e.WorkflowID, WorkflowStepID: e.WorkflowStepID, @@ -77,7 +77,16 @@ func (s *LinearWatcherSource) BuildTaskRequest(evt any) (*IssueTaskRequest, erro models.MetaKeyAgentProfileID: e.AgentProfileID, models.MetaKeyExecutorProfileID: e.ExecutorProfileID, }, - }, nil + } + // Only a bound watch carries Repositories. An unbound watch leaves the slice + // nil so the launch path falls to the historical blank-scratch behaviour. + if e.RepositoryID != "" { + req.Repositories = []IssueTaskRepository{{ + RepositoryID: e.RepositoryID, + BaseBranch: e.BaseBranch, + }} + } + return req, nil } func (s *LinearWatcherSource) AttachTaskID(ctx context.Context, evt any, taskID string) error { diff --git a/apps/backend/internal/orchestrator/source_linear_test.go b/apps/backend/internal/orchestrator/source_linear_test.go index 0cb7486ffc..8f874adc1f 100644 --- a/apps/backend/internal/orchestrator/source_linear_test.go +++ b/apps/backend/internal/orchestrator/source_linear_test.go @@ -128,6 +128,28 @@ func TestLinearSource_BuildTaskRequest(t *testing.T) { if req.Metadata["agent_profile_id"] != "agent-1" { t.Errorf("missing agent_profile_id metadata") } + // Unbound event (no RepositoryID) must leave Repositories nil so the launch + // path keeps the historical repo-less behaviour. + if req.Repositories != nil { + t.Errorf("unbound event should yield nil Repositories, got %+v", req.Repositories) + } +} + +func TestLinearSource_BuildTaskRequest_RepositoryBinding(t *testing.T) { + src := &LinearWatcherSource{} + evt := sampleLinearEvent() + evt.RepositoryID = "repo-9" + evt.BaseBranch = "develop" + req, err := src.BuildTaskRequest(evt) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(req.Repositories) != 1 { + t.Fatalf("expected one repository, got %d", len(req.Repositories)) + } + if req.Repositories[0].RepositoryID != "repo-9" || req.Repositories[0].BaseBranch != "develop" { + t.Errorf("repository binding wrong: %+v", req.Repositories[0]) + } } func TestLinearSource_BuildTaskRequest_WrongType(t *testing.T) { diff --git a/apps/backend/internal/orchestrator/source_sentry.go b/apps/backend/internal/orchestrator/source_sentry.go index 46e9231b1f..1aeb020c74 100644 --- a/apps/backend/internal/orchestrator/source_sentry.go +++ b/apps/backend/internal/orchestrator/source_sentry.go @@ -98,7 +98,7 @@ func (s *SentryWatcherSource) BuildTaskRequest(evt any) (*IssueTaskRequest, erro if !ok || e == nil || e.Issue == nil { return nil, errors.New("sentry source: event payload missing or wrong type") } - return &IssueTaskRequest{ + req := &IssueTaskRequest{ WorkspaceID: e.WorkspaceID, WorkflowID: e.WorkflowID, WorkflowStepID: e.WorkflowStepID, @@ -114,7 +114,16 @@ func (s *SentryWatcherSource) BuildTaskRequest(evt any) (*IssueTaskRequest, erro models.MetaKeyAgentProfileID: e.AgentProfileID, models.MetaKeyExecutorProfileID: e.ExecutorProfileID, }, - }, nil + } + // Only a bound watch carries Repositories. An unbound watch leaves the slice + // nil so the launch path falls to the historical blank-scratch behaviour. + if e.RepositoryID != "" { + req.Repositories = []IssueTaskRepository{{ + RepositoryID: e.RepositoryID, + BaseBranch: e.BaseBranch, + }} + } + return req, nil } func (s *SentryWatcherSource) AttachTaskID(ctx context.Context, evt any, taskID string) error { diff --git a/apps/backend/internal/orchestrator/source_sentry_test.go b/apps/backend/internal/orchestrator/source_sentry_test.go index e795bdcb4e..917b9f6c82 100644 --- a/apps/backend/internal/orchestrator/source_sentry_test.go +++ b/apps/backend/internal/orchestrator/source_sentry_test.go @@ -145,6 +145,28 @@ func TestSentrySource_BuildTaskRequest(t *testing.T) { if req.Metadata["executor_profile_id"] != "exec-1" { t.Errorf("missing executor_profile_id metadata") } + // Unbound event (no RepositoryID) must leave Repositories nil so the launch + // path keeps the historical repo-less behaviour. + if req.Repositories != nil { + t.Errorf("unbound event should yield nil Repositories, got %+v", req.Repositories) + } +} + +func TestSentrySource_BuildTaskRequest_RepositoryBinding(t *testing.T) { + src := &SentryWatcherSource{} + evt := sampleSentryEvent() + evt.RepositoryID = "repo-9" + evt.BaseBranch = "develop" + req, err := src.BuildTaskRequest(evt) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(req.Repositories) != 1 { + t.Fatalf("expected one repository, got %d", len(req.Repositories)) + } + if req.Repositories[0].RepositoryID != "repo-9" || req.Repositories[0].BaseBranch != "develop" { + t.Errorf("repository binding wrong: %+v", req.Repositories[0]) + } } func TestSentrySource_BuildTaskRequest_WrongType(t *testing.T) { diff --git a/apps/backend/internal/orchestrator/watcher_dispatch.go b/apps/backend/internal/orchestrator/watcher_dispatch.go index 464a79cc59..7d51a5ba4f 100644 --- a/apps/backend/internal/orchestrator/watcher_dispatch.go +++ b/apps/backend/internal/orchestrator/watcher_dispatch.go @@ -38,7 +38,38 @@ type WatcherDispatchCoordinator struct { // row. nil means "skip the check" — production wires this in; tests can // leave it unset. profileLookup ProfileLookup - logger *logger.Logger + // repoChecker pre-flight checks the watcher's bound repository (when the + // built request carries one). A repository soft-deleted after the watch was + // bound would otherwise let CreateTask insert the task row before repository + // association fails — leaving an orphan row and a reservation that a later + // poll repeats. When the repo is gone the coordinator self-heals the watch + // and skips. nil means "skip the check". + repoChecker RepositoryChecker + logger *logger.Logger +} + +// RepositoryChecker answers "does this repository still exist in the workspace?" +// — used by the watcher dispatch self-heal flow to detect a binding whose +// repository was soft-deleted after the watch was configured. Membership-based +// (a workspace listing) so a definitive "absent" is distinguishable from a +// transient error: (false, nil) = gone → self-heal; a non-nil err = "couldn't +// tell" → fail open (the existing pipeline runs). +type RepositoryChecker interface { + RepositoryExists(ctx context.Context, workspaceID, repositoryID string) (bool, error) +} + +// SetRepositoryChecker wires the repository pre-flight check. nil-ok; the +// check becomes a no-op until a real checker is provided. +func (c *WatcherDispatchCoordinator) SetRepositoryChecker(r RepositoryChecker) { + c.mu.Lock() + defer c.mu.Unlock() + c.repoChecker = r +} + +func (c *WatcherDispatchCoordinator) getRepositoryChecker() RepositoryChecker { + c.mu.RLock() + defer c.mu.RUnlock() + return c.repoChecker } // ProfileLookup answers "is this agent profile still live, and what was its @@ -205,6 +236,50 @@ func (c *WatcherDispatchCoordinator) preflightDeletedProfile(ctx context.Context return true } +// preflightDeletedRepository checks the bound repository of an already-built +// request. When the repo was soft-deleted after binding, it self-heals the +// watch (so a later poll doesn't repeat) and returns true so the caller skips +// task creation — avoiding an orphan task row from CreateTask's non-atomic +// task-then-repository insert. A lookup error fails open (returns false). +func (c *WatcherDispatchCoordinator) preflightDeletedRepository(ctx context.Context, src WatcherSource, evt any, req *IssueTaskRequest) bool { + checker := c.getRepositoryChecker() + if checker == nil || req == nil { + return false + } + for _, r := range req.Repositories { + if r.RepositoryID == "" { + continue + } + exists, err := checker.RepositoryExists(ctx, req.WorkspaceID, r.RepositoryID) + if err != nil { + if c.logger != nil { + c.logger.Warn("watcher dispatch: repository check failed, falling through", + zap.String("source", src.Name()), + zap.String("repository_id", r.RepositoryID), + zap.Error(err)) + } + return false + } + if exists { + continue + } + cause := "bound repository " + r.RepositoryID + " was removed" + if c.logger != nil { + c.logger.Warn("watcher dispatch: bound repository removed, self-healing", + zap.String("source", src.Name()), + zap.String("repository_id", r.RepositoryID)) + } + if err := src.SelfHeal(ctx, evt, cause); err != nil && c.logger != nil { + c.logger.Error("watcher dispatch: self-heal failed", + zap.String("source", src.Name()), + zap.String("repository_id", r.RepositoryID), + zap.Error(err)) + } + return true + } + return false +} + // formatDeletedProfileCause renders the human-readable string stamped onto // the watcher's last_error column. Centralised so every integration uses // the same phrasing. @@ -265,6 +340,14 @@ func (c *WatcherDispatchCoordinator) Dispatch(ctx context.Context, src WatcherSo return } + // A bound repository deleted after the watch was configured would let + // CreateTask insert a task row before repository association fails. Self-heal + // and release the reservation instead of creating an orphan task. + if c.preflightDeletedRepository(ctx, src, evt, req) { + src.Release(ctx, evt) + return + } + task, err := c.getTaskCreator().CreateIssueTask(ctx, req) if err != nil { c.logger.Error("watcher dispatch: create issue task failed", diff --git a/apps/backend/internal/orchestrator/watcher_dispatch_repo_test.go b/apps/backend/internal/orchestrator/watcher_dispatch_repo_test.go new file mode 100644 index 0000000000..ae2a00877f --- /dev/null +++ b/apps/backend/internal/orchestrator/watcher_dispatch_repo_test.go @@ -0,0 +1,115 @@ +package orchestrator + +import ( + "context" + "errors" + "strings" + "testing" +) + +// fakeRepositoryChecker answers a fixed verdict for any repository. +type fakeRepositoryChecker struct { + exists bool + err error + calls int +} + +func (f *fakeRepositoryChecker) RepositoryExists(_ context.Context, _, _ string) (bool, error) { + f.calls++ + return f.exists, f.err +} + +// TestWatcherDispatchCoordinator_SelfHealsOnDeletedRepository: when the bound +// repository was soft-deleted after the watch was configured, the coordinator +// must NOT create a task (which would leave an orphan row, since CreateTask +// inserts the task before repository association), must release the reservation, +// and must self-heal the watch so a later poll doesn't repeat. +func TestWatcherDispatchCoordinator_SelfHealsOnDeletedRepository(t *testing.T) { + src := &stubWatcherSource{ + name: "stub", + workspaceID: "ws-1", + repositories: []IssueTaskRepository{{RepositoryID: "repo-gone", BaseBranch: "main"}}, + } + checker := &fakeRepositoryChecker{exists: false} + creator := &countingIssueTaskCreator{taskID: "task-1"} + + c := &WatcherDispatchCoordinator{ + repoChecker: checker, + shouldAutoStart: func(_ context.Context, _ string) bool { return false }, + logger: newTestLogger(), + } + c.SetTaskCreator(creator) + + c.Dispatch(context.Background(), src, struct{}{}) + + if checker.calls != 1 { + t.Errorf("expected 1 repository check, got %d", checker.calls) + } + if creator.calls != 0 { + t.Errorf("CreateIssueTask must not run when bound repo is deleted, got %d", creator.calls) + } + if src.selfHealCalls != 1 { + t.Fatalf("expected 1 SelfHeal call, got %d", src.selfHealCalls) + } + if !strings.Contains(src.selfHealCause, "repo-gone") { + t.Errorf("SelfHeal cause should name the removed repository, got %q", src.selfHealCause) + } + if src.releaseCalls != 1 { + t.Errorf("reservation must be released on self-heal, got %d Release calls", src.releaseCalls) + } +} + +// A repository-check error must fail open (run the pipeline), not self-heal — +// a transient DB hiccup shouldn't disable a watch. +func TestWatcherDispatchCoordinator_RepoCheckErrorFailsOpen(t *testing.T) { + src := &stubWatcherSource{ + name: "stub", + workspaceID: "ws-1", + repositories: []IssueTaskRepository{{RepositoryID: "repo-1", BaseBranch: "main"}}, + } + checker := &fakeRepositoryChecker{err: errors.New("db unavailable")} + creator := &countingIssueTaskCreator{taskID: "task-1"} + + c := &WatcherDispatchCoordinator{ + repoChecker: checker, + shouldAutoStart: func(_ context.Context, _ string) bool { return false }, + logger: newTestLogger(), + } + c.SetTaskCreator(creator) + + c.Dispatch(context.Background(), src, struct{}{}) + + if src.selfHealCalls != 0 { + t.Errorf("SelfHeal must NOT run on a repo-check error, got %d", src.selfHealCalls) + } + if creator.calls != 1 { + t.Errorf("pipeline must fall through on repo-check error, got %d CreateIssueTask calls", creator.calls) + } +} + +// A live bound repository passes the pre-flight and the task is created. +func TestWatcherDispatchCoordinator_LiveRepositoryPassesPreflight(t *testing.T) { + src := &stubWatcherSource{ + name: "stub", + workspaceID: "ws-1", + repositories: []IssueTaskRepository{{RepositoryID: "repo-1", BaseBranch: "main"}}, + } + checker := &fakeRepositoryChecker{exists: true} + creator := &countingIssueTaskCreator{taskID: "task-1"} + + c := &WatcherDispatchCoordinator{ + repoChecker: checker, + shouldAutoStart: func(_ context.Context, _ string) bool { return false }, + logger: newTestLogger(), + } + c.SetTaskCreator(creator) + + c.Dispatch(context.Background(), src, struct{}{}) + + if src.selfHealCalls != 0 { + t.Errorf("SelfHeal must NOT run for a live repo, got %d", src.selfHealCalls) + } + if creator.calls != 1 { + t.Errorf("expected 1 CreateIssueTask call on the happy path, got %d", creator.calls) + } +} diff --git a/apps/backend/internal/orchestrator/watcher_dispatch_selfheal_test.go b/apps/backend/internal/orchestrator/watcher_dispatch_selfheal_test.go index b1671e6857..3e0d78613e 100644 --- a/apps/backend/internal/orchestrator/watcher_dispatch_selfheal_test.go +++ b/apps/backend/internal/orchestrator/watcher_dispatch_selfheal_test.go @@ -39,6 +39,11 @@ type stubWatcherSource struct { selfHealCalls int selfHealCause string buildCalls int + releaseCalls int + // repositories, when set, is returned in the built IssueTaskRequest so the + // repository pre-flight can be exercised. + repositories []IssueTaskRepository + workspaceID string } func (s *stubWatcherSource) Name() string { return s.name } @@ -50,11 +55,11 @@ func (s *stubWatcherSource) Reserve(_ context.Context, _ any) (bool, error) { return true, nil } -func (s *stubWatcherSource) Release(_ context.Context, _ any) {} +func (s *stubWatcherSource) Release(_ context.Context, _ any) { s.releaseCalls++ } func (s *stubWatcherSource) BuildTaskRequest(_ any) (*IssueTaskRequest, error) { s.buildCalls++ - return &IssueTaskRequest{}, nil + return &IssueTaskRequest{WorkspaceID: s.workspaceID, Repositories: s.repositories}, nil } func (s *stubWatcherSource) AttachTaskID(_ context.Context, _ any, _ string) error { return nil } diff --git a/apps/backend/internal/orchestrator/watcher_dispatch_wiring.go b/apps/backend/internal/orchestrator/watcher_dispatch_wiring.go index 79ccdfa005..3b1e52167a 100644 --- a/apps/backend/internal/orchestrator/watcher_dispatch_wiring.go +++ b/apps/backend/internal/orchestrator/watcher_dispatch_wiring.go @@ -47,6 +47,23 @@ func (s *Service) initWatcherCoordinatorLocked() { if s.profileLookup != nil { s.watcherCoordinator.SetProfileLookup(s.profileLookup) } + if s.repoChecker != nil { + s.watcherCoordinator.SetRepositoryChecker(s.repoChecker) + } +} + +// SetRepositoryChecker wires the deleted-repository pre-flight into the +// coordinator (Linear/Jira/Sentry dispatch). Mirrors SetProfileLookup: safe to +// call before or after SetIssueTaskCreator; the coordinator picks it up on its +// next initWatcherCoordinatorLocked pass. +func (s *Service) SetRepositoryChecker(r RepositoryChecker) { + s.mu.Lock() + s.repoChecker = r + coord := s.watcherCoordinator + s.mu.Unlock() + if coord != nil { + coord.SetRepositoryChecker(r) + } } // SetProfileLookup wires the soft-deleted-profile pre-flight check into both diff --git a/apps/backend/internal/sentry/models.go b/apps/backend/internal/sentry/models.go index 6ceebfa435..43e3ba8377 100644 --- a/apps/backend/internal/sentry/models.go +++ b/apps/backend/internal/sentry/models.go @@ -131,10 +131,20 @@ const ( // whenever a new matching issue appears. Mirrors the Linear/Jira shape so the // orchestrator's WatcherSource pipeline applies uniformly. type IssueWatch struct { - ID string `json:"id" db:"id"` - WorkspaceID string `json:"workspaceId" db:"workspace_id"` - WorkflowID string `json:"workflowId" db:"workflow_id"` - WorkflowStepID string `json:"workflowStepId" db:"workflow_step_id"` + ID string `json:"id" db:"id"` + WorkspaceID string `json:"workspaceId" db:"workspace_id"` + WorkflowID string `json:"workflowId" db:"workflow_id"` + WorkflowStepID string `json:"workflowStepId" db:"workflow_step_id"` + // RepositoryID optionally binds watcher-created tasks to a repository so the + // agent launches in an isolated worktree of that repo instead of a blank + // scratch checkout. Empty = unbound, which preserves the historical + // repo-less behaviour. When set, the resulting task carries a single + // (repository_id, base_branch) pair. + RepositoryID string `json:"repositoryId" db:"repository_id"` + // BaseBranch is the branch the per-task worktree is cut from. Empty defaults + // to the repository's default branch (resolved at create/update time). + // Meaningful only when RepositoryID is set. + BaseBranch string `json:"baseBranch" db:"base_branch"` Filter SearchFilter `json:"filter"` AgentProfileID string `json:"agentProfileId" db:"agent_profile_id"` ExecutorProfileID string `json:"executorProfileId" db:"executor_profile_id"` @@ -171,10 +181,15 @@ type IssueWatchTask struct { // issue matching a watch that has no existing dedup row. The orchestrator // consumes this to create (and optionally auto-start) a Kandev task. type NewSentryIssueEvent struct { - IssueWatchID string `json:"issueWatchId"` - WorkspaceID string `json:"workspaceId"` - WorkflowID string `json:"workflowId"` - WorkflowStepID string `json:"workflowStepId"` + IssueWatchID string `json:"issueWatchId"` + WorkspaceID string `json:"workspaceId"` + WorkflowID string `json:"workflowId"` + WorkflowStepID string `json:"workflowStepId"` + // RepositoryID / BaseBranch carry the watch's optional repository binding so + // the orchestrator source can populate IssueTaskRequest.Repositories without + // reloading the watch row. Empty RepositoryID = unbound (repo-less task). + RepositoryID string `json:"repositoryId,omitempty"` + BaseBranch string `json:"baseBranch,omitempty"` AgentProfileID string `json:"agentProfileId"` ExecutorProfileID string `json:"executorProfileId"` Prompt string `json:"prompt"` @@ -190,6 +205,8 @@ type CreateIssueWatchRequest struct { WorkspaceID string `json:"workspaceId"` WorkflowID string `json:"workflowId"` WorkflowStepID string `json:"workflowStepId"` + RepositoryID string `json:"repositoryId"` + BaseBranch string `json:"baseBranch"` Filter SearchFilter `json:"filter"` AgentProfileID string `json:"agentProfileId"` ExecutorProfileID string `json:"executorProfileId"` @@ -204,6 +221,8 @@ type CreateIssueWatchRequest struct { type UpdateIssueWatchRequest struct { WorkflowID *string `json:"workflowId,omitempty"` WorkflowStepID *string `json:"workflowStepId,omitempty"` + RepositoryID *string `json:"repositoryId,omitempty"` + BaseBranch *string `json:"baseBranch,omitempty"` Filter *SearchFilter `json:"filter,omitempty"` AgentProfileID *string `json:"agentProfileId,omitempty"` ExecutorProfileID *string `json:"executorProfileId,omitempty"` diff --git a/apps/backend/internal/sentry/service.go b/apps/backend/internal/sentry/service.go index 412ed53e92..80a666b71d 100644 --- a/apps/backend/internal/sentry/service.go +++ b/apps/backend/internal/sentry/service.go @@ -24,6 +24,15 @@ type SecretStore interface { Exists(ctx context.Context, id string) (bool, error) } +// RepositoryLookup is the subset of the task service used to validate a watch's +// optional repository binding (workspace ownership + default-branch fill). +// Wired post-construction via SetRepositoryLookup to avoid an import cycle with +// the task service. ok is false when the repository does not exist or has been +// soft-deleted. +type RepositoryLookup interface { + GetRepository(ctx context.Context, id string) (workspaceID, defaultBranch string, ok bool) +} + // Service orchestrates Sentry config storage, the cached client, and the // browse operations used by the HTTP handlers. type Service struct { @@ -49,6 +58,10 @@ type Service struct { // Wired post-construction via SetTaskDeleter to avoid an import cycle // with the task service. taskDeleter watchreset.TaskDeleter + // repoLookup validates an optional repository binding on create/update. + // Wired post-construction via SetRepositoryLookup. When nil (e.g. unit + // tests), the binding is accepted as-is and default-branch fill is skipped. + repoLookup RepositoryLookup } // SetTaskDeleter wires the cascade-delete dependency used by ResetIssueWatch. @@ -60,6 +73,21 @@ func (s *Service) SetTaskDeleter(td watchreset.TaskDeleter) { s.taskDeleter = td } +// SetRepositoryLookup wires the repository validator used by CreateIssueWatch / +// UpdateIssueWatch. Optional — when unset, a repository binding is persisted +// as-is without workspace/default-branch resolution. +func (s *Service) SetRepositoryLookup(rl RepositoryLookup) { + s.mu.Lock() + defer s.mu.Unlock() + s.repoLookup = rl +} + +func (s *Service) getRepositoryLookup() RepositoryLookup { + s.mu.Lock() + defer s.mu.Unlock() + return s.repoLookup +} + // MockClient returns the shared mock client when the service was built in mock // mode, or nil for production builds. func (s *Service) MockClient() *MockClient { diff --git a/apps/backend/internal/sentry/service_issue_watch.go b/apps/backend/internal/sentry/service_issue_watch.go index 43e3380c5f..1940631fb2 100644 --- a/apps/backend/internal/sentry/service_issue_watch.go +++ b/apps/backend/internal/sentry/service_issue_watch.go @@ -9,6 +9,7 @@ import ( "go.uber.org/zap" + "github.com/kandev/kandev/internal/common/securityutil" "github.com/kandev/kandev/internal/events" "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/watchreset" @@ -31,10 +32,16 @@ func (s *Service) CreateIssueWatch(ctx context.Context, req *CreateIssueWatchReq if err := validateIssueWatchCreate(req); err != nil { return nil, err } + repositoryID, baseBranch, err := s.resolveRepositoryBinding(ctx, req.WorkspaceID, req.RepositoryID, req.BaseBranch) + if err != nil { + return nil, err + } w := &IssueWatch{ WorkspaceID: req.WorkspaceID, WorkflowID: req.WorkflowID, WorkflowStepID: req.WorkflowStepID, + RepositoryID: repositoryID, + BaseBranch: baseBranch, Filter: normalizeFilter(req.Filter), AgentProfileID: req.AgentProfileID, ExecutorProfileID: req.ExecutorProfileID, @@ -81,6 +88,7 @@ func (s *Service) UpdateIssueWatch(ctx context.Context, id string, req *UpdateIs if err != nil { return nil, err } + prevRepositoryID, prevBaseBranch := w.RepositoryID, w.BaseBranch applyIssueWatchPatch(w, req) if err := validateFilter(w.Filter); err != nil { return nil, err @@ -102,6 +110,18 @@ func (s *Service) UpdateIssueWatch(ctx context.Context, id string, req *UpdateIs if err := validatePollInterval(w.PollIntervalSeconds); err != nil { return nil, err } + // Only validate/resolve the binding when its value actually changed. The + // dialog re-sends repositoryId/baseBranch on every PATCH, and an unchanged + // binding whose repo was since soft-deleted must not block edits to other + // fields (prompt, filter, …). + if w.RepositoryID != prevRepositoryID || w.BaseBranch != prevBaseBranch { + repositoryID, baseBranch, err := s.resolveRepositoryBinding(ctx, w.WorkspaceID, w.RepositoryID, w.BaseBranch) + if err != nil { + return nil, err + } + w.RepositoryID = repositoryID + w.BaseBranch = baseBranch + } if err := s.store.UpdateIssueWatch(ctx, w); err != nil { return nil, err } @@ -237,6 +257,8 @@ func (s *Service) publishNewSentryIssueEvent(ctx context.Context, w *IssueWatch, WorkspaceID: w.WorkspaceID, WorkflowID: w.WorkflowID, WorkflowStepID: w.WorkflowStepID, + RepositoryID: w.RepositoryID, + BaseBranch: w.BaseBranch, AgentProfileID: w.AgentProfileID, ExecutorProfileID: w.ExecutorProfileID, Prompt: w.Prompt, @@ -249,6 +271,41 @@ func (s *Service) publishNewSentryIssueEvent(ctx context.Context, w *IssueWatch, } } +// resolveRepositoryBinding validates the watch's optional repository binding +// against its workspace and fills an empty base branch with the repository's +// default branch. An empty repositoryID clears the binding (and forces an empty +// base branch), preserving the historical repo-less behaviour. When no +// RepositoryLookup is wired (unit tests, early boot), the binding is accepted +// as-is and the default-branch fill is skipped. +func (s *Service) resolveRepositoryBinding(ctx context.Context, workspaceID, repositoryID, baseBranch string) (string, string, error) { + repositoryID = strings.TrimSpace(repositoryID) + baseBranch = strings.TrimSpace(baseBranch) + if repositoryID == "" { + return "", "", nil + } + // Reject a non-empty base branch that isn't a safe git ref before it can be + // persisted and copied into watcher-created tasks (then fail at worktree + // launch). Empty defers to the repo's default branch below. + if baseBranch != "" && !securityutil.IsValidBaseBranchRef(baseBranch) { + return "", "", fmt.Errorf("%w: base branch %q is not a valid git ref", ErrInvalidConfig, baseBranch) + } + rl := s.getRepositoryLookup() + if rl == nil { + return repositoryID, baseBranch, nil + } + repoWorkspace, defaultBranch, ok := rl.GetRepository(ctx, repositoryID) + if !ok { + return "", "", fmt.Errorf("%w: repository %q not found", ErrInvalidConfig, repositoryID) + } + if repoWorkspace != workspaceID { + return "", "", fmt.Errorf("%w: repository %q does not belong to this workspace", ErrInvalidConfig, repositoryID) + } + if baseBranch == "" { + baseBranch = defaultBranch + } + return repositoryID, baseBranch, nil +} + func validateIssueWatchCreate(req *CreateIssueWatchRequest) error { if req.WorkspaceID == "" { return fmt.Errorf("%w: workspaceId required", ErrInvalidConfig) @@ -353,6 +410,20 @@ func applyIssueWatchPatch(w *IssueWatch, req *UpdateIssueWatchRequest) { if req.WorkflowStepID != nil { w.WorkflowStepID = *req.WorkflowStepID } + // RepositoryID / BaseBranch are applied here; UpdateIssueWatch then runs them + // through resolveRepositoryBinding (workspace check + default-branch fill, or + // clear when empty). An empty RepositoryID unbinds the watch. Switching to a + // different repository without an explicit base branch resets the branch so + // the new repo's default is used instead of carrying the old repo's branch. + if req.RepositoryID != nil { + if *req.RepositoryID != w.RepositoryID && req.BaseBranch == nil { + w.BaseBranch = "" + } + w.RepositoryID = *req.RepositoryID + } + if req.BaseBranch != nil { + w.BaseBranch = *req.BaseBranch + } if req.Filter != nil { w.Filter = normalizeFilter(*req.Filter) } diff --git a/apps/backend/internal/sentry/service_issue_watch_repository_test.go b/apps/backend/internal/sentry/service_issue_watch_repository_test.go new file mode 100644 index 0000000000..e600d36653 --- /dev/null +++ b/apps/backend/internal/sentry/service_issue_watch_repository_test.go @@ -0,0 +1,194 @@ +package sentry + +import ( + "context" + "errors" + "testing" +) + +// fakeRepoLookup is a static RepositoryLookup for repository-binding tests. +type fakeRepoLookup struct { + workspaceID string + defaultBranch string + ok bool +} + +func (f fakeRepoLookup) GetRepository(_ context.Context, _ string) (string, string, bool) { + return f.workspaceID, f.defaultBranch, f.ok +} + +func TestService_CreateIssueWatch_RepositoryBinding(t *testing.T) { + ctx := context.Background() + baseReq := func() *CreateIssueWatchRequest { + return &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf", + WorkflowStepID: "step", + Filter: SearchFilter{OrgSlug: "acme", ProjectSlug: "frontend"}, + } + } + + t.Run("default branch filled from repo", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "repo-1" || w.BaseBranch != "main" { + t.Fatalf("expected repo-1@main, got repo=%q branch=%q", w.RepositoryID, w.BaseBranch) + } + }) + + t.Run("explicit branch preserved", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + req.BaseBranch = "release/v2" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.BaseBranch != "release/v2" { + t.Fatalf("explicit branch overwritten: %q", w.BaseBranch) + } + }) + + t.Run("cross-workspace repo rejected", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-other", defaultBranch: "main", ok: true}) + req := baseReq() + req.RepositoryID = "repo-1" + if _, err := f.svc.CreateIssueWatch(ctx, req); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for cross-workspace repo, got %v", err) + } + }) + + t.Run("missing repo rejected", func(t *testing.T) { + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{ok: false}) + req := baseReq() + req.RepositoryID = "ghost" + if _, err := f.svc.CreateIssueWatch(ctx, req); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for missing repo, got %v", err) + } + }) + + t.Run("unbound skips lookup and clears branch", func(t *testing.T) { + f := newSvcFixture(t) // no RepositoryLookup wired + req := baseReq() + req.BaseBranch = "ignored-without-repo" + w, err := f.svc.CreateIssueWatch(ctx, req) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "" || w.BaseBranch != "" { + t.Fatalf("unbound watch must stay empty, got repo=%q branch=%q", w.RepositoryID, w.BaseBranch) + } + }) +} + +func TestService_UpdateIssueWatch_RepositoryBinding(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + + w, err := f.svc.CreateIssueWatch(ctx, &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf", + WorkflowStepID: "step", + Filter: SearchFilter{OrgSlug: "acme", ProjectSlug: "frontend"}, + }) + if err != nil { + t.Fatalf("create: %v", err) + } + if w.RepositoryID != "" { + t.Fatalf("expected unbound on create, got %q", w.RepositoryID) + } + + repoID := "repo-1" + updated, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repoID}) + if err != nil { + t.Fatalf("update bind: %v", err) + } + if updated.RepositoryID != "repo-1" || updated.BaseBranch != "main" { + t.Fatalf("expected repo-1@main after update, got repo=%q branch=%q", updated.RepositoryID, updated.BaseBranch) + } + + empty := "" + cleared, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &empty}) + if err != nil { + t.Fatalf("update unbind: %v", err) + } + if cleared.RepositoryID != "" || cleared.BaseBranch != "" { + t.Fatalf("expected cleared after update, got repo=%q branch=%q", cleared.RepositoryID, cleared.BaseBranch) + } +} + +func TestService_UpdateIssueWatch_RebindAndDeletedRepo(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + + w, err := f.svc.CreateIssueWatch(ctx, &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf", WorkflowStepID: "step", + Filter: SearchFilter{OrgSlug: "acme", ProjectSlug: "frontend"}, + }) + if err != nil { + t.Fatalf("create: %v", err) + } + repo1 := "repo-1" + if _, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repo1}); err != nil { + t.Fatalf("bind: %v", err) + } + + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "develop", ok: true}) + repo2 := "repo-2" + rebound, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{RepositoryID: &repo2}) + if err != nil { + t.Fatalf("rebind: %v", err) + } + if rebound.RepositoryID != "repo-2" || rebound.BaseBranch != "develop" { + t.Fatalf("rebind should reset branch to new default, got repo=%q branch=%q", rebound.RepositoryID, rebound.BaseBranch) + } + + f.svc.SetRepositoryLookup(fakeRepoLookup{ok: false}) + prompt := "updated prompt" + edited, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{Prompt: &prompt}) + if err != nil { + t.Fatalf("unrelated edit blocked by deleted bound repo: %v", err) + } + if edited.Prompt != "updated prompt" || edited.RepositoryID != "repo-2" || edited.BaseBranch != "develop" { + t.Fatalf("expected prompt updated + binding preserved, got prompt=%q repo=%q branch=%q", edited.Prompt, edited.RepositoryID, edited.BaseBranch) + } +} + +func TestService_IssueWatch_RejectsInvalidBaseBranch(t *testing.T) { + ctx := context.Background() + f := newSvcFixture(t) + f.svc.SetRepositoryLookup(fakeRepoLookup{workspaceID: "ws-1", defaultBranch: "main", ok: true}) + base := func() *CreateIssueWatchRequest { + return &CreateIssueWatchRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf", WorkflowStepID: "step", + Filter: SearchFilter{OrgSlug: "acme", ProjectSlug: "frontend"}, RepositoryID: "repo-1", + } + } + + bad := base() + bad.BaseBranch = "bad..ref" + if _, err := f.svc.CreateIssueWatch(ctx, bad); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for invalid base branch on create, got %v", err) + } + + w, err := f.svc.CreateIssueWatch(ctx, base()) + if err != nil { + t.Fatalf("create: %v", err) + } + badRef := "bad..ref" + if _, err := f.svc.UpdateIssueWatch(ctx, w.ID, &UpdateIssueWatchRequest{BaseBranch: &badRef}); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("expected ErrInvalidConfig for invalid base branch on update, got %v", err) + } +} diff --git a/apps/backend/internal/sentry/store.go b/apps/backend/internal/sentry/store.go index ef6c8639c6..75f9137dfa 100644 --- a/apps/backend/internal/sentry/store.go +++ b/apps/backend/internal/sentry/store.go @@ -43,6 +43,11 @@ const createTablesSQL = ` workspace_id TEXT NOT NULL, workflow_id TEXT NOT NULL, workflow_step_id TEXT NOT NULL, + -- Optional repository binding. Empty = unbound (repo-less task, the + -- historical behaviour). When set, watcher-created tasks launch in an + -- isolated worktree of this repo cut from base_branch. + repository_id TEXT NOT NULL DEFAULT '', + base_branch TEXT NOT NULL DEFAULT '', filter_json TEXT NOT NULL DEFAULT '{}', agent_profile_id TEXT NOT NULL DEFAULT '', executor_profile_id TEXT NOT NULL DEFAULT '', @@ -86,7 +91,37 @@ func (s *Store) initSchema() error { if err := s.addMaxInflightTasksColumn(); err != nil { return err } - return s.addIssueWatchLastErrorColumns() + if err := s.addIssueWatchLastErrorColumns(); err != nil { + return err + } + return s.addIssueWatchRepositoryColumns() +} + +// addIssueWatchRepositoryColumns brings older databases up to the current +// schema by appending repository_id / base_branch to sentry_issue_watches when +// missing. Both backfill to ” (unbound), so existing repo-less watches keep +// their behaviour. Fresh installs hit the column-already-present branch since +// createTablesSQL declares both columns. Idempotent — column lookup before each +// ALTER avoids the "duplicate column name" error. +func (s *Store) addIssueWatchRepositoryColumns() error { + cols, err := s.tableColumns("sentry_issue_watches") + if err != nil { + return err + } + if len(cols) == 0 { + return nil + } + if _, ok := cols["repository_id"]; !ok { + if _, err := s.db.Exec(`ALTER TABLE sentry_issue_watches ADD COLUMN repository_id TEXT NOT NULL DEFAULT ''`); err != nil { + return fmt.Errorf("add repository_id column: %w", err) + } + } + if _, ok := cols["base_branch"]; !ok { + if _, err := s.db.Exec(`ALTER TABLE sentry_issue_watches ADD COLUMN base_branch TEXT NOT NULL DEFAULT ''`); err != nil { + return fmt.Errorf("add base_branch column: %w", err) + } + } + return nil } // addConfigURLColumn brings older databases up to the current schema by adding diff --git a/apps/backend/internal/sentry/store_issue_watch.go b/apps/backend/internal/sentry/store_issue_watch.go index 50928bed5d..7d47f50204 100644 --- a/apps/backend/internal/sentry/store_issue_watch.go +++ b/apps/backend/internal/sentry/store_issue_watch.go @@ -19,6 +19,8 @@ type issueWatchRow struct { WorkspaceID string `db:"workspace_id"` WorkflowID string `db:"workflow_id"` WorkflowStepID string `db:"workflow_step_id"` + RepositoryID string `db:"repository_id"` + BaseBranch string `db:"base_branch"` FilterJSON string `db:"filter_json"` AgentProfileID string `db:"agent_profile_id"` ExecutorProfileID string `db:"executor_profile_id"` @@ -50,6 +52,8 @@ func (r *issueWatchRow) toIssueWatch() (*IssueWatch, error) { WorkspaceID: r.WorkspaceID, WorkflowID: r.WorkflowID, WorkflowStepID: r.WorkflowStepID, + RepositoryID: r.RepositoryID, + BaseBranch: r.BaseBranch, Filter: filter, AgentProfileID: r.AgentProfileID, ExecutorProfileID: r.ExecutorProfileID, @@ -87,13 +91,15 @@ func encodeFilter(f SearchFilter) (string, error) { // SELECTs use issueWatchSelectColumns which wraps the nullable last_error in // COALESCE so older databases (pre-self-heal migration) read back as empty // strings rather than NULL. -const issueWatchInsertColumns = `id, workspace_id, workflow_id, workflow_step_id, filter_json, +const issueWatchInsertColumns = `id, workspace_id, workflow_id, workflow_step_id, + repository_id, base_branch, filter_json, agent_profile_id, executor_profile_id, prompt, enabled, poll_interval_seconds, max_inflight_tasks, last_polled_at, last_error, last_error_at, created_at, updated_at` -const issueWatchSelectColumns = `id, workspace_id, workflow_id, workflow_step_id, filter_json, +const issueWatchSelectColumns = `id, workspace_id, workflow_id, workflow_step_id, + COALESCE(repository_id, '') AS repository_id, COALESCE(base_branch, '') AS base_branch, filter_json, agent_profile_id, executor_profile_id, prompt, enabled, poll_interval_seconds, max_inflight_tasks, last_polled_at, COALESCE(last_error, '') AS last_error, last_error_at, @@ -117,8 +123,9 @@ func (s *Store) CreateIssueWatch(ctx context.Context, w *IssueWatch) error { } _, err = s.db.ExecContext(ctx, ` INSERT INTO sentry_issue_watches (`+issueWatchInsertColumns+`) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, - w.ID, w.WorkspaceID, w.WorkflowID, w.WorkflowStepID, filterJSON, + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + w.ID, w.WorkspaceID, w.WorkflowID, w.WorkflowStepID, + w.RepositoryID, w.BaseBranch, filterJSON, w.AgentProfileID, w.ExecutorProfileID, w.Prompt, w.Enabled, w.PollIntervalSeconds, nullableInt(w.MaxInflightTasks), w.LastPolledAt, w.LastError, w.LastErrorAt, @@ -203,11 +210,13 @@ func (s *Store) UpdateIssueWatch(ctx context.Context, w *IssueWatch) error { return err } _, err = s.db.ExecContext(ctx, ` - UPDATE sentry_issue_watches SET workflow_id = ?, workflow_step_id = ?, filter_json = ?, + UPDATE sentry_issue_watches SET workflow_id = ?, workflow_step_id = ?, + repository_id = ?, base_branch = ?, filter_json = ?, agent_profile_id = ?, executor_profile_id = ?, prompt = ?, enabled = ?, poll_interval_seconds = ?, max_inflight_tasks = ?, updated_at = ? WHERE id = ?`, - w.WorkflowID, w.WorkflowStepID, filterJSON, + w.WorkflowID, w.WorkflowStepID, + w.RepositoryID, w.BaseBranch, filterJSON, w.AgentProfileID, w.ExecutorProfileID, w.Prompt, w.Enabled, w.PollIntervalSeconds, nullableInt(w.MaxInflightTasks), w.UpdatedAt, w.ID) return err diff --git a/apps/backend/internal/sentry/store_issue_watch_repository_test.go b/apps/backend/internal/sentry/store_issue_watch_repository_test.go new file mode 100644 index 0000000000..fb25ea1f76 --- /dev/null +++ b/apps/backend/internal/sentry/store_issue_watch_repository_test.go @@ -0,0 +1,133 @@ +package sentry + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" +) + +func TestStore_IssueWatch_RepositoryBindingRoundTrip(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + + w := newTestIssueWatch("ws-1") + w.RepositoryID = "repo-123" + w.BaseBranch = "develop" + if err := store.CreateIssueWatch(ctx, w); err != nil { + t.Fatalf("create: %v", err) + } + + got, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.RepositoryID != "repo-123" || got.BaseBranch != "develop" { + t.Fatalf("repository binding lost on round-trip: repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } + + // Update clears the binding (unbind path). + got.RepositoryID = "" + got.BaseBranch = "" + if err := store.UpdateIssueWatch(ctx, got); err != nil { + t.Fatalf("update: %v", err) + } + after, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get after update: %v", err) + } + if after.RepositoryID != "" || after.BaseBranch != "" { + t.Fatalf("expected binding cleared, got repo=%q branch=%q", after.RepositoryID, after.BaseBranch) + } +} + +// TestStore_IssueWatch_DefaultsUnbound pins the backward-compat invariant: a +// watch created without a repository binding reads back as unbound (empty), +// preserving the repo-less behaviour. +func TestStore_IssueWatch_DefaultsUnbound(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + + w := newTestIssueWatch("ws-1") + if err := store.CreateIssueWatch(ctx, w); err != nil { + t.Fatalf("create: %v", err) + } + got, err := store.GetIssueWatch(ctx, w.ID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.RepositoryID != "" || got.BaseBranch != "" { + t.Fatalf("unbound watch should have empty binding, got repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } +} + +// preRepoIssueWatchSchema is the sentry_issue_watches DDL from before the +// repository-binding columns were introduced (but after the max_inflight / +// last_error migrations). Used to exercise addIssueWatchRepositoryColumns on an +// older database. +const preRepoIssueWatchSchema = ` + CREATE TABLE sentry_issue_watches ( + id TEXT PRIMARY KEY, + workspace_id TEXT NOT NULL, + workflow_id TEXT NOT NULL, + workflow_step_id TEXT NOT NULL, + filter_json TEXT NOT NULL DEFAULT '{}', + agent_profile_id TEXT NOT NULL DEFAULT '', + executor_profile_id TEXT NOT NULL DEFAULT '', + prompt TEXT NOT NULL DEFAULT '', + enabled BOOLEAN NOT NULL DEFAULT 1, + poll_interval_seconds INTEGER NOT NULL DEFAULT 300, + max_inflight_tasks INTEGER DEFAULT 5, + last_polled_at DATETIME, + last_error TEXT NOT NULL DEFAULT '', + last_error_at DATETIME, + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + );` + +func TestStore_IssueWatch_AddRepositoryColumns_Migration(t *testing.T) { + ctx := context.Background() + raw, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open: %v", err) + } + raw.SetMaxOpenConns(1) + t.Cleanup(func() { _ = raw.Close() }) + + // Seed an old-schema table with one existing row. + if _, err := raw.Exec(preRepoIssueWatchSchema); err != nil { + t.Fatalf("legacy schema: %v", err) + } + now := time.Now().UTC() + id := uuid.New().String() + if _, err := raw.Exec(`INSERT INTO sentry_issue_watches + (id, workspace_id, workflow_id, workflow_step_id, filter_json, created_at, updated_at) + VALUES (?, 'ws-1', 'wf-1', 'step-1', '{}', ?, ?)`, id, now, now); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + + // NewStore runs initSchema, which must add the new columns idempotently. + store, err := NewStore(raw, raw) + if err != nil { + t.Fatalf("NewStore on legacy table: %v", err) + } + + got, err := store.GetIssueWatch(ctx, id) + if err != nil { + t.Fatalf("get migrated row: %v", err) + } + if got == nil { + t.Fatal("expected migrated row, got nil") + } + if got.RepositoryID != "" || got.BaseBranch != "" { + t.Fatalf("existing row should backfill to unbound, got repo=%q branch=%q", got.RepositoryID, got.BaseBranch) + } + + // Idempotent: running initSchema again must not fail on duplicate columns. + if err := store.initSchema(); err != nil { + t.Fatalf("second initSchema (idempotency): %v", err) + } +} diff --git a/apps/web/components/jira/jira-issue-watch-dialog.tsx b/apps/web/components/jira/jira-issue-watch-dialog.tsx index ed419dca31..c885740786 100644 --- a/apps/web/components/jira/jira-issue-watch-dialog.tsx +++ b/apps/web/components/jira/jira-issue-watch-dialog.tsx @@ -32,6 +32,8 @@ import { DEFAULT_JIRA_ISSUE_WATCH_PROMPT, } from "@/components/jira/jira-issue-watch-placeholders"; import { STEP_DEFAULT, STEP_DEFAULT_LABEL, resolveProfileId } from "@/lib/watcher-profile-default"; +import { WatcherRepositoryFields } from "@/components/watcher-repository-fields"; +import { clearWorkspaceScopedForm } from "@/lib/watcher-repository-default"; import type { CreateJiraIssueWatchInput, JiraIssueWatch, @@ -56,6 +58,10 @@ type FormState = { jql: string; workflowId: string; workflowStepId: string; + /** Optional repository binding; "" = unbound (repo-less task). */ + repositoryId: string; + /** Base branch for the worktree; "" = the repository's default branch. */ + baseBranch: string; agentProfileId: string; executorProfileId: string; prompt: string; @@ -90,6 +96,8 @@ function makeEmptyForm(workspaceId: string): FormState { jql: DEFAULT_JQL, workflowId: "", workflowStepId: "", + repositoryId: "", + baseBranch: "", agentProfileId: "", executorProfileId: "", prompt: DEFAULT_JIRA_ISSUE_WATCH_PROMPT, @@ -105,6 +113,8 @@ function formStateFromWatch(w: JiraIssueWatch): FormState { jql: w.jql, workflowId: w.workflowId, workflowStepId: w.workflowStepId, + repositoryId: w.repositoryId ?? "", + baseBranch: w.baseBranch ?? "", agentProfileId: w.agentProfileId, executorProfileId: w.executorProfileId, prompt: w.prompt.trim() ? w.prompt : DEFAULT_JIRA_ISSUE_WATCH_PROMPT, @@ -323,6 +333,15 @@ function AutomationFields({ disabled={!form.workflowId || stepsLoading || steps.length === 0} /> + + setForm((p) => ({ ...p, repositoryId, baseBranch: "" })) + } + onBaseBranchChange={(baseBranch) => setForm((p) => ({ ...p, baseBranch }))} + />
{watch ? "Edit JIRA Watcher" : "Create JIRA Watcher"} - Poll a JQL query and auto-create a Kandev task for each newly-matching ticket. Tickets - are not bound to a repository — the workflow step's defaults decide where the task - runs. + Poll a JQL query and auto-create a Kandev task for each newly-matching ticket. + Optionally bind a repository so each task runs against that codebase, or leave it unset + to run with no repository.
- setForm((p) => ({ ...p, workspaceId: v, workflowId: "", workflowStepId: "" })) - } + onChange={(v) => setForm((p) => clearWorkspaceScopedForm(p, v))} disabled={workspaceLocked} /> setForm((p) => ({ ...p, jql: v }))} /> diff --git a/apps/web/components/linear/linear-issue-watch-dialog.tsx b/apps/web/components/linear/linear-issue-watch-dialog.tsx index 18258fdd8d..49bfcf7c9c 100644 --- a/apps/web/components/linear/linear-issue-watch-dialog.tsx +++ b/apps/web/components/linear/linear-issue-watch-dialog.tsx @@ -3,7 +3,6 @@ import { useState, useEffect, useCallback, useMemo } from "react"; import { Button } from "@kandev/ui/button"; import { Separator } from "@kandev/ui/separator"; -import { Switch } from "@kandev/ui/switch"; import { Label } from "@kandev/ui/label"; import { Input } from "@kandev/ui/input"; import { @@ -29,11 +28,14 @@ import { import { LabelMultiSelect, PriorityMultiSelect, + SettingsFields, StateMultiSelect, useTeamsAndStates, } from "./linear-issue-watch-fields"; import { LINEAR_ISSUE_WATCH_PLACEHOLDERS } from "./linear-issue-watch-placeholders"; import { STEP_DEFAULT, STEP_DEFAULT_LABEL, resolveProfileId } from "@/lib/watcher-profile-default"; +import { WatcherRepositoryFields } from "@/components/watcher-repository-fields"; +import { clearWorkspaceScopedForm } from "@/lib/watcher-repository-default"; import { ASSIGNED_ANY, CREATOR_ANY, @@ -44,7 +46,6 @@ import { formStateFromWatch, isWatchFormReady, makeEmptyForm, - parseMaxInflightTasks, userOptionLabel, } from "./linear-issue-watch-form"; import type { @@ -428,6 +429,15 @@ function AutomationFields({ disabled={!form.workflowId || stepsLoading || steps.length === 0} />
+ + setForm((p) => ({ ...p, repositoryId, baseBranch: "" })) + } + onBaseBranchChange={(baseBranch) => setForm((p) => ({ ...p, baseBranch }))} + />
>; -}) { - const parsed = parseMaxInflightTasks(form.maxInflightTasks); - const invalid = parsed === "invalid"; - return ( -
- -

- Cap on open tasks created by this watcher. Leave blank for no cap. New matches are deferred - to the next poll when the cap is reached. -

- setForm((p) => ({ ...p, maxInflightTasks: e.target.value }))} - min={1} - step={1} - placeholder="(no cap)" - aria-invalid={invalid} - /> - {invalid && ( -

Enter a positive integer or leave blank.

- )} -
- ); -} - -function SettingsFields({ - form, - setForm, -}: { - form: FormState; - setForm: React.Dispatch>; -}) { - return ( - <> -
- -

- How often to re-run the search. Minimum 60s, maximum 3600s. -

- setForm((p) => ({ ...p, pollInterval: Number(e.target.value) }))} - min={60} - max={3600} - /> -
- -
-
- -

Pause or resume polling.

-
- setForm((p) => ({ ...p, enabled: v }))} - className="cursor-pointer" - /> -
- - ); -} - function savingLabel(saving: boolean, isEdit: boolean): string { if (saving) return "Saving…"; return isEdit ? "Update" : "Create"; @@ -583,16 +523,14 @@ export function LinearIssueWatchDialog({ {watch ? "Edit Linear Watcher" : "Create Linear Watcher"} Poll Linear with a structured filter and auto-create a Kandev task for each - newly-matching issue. Issues are not bound to a repository — the workflow step's - defaults decide where the task runs. + newly-matching issue. Optionally bind a repository so each task runs against that + codebase, or leave it unset to run with no repository.
- setForm((p) => ({ ...p, workspaceId: v, workflowId: "", workflowStepId: "" })) - } + onChange={(v) => setForm((p) => clearWorkspaceScopedForm(p, v))} disabled={workspaceLocked} /> {/* Hairlines separate the five conceptual blocks (Destination / diff --git a/apps/web/components/linear/linear-issue-watch-fields.tsx b/apps/web/components/linear/linear-issue-watch-fields.tsx index ecf46bb505..70b16cc097 100644 --- a/apps/web/components/linear/linear-issue-watch-fields.tsx +++ b/apps/web/components/linear/linear-issue-watch-fields.tsx @@ -1,14 +1,22 @@ "use client"; -import { useEffect, useRef, useState } from "react"; +import { type Dispatch, type SetStateAction, useEffect, useRef, useState } from "react"; import { Badge } from "@kandev/ui/badge"; +import { Input } from "@kandev/ui/input"; +import { Label } from "@kandev/ui/label"; +import { Switch } from "@kandev/ui/switch"; import { listLinearLabels, listLinearStates, listLinearTeams, listLinearUsers, } from "@/lib/api/domains/linear-api"; -import { PRIORITY_OPTIONS, type LinearPriority } from "./linear-issue-watch-form"; +import { + PRIORITY_OPTIONS, + parseMaxInflightTasks, + type FormState, + type LinearPriority, +} from "./linear-issue-watch-form"; import type { LinearLabel, LinearTeam, LinearUser, LinearWorkflowState } from "@/lib/types/linear"; // useTeamsAndStates loads the team list once Linear is configured, plus the @@ -219,3 +227,68 @@ export function LabelMultiSelect({
); } + +type FormSetter = Dispatch>; + +// MaxInflightTasksField renders the per-watcher throttle-cap input with inline +// validation. Lives here (rather than the dialog) to keep the dialog file under +// its line ceiling. +export function MaxInflightTasksField({ form, setForm }: { form: FormState; setForm: FormSetter }) { + const parsed = parseMaxInflightTasks(form.maxInflightTasks); + const invalid = parsed === "invalid"; + return ( +
+ +

+ Cap on open tasks created by this watcher. Leave blank for no cap. New matches are deferred + to the next poll when the cap is reached. +

+ setForm((p) => ({ ...p, maxInflightTasks: e.target.value }))} + min={1} + step={1} + placeholder="(no cap)" + aria-invalid={invalid} + /> + {invalid && ( +

Enter a positive integer or leave blank.

+ )} +
+ ); +} + +// SettingsFields renders the poll-interval, throttle-cap, and enabled toggle — +// the trailing "Settings" block of the watcher dialog. +export function SettingsFields({ form, setForm }: { form: FormState; setForm: FormSetter }) { + return ( + <> +
+ +

+ How often to re-run the search. Minimum 60s, maximum 3600s. +

+ setForm((p) => ({ ...p, pollInterval: Number(e.target.value) }))} + min={60} + max={3600} + /> +
+ +
+
+ +

Pause or resume polling.

+
+ setForm((p) => ({ ...p, enabled: v }))} + className="cursor-pointer" + /> +
+ + ); +} diff --git a/apps/web/components/linear/linear-issue-watch-form.ts b/apps/web/components/linear/linear-issue-watch-form.ts index 658958d146..4757c3f3e0 100644 --- a/apps/web/components/linear/linear-issue-watch-form.ts +++ b/apps/web/components/linear/linear-issue-watch-form.ts @@ -29,6 +29,10 @@ export interface FormState { estimateMax: string; workflowId: string; workflowStepId: string; + /** Optional repository binding; "" = unbound (repo-less task). */ + repositoryId: string; + /** Base branch for the worktree; "" = the repository's default branch. */ + baseBranch: string; agentProfileId: string; executorProfileId: string; prompt: string; @@ -57,6 +61,8 @@ export function makeEmptyForm(workspaceId: string): FormState { estimateMax: "", workflowId: "", workflowStepId: "", + repositoryId: "", + baseBranch: "", agentProfileId: "", executorProfileId: "", prompt: DEFAULT_LINEAR_ISSUE_WATCH_PROMPT, @@ -85,6 +91,8 @@ export function formStateFromWatch(w: LinearIssueWatch): FormState { estimateMax: estimateString(f.estimateMax), workflowId: w.workflowId, workflowStepId: w.workflowStepId, + repositoryId: w.repositoryId ?? "", + baseBranch: w.baseBranch ?? "", agentProfileId: w.agentProfileId, executorProfileId: w.executorProfileId, prompt: w.prompt || DEFAULT_LINEAR_ISSUE_WATCH_PROMPT, @@ -147,6 +155,8 @@ export function buildWatchPayload(form: FormState): { filter: LinearSearchFilter; workflowId: string; workflowStepId: string; + repositoryId: string; + baseBranch: string; agentProfileId: string; executorProfileId: string; prompt: string; @@ -160,6 +170,10 @@ export function buildWatchPayload(form: FormState): { filter: buildFilterPayload(form), workflowId: form.workflowId, workflowStepId: form.workflowStepId, + // An empty repositoryId clears the binding; the empty base branch is sent + // verbatim so the backend fills it with the repo's default at save time. + repositoryId: form.repositoryId, + baseBranch: form.repositoryId ? form.baseBranch : "", agentProfileId: form.agentProfileId, executorProfileId: form.executorProfileId, prompt: form.prompt, diff --git a/apps/web/components/sentry/sentry-issue-watch-dialog.tsx b/apps/web/components/sentry/sentry-issue-watch-dialog.tsx index ad719b6a27..19df592d2e 100644 --- a/apps/web/components/sentry/sentry-issue-watch-dialog.tsx +++ b/apps/web/components/sentry/sentry-issue-watch-dialog.tsx @@ -26,6 +26,8 @@ import { computeEditorHeight, } from "@/components/settings/profile-edit/script-editor"; import { listSentryProjects, listSentryOrganizations } from "@/lib/api/domains/sentry-api"; +import { WatcherRepositoryFields } from "@/components/watcher-repository-fields"; +import { clearWorkspaceScopedForm } from "@/lib/watcher-repository-default"; import { SENTRY_ISSUE_WATCH_PLACEHOLDERS } from "./sentry-issue-watch-placeholders"; import { LevelMultiSelect, StatusMultiSelect } from "./sentry-issue-watch-multiselect"; import { MaxInflightTasksField } from "./sentry-issue-watch-throttle-field"; @@ -356,6 +358,15 @@ function AutomationFields({ form, setForm }: { form: FormState; setForm: FormSet disabled={!form.workflowId || stepsLoading || steps.length === 0} />
+ + setForm((p) => ({ ...p, repositoryId, baseBranch: "" })) + } + onBaseBranchChange={(baseBranch) => setForm((p) => ({ ...p, baseBranch }))} + />
{watch ? "Edit Sentry Watcher" : "Create Sentry Watcher"} Poll Sentry with a structured filter and auto-create a Kandev task for each - newly-matching issue. The workflow step's defaults decide where the task runs. + newly-matching issue. Optionally bind a repository so each task runs against that + codebase, or leave it unset to run with no repository.
- setForm((p) => ({ ...p, workspaceId: v, workflowId: "", workflowStepId: "" })) - } + onChange={(v) => setForm((p) => clearWorkspaceScopedForm(p, v))} disabled={workspaceLocked} /> diff --git a/apps/web/components/sentry/sentry-issue-watch-form.ts b/apps/web/components/sentry/sentry-issue-watch-form.ts index 59251c466d..c9022bfe48 100644 --- a/apps/web/components/sentry/sentry-issue-watch-form.ts +++ b/apps/web/components/sentry/sentry-issue-watch-form.ts @@ -29,6 +29,10 @@ export interface FormState { statsPeriod: string; workflowId: string; workflowStepId: string; + /** Optional repository binding; "" = unbound (repo-less task). */ + repositoryId: string; + /** Base branch for the worktree; "" = the repository's default branch. */ + baseBranch: string; agentProfileId: string; executorProfileId: string; prompt: string; @@ -49,6 +53,8 @@ export function makeEmptyForm(workspaceId: string): FormState { statsPeriod: "24h", workflowId: "", workflowStepId: "", + repositoryId: "", + baseBranch: "", agentProfileId: "", executorProfileId: "", prompt: DEFAULT_SENTRY_ISSUE_WATCH_PROMPT, @@ -71,6 +77,8 @@ export function formStateFromWatch(w: SentryIssueWatch): FormState { statsPeriod: f.statsPeriod ?? "", workflowId: w.workflowId, workflowStepId: w.workflowStepId, + repositoryId: w.repositoryId ?? "", + baseBranch: w.baseBranch ?? "", agentProfileId: w.agentProfileId, executorProfileId: w.executorProfileId, prompt: w.prompt || DEFAULT_SENTRY_ISSUE_WATCH_PROMPT, diff --git a/apps/web/components/watcher-repository-fields.tsx b/apps/web/components/watcher-repository-fields.tsx new file mode 100644 index 0000000000..37ff3e723c --- /dev/null +++ b/apps/web/components/watcher-repository-fields.tsx @@ -0,0 +1,114 @@ +"use client"; + +import { Label } from "@kandev/ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; +import { useRepositories } from "@/hooks/domains/workspace/use-repositories"; +import { useBranches } from "@/hooks/domains/workspace/use-repository-branches"; +import { + NO_REPOSITORY, + NO_REPOSITORY_LABEL, + DEFAULT_BRANCH, + DEFAULT_BRANCH_LABEL, + branchPlaceholder, + resolveBaseBranch, + resolveRepositoryId, +} from "@/lib/watcher-repository-default"; + +type PickItem = { id: string; label: string }; + +function PickSelect(props: { + label: string; + description: string; + value: string; + onChange: (v: string) => void; + placeholder: string; + items: PickItem[]; + disabled?: boolean; +}) { + return ( +
+ +

{props.description}

+ +
+ ); +} + +/** + * Shared repository + base-branch picker for the Linear / Jira / Sentry watcher + * dialogs. Binds the watcher to an optional repository so its tasks run against + * that codebase instead of an empty scratch checkout; an empty repository = + * unbound (repo-less task). How the repository is materialised (isolated + * worktree vs in-place) is decided by the executor profile, not this field. The + * base-branch select is disabled until a repository is chosen and defaults to + * the repository's default branch. The `onChange` callbacks receive values + * already collapsed from the dropdown sentinels back to "". + */ +export function WatcherRepositoryFields({ + workspaceId, + repositoryId, + baseBranch, + onRepositoryChange, + onBaseBranchChange, +}: { + workspaceId: string; + repositoryId: string; + baseBranch: string; + onRepositoryChange: (repositoryId: string) => void; + onBaseBranchChange: (baseBranch: string) => void; +}) { + // forceRefresh: a repo created in settings doesn't update the shared store + // slice, and the lazy fetch is gated by isLoaded — so without this the picker + // could miss repositories created since the slice first loaded. The hook owns + // the fetch (keeps data-flow in the hook layer, not the component). + const { repositories } = useRepositories(workspaceId, !!workspaceId, true); + const branchSource = repositoryId ? ({ kind: "id", workspaceId, repositoryId } as const) : null; + const { branches, isLoading: branchesLoading } = useBranches(branchSource, !!repositoryId); + // A branch name can appear twice (local + remote tracking, e.g. "main" and + // origin/"main"), which would emit two — Radix then + // renders every matching item's text in the trigger ("mainmain") and React + // warns on duplicate keys. Dedupe by name so each branch is one option. + const uniqueBranchNames = Array.from(new Set(branches.map((b) => b.name))); + return ( +
+ onRepositoryChange(resolveRepositoryId(v))} + placeholder={NO_REPOSITORY_LABEL} + items={[ + { id: NO_REPOSITORY, label: NO_REPOSITORY_LABEL }, + ...repositories.map((r) => ({ id: r.id, label: r.name })), + ]} + /> + onBaseBranchChange(resolveBaseBranch(v))} + placeholder={branchPlaceholder(repositoryId, branchesLoading)} + items={[ + { id: DEFAULT_BRANCH, label: DEFAULT_BRANCH_LABEL }, + ...uniqueBranchNames.map((name) => ({ id: name, label: name })), + ]} + disabled={!repositoryId || branchesLoading} + /> +
+ ); +} diff --git a/apps/web/hooks/domains/workspace/use-repositories.test.ts b/apps/web/hooks/domains/workspace/use-repositories.test.ts new file mode 100644 index 0000000000..bad3e17fce --- /dev/null +++ b/apps/web/hooks/domains/workspace/use-repositories.test.ts @@ -0,0 +1,78 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; + +const mockListRepositories = vi.fn(); +const mockSetRepositories = vi.fn(); +const mockSetRepositoriesLoading = vi.fn(); + +type Repos = { id: string; name: string }[]; +type MockState = { + repositories: { + itemsByWorkspaceId: Record; + loadingByWorkspaceId: Record; + loadedByWorkspaceId: Record; + }; + setRepositories: typeof mockSetRepositories; + setRepositoriesLoading: typeof mockSetRepositoriesLoading; +}; + +let mockState: MockState; + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (s: MockState) => unknown) => selector(mockState), +})); + +vi.mock("@/lib/api", () => ({ + listRepositories: (...args: unknown[]) => mockListRepositories(...args), +})); + +import { useRepositories } from "./use-repositories"; + +function setup(loaded: boolean) { + vi.clearAllMocks(); + mockState = { + repositories: { + itemsByWorkspaceId: {}, + loadingByWorkspaceId: {}, + loadedByWorkspaceId: { "ws-1": loaded }, + }, + setRepositories: mockSetRepositories, + setRepositoriesLoading: mockSetRepositoriesLoading, + }; +} + +describe("useRepositories", () => { + beforeEach(() => { + mockListRepositories.mockResolvedValue({ repositories: [{ id: "r1", name: "Repo One" }] }); + }); + + it("force-refreshes a fresh list even when the workspace is already loaded", async () => { + setup(/* loaded */ true); + renderHook(() => useRepositories("ws-1", true, true)); + await waitFor(() => expect(mockSetRepositories).toHaveBeenCalled()); + expect(mockListRepositories).toHaveBeenCalledWith("ws-1", undefined, { cache: "no-store" }); + expect(mockSetRepositories).toHaveBeenCalledWith("ws-1", [{ id: "r1", name: "Repo One" }]); + }); + + it("does not fetch on the lazy path when already loaded", async () => { + setup(/* loaded */ true); + renderHook(() => useRepositories("ws-1", true, false)); + await Promise.resolve(); + expect(mockListRepositories).not.toHaveBeenCalled(); + }); + + it("fetches on the lazy path when not yet loaded", async () => { + setup(/* loaded */ false); + renderHook(() => useRepositories("ws-1", true, false)); + await waitFor(() => expect(mockSetRepositories).toHaveBeenCalled()); + expect(mockListRepositories).toHaveBeenCalledWith("ws-1", undefined, { cache: "no-store" }); + }); + + it("does nothing when disabled or workspace is null", async () => { + setup(/* loaded */ false); + renderHook(() => useRepositories(null, true, true)); + renderHook(() => useRepositories("ws-1", false, true)); + await Promise.resolve(); + expect(mockListRepositories).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/hooks/domains/workspace/use-repositories.ts b/apps/web/hooks/domains/workspace/use-repositories.ts index d7675b1e20..ae893ebcd1 100644 --- a/apps/web/hooks/domains/workspace/use-repositories.ts +++ b/apps/web/hooks/domains/workspace/use-repositories.ts @@ -5,7 +5,15 @@ import { listRepositories } from "@/lib/api"; const EMPTY_REPOSITORIES: Repository[] = []; -export function useRepositories(workspaceId: string | null, enabled = true) { +/** + * Loads a workspace's repositories from the store, fetching once when not yet + * loaded. Pass `forceRefresh` to instead pull a fresh list once per workspace on + * mount (e.g. a picker that must reflect repos created since the slice was first + * loaded) — the lazy path is disabled in that mode so there's no double fetch, + * and the per-workspace guard is only marked on success so a failed fetch can + * retry on the next mount. + */ +export function useRepositories(workspaceId: string | null, enabled = true, forceRefresh = false) { const repositories = useAppStore((state) => workspaceId ? (state.repositories.itemsByWorkspaceId[workspaceId] ?? EMPTY_REPOSITORIES) @@ -19,7 +27,11 @@ export function useRepositories(workspaceId: string | null, enabled = true) { ); const setRepositories = useAppStore((state) => state.setRepositories); const setRepositoriesLoading = useAppStore((state) => state.setRepositoriesLoading); - const inFlightRef = useRef(false); + // No in-flight ref: the effect deps (enabled/forceRefresh/workspaceId + stable + // store actions) don't change mid-fetch, so the effect can't re-run and start + // a duplicate fetch for the same workspace; `cancelled` discards stale results + // on workspace switch, and forcedRef/isLoaded gate re-fetches after success. + const forcedRef = useRef(null); useEffect(() => { if (!enabled || !workspaceId) return; @@ -28,11 +40,35 @@ export function useRepositories(workspaceId: string | null, enabled = true) { } }, [enabled, isLoaded, isLoading, setRepositoriesLoading, workspaceId]); + // Force-refresh: pull a fresh list once per workspace, bypassing the + // isLoaded cache. forcedRef is set only on success so a failed fetch retries. useEffect(() => { - if (!enabled || !workspaceId) return; - if (isLoaded || inFlightRef.current) return; + if (!enabled || !workspaceId || !forceRefresh) return; + if (forcedRef.current === workspaceId) return; + let cancelled = false; + setRepositoriesLoading(workspaceId, true); + listRepositories(workspaceId, undefined, { cache: "no-store" }) + .then((response) => { + if (cancelled) return; + forcedRef.current = workspaceId; + setRepositories(workspaceId, response.repositories); + }) + .catch(() => { + // Leave forcedRef unset so the next mount retries; keep cached repos. + }) + .finally(() => { + if (cancelled) return; + setRepositoriesLoading(workspaceId, false); + }); + return () => { + cancelled = true; + }; + }, [enabled, forceRefresh, workspaceId, setRepositories, setRepositoriesLoading]); + + useEffect(() => { + if (!enabled || !workspaceId || forceRefresh) return; + if (isLoaded) return; let cancelled = false; - inFlightRef.current = true; setRepositoriesLoading(workspaceId, true); listRepositories(workspaceId, undefined, { cache: "no-store" }) .then((response) => { @@ -44,14 +80,13 @@ export function useRepositories(workspaceId: string | null, enabled = true) { setRepositories(workspaceId, []); }) .finally(() => { - inFlightRef.current = false; if (cancelled) return; setRepositoriesLoading(workspaceId, false); }); return () => { cancelled = true; }; - }, [enabled, isLoaded, setRepositories, setRepositoriesLoading, workspaceId]); + }, [enabled, forceRefresh, isLoaded, setRepositories, setRepositoriesLoading, workspaceId]); return { repositories, isLoading }; } diff --git a/apps/web/lib/state/slices/jira/jira-slice.test.ts b/apps/web/lib/state/slices/jira/jira-slice.test.ts index 82eff6d0ef..3ceb8e5438 100644 --- a/apps/web/lib/state/slices/jira/jira-slice.test.ts +++ b/apps/web/lib/state/slices/jira/jira-slice.test.ts @@ -18,6 +18,8 @@ function watch(id: string, overrides: Partial = {}): JiraIssueWa workspaceId: "ws-1", workflowId: "wf-1", workflowStepId: "step-1", + repositoryId: "", + baseBranch: "", jql: "project = PROJ", agentProfileId: "", executorProfileId: "", diff --git a/apps/web/lib/state/slices/linear/linear-slice.test.ts b/apps/web/lib/state/slices/linear/linear-slice.test.ts index 85f0b23f37..6fd2f5dbe2 100644 --- a/apps/web/lib/state/slices/linear/linear-slice.test.ts +++ b/apps/web/lib/state/slices/linear/linear-slice.test.ts @@ -18,6 +18,8 @@ function watch(id: string, overrides: Partial = {}): LinearIss workspaceId: "ws-1", workflowId: "wf-1", workflowStepId: "step-1", + repositoryId: "", + baseBranch: "", filter: { teamKey: "ENG" }, agentProfileId: "", executorProfileId: "", diff --git a/apps/web/lib/types/jira.ts b/apps/web/lib/types/jira.ts index a03676f95c..ed04b38ef9 100644 --- a/apps/web/lib/types/jira.ts +++ b/apps/web/lib/types/jira.ts @@ -103,6 +103,14 @@ export interface JiraIssueWatch { workspaceId: string; workflowId: string; workflowStepId: string; + /** + * Optional repository binding. Empty string = unbound: watcher-created tasks + * launch in a blank scratch checkout (historical behaviour). When set, tasks + * launch in an isolated worktree of this repository cut from `baseBranch`. + */ + repositoryId: string; + /** Branch the per-task worktree is cut from; empty = the repo's default. */ + baseBranch: string; jql: string; agentProfileId: string; executorProfileId: string; @@ -125,6 +133,10 @@ export interface CreateJiraIssueWatchInput { workspaceId: string; workflowId: string; workflowStepId: string; + /** Optional repository binding; empty/omitted = unbound (repo-less task). */ + repositoryId?: string; + /** Base branch for the worktree; empty defaults to the repo's default branch. */ + baseBranch?: string; jql: string; agentProfileId?: string; executorProfileId?: string; @@ -139,6 +151,8 @@ export interface CreateJiraIssueWatchInput { export interface UpdateJiraIssueWatchInput { workflowId?: string; workflowStepId?: string; + repositoryId?: string; + baseBranch?: string; jql?: string; agentProfileId?: string; executorProfileId?: string; diff --git a/apps/web/lib/types/linear.ts b/apps/web/lib/types/linear.ts index 936ea82838..95d84c65a5 100644 --- a/apps/web/lib/types/linear.ts +++ b/apps/web/lib/types/linear.ts @@ -124,6 +124,14 @@ export interface LinearIssueWatch { workspaceId: string; workflowId: string; workflowStepId: string; + /** + * Optional repository binding. Empty string = unbound: watcher-created tasks + * launch in a blank scratch checkout (historical behaviour). When set, tasks + * launch in an isolated worktree of this repository cut from `baseBranch`. + */ + repositoryId: string; + /** Branch the per-task worktree is cut from; empty = the repo's default. */ + baseBranch: string; filter: LinearSearchFilter; agentProfileId: string; executorProfileId: string; @@ -146,6 +154,10 @@ export interface CreateLinearIssueWatchInput { workspaceId: string; workflowId: string; workflowStepId: string; + /** Optional repository binding; empty/omitted = unbound (repo-less task). */ + repositoryId?: string; + /** Base branch for the worktree; empty defaults to the repo's default branch. */ + baseBranch?: string; filter: LinearSearchFilter; agentProfileId?: string; executorProfileId?: string; @@ -160,6 +172,8 @@ export interface CreateLinearIssueWatchInput { export interface UpdateLinearIssueWatchInput { workflowId?: string; workflowStepId?: string; + repositoryId?: string; + baseBranch?: string; filter?: LinearSearchFilter; agentProfileId?: string; executorProfileId?: string; diff --git a/apps/web/lib/types/sentry.ts b/apps/web/lib/types/sentry.ts index 46325adbfa..85d75f3e05 100644 --- a/apps/web/lib/types/sentry.ts +++ b/apps/web/lib/types/sentry.ts @@ -87,6 +87,14 @@ export interface SentryIssueWatch { workspaceId: string; workflowId: string; workflowStepId: string; + /** + * Optional repository binding. Empty string = unbound: watcher-created tasks + * launch in a blank scratch checkout (historical behaviour). When set, tasks + * launch in an isolated worktree of this repository cut from `baseBranch`. + */ + repositoryId: string; + /** Branch the per-task worktree is cut from; empty = the repo's default. */ + baseBranch: string; filter: SentrySearchFilter; agentProfileId: string; executorProfileId: string; @@ -103,6 +111,10 @@ export interface CreateSentryIssueWatchRequest { workspaceId: string; workflowId: string; workflowStepId: string; + /** Optional repository binding; empty/omitted = unbound (repo-less task). */ + repositoryId?: string; + /** Base branch for the worktree; empty defaults to the repo's default branch. */ + baseBranch?: string; filter: SentrySearchFilter; agentProfileId: string; executorProfileId: string; @@ -115,6 +127,8 @@ export interface CreateSentryIssueWatchRequest { export interface UpdateSentryIssueWatchRequest { workflowId?: string; workflowStepId?: string; + repositoryId?: string; + baseBranch?: string; filter?: SentrySearchFilter; agentProfileId?: string; executorProfileId?: string; diff --git a/apps/web/lib/watcher-repository-default.test.ts b/apps/web/lib/watcher-repository-default.test.ts new file mode 100644 index 0000000000..a5c77ad7f6 --- /dev/null +++ b/apps/web/lib/watcher-repository-default.test.ts @@ -0,0 +1,72 @@ +import { describe, it, expect } from "vitest"; +import { + NO_REPOSITORY, + DEFAULT_BRANCH, + DEFAULT_BRANCH_LABEL, + resolveRepositoryId, + resolveBaseBranch, + clearWorkspaceScopedForm, + branchPlaceholder, +} from "./watcher-repository-default"; + +describe("resolveRepositoryId", () => { + it("collapses the no-repository sentinel to an empty string", () => { + expect(resolveRepositoryId(NO_REPOSITORY)).toBe(""); + }); + it("passes a real repository id through unchanged", () => { + expect(resolveRepositoryId("repo-123")).toBe("repo-123"); + }); +}); + +describe("resolveBaseBranch", () => { + it("collapses the default-branch sentinel to an empty string", () => { + expect(resolveBaseBranch(DEFAULT_BRANCH)).toBe(""); + }); + it("passes a real branch name through unchanged", () => { + expect(resolveBaseBranch("main")).toBe("main"); + }); +}); + +describe("branchPlaceholder", () => { + it("prompts to pick a repository first when none is selected", () => { + expect(branchPlaceholder("", false)).toBe("Pick a repository first"); + // No repo takes precedence over the loading state. + expect(branchPlaceholder("", true)).toBe("Pick a repository first"); + }); + it("shows a loading hint while branches stream in", () => { + expect(branchPlaceholder("repo-1", true)).toBe("Loading…"); + }); + it("shows the default-branch label once a repo is selected and loaded", () => { + expect(branchPlaceholder("repo-1", false)).toBe(DEFAULT_BRANCH_LABEL); + }); +}); + +describe("clearWorkspaceScopedForm", () => { + const base = { + workspaceId: "ws-1", + workflowId: "wf-1", + workflowStepId: "step-1", + repositoryId: "repo-1", + baseBranch: "main", + // an unrelated field that must be preserved + prompt: "keep me", + }; + + it("clears workflow/step + repository binding when the workspace changes", () => { + const next = clearWorkspaceScopedForm(base, "ws-2"); + expect(next).toEqual({ + workspaceId: "ws-2", + workflowId: "", + workflowStepId: "", + repositoryId: "", + baseBranch: "", + prompt: "keep me", + }); + }); + + it("returns the previous object unchanged when the workspace is the same", () => { + const next = clearWorkspaceScopedForm(base, "ws-1"); + // Same reference: no-op so the user's selections are preserved. + expect(next).toBe(base); + }); +}); diff --git a/apps/web/lib/watcher-repository-default.ts b/apps/web/lib/watcher-repository-default.ts new file mode 100644 index 0000000000..054885a481 --- /dev/null +++ b/apps/web/lib/watcher-repository-default.ts @@ -0,0 +1,63 @@ +// Sentinels for the optional repository binding in the watcher dialogs (Linear, +// Jira, Sentry). The form stores "" to mean "no repository" (repo-less task, +// the historical behaviour) and "" for the base branch to mean "the +// repository's default branch". Radix disallows , so the +// dropdown options carry these sentinel values and map back to "" on change. +// +// Note: there is no "step default" for a repository — a workflow step has no +// repository association — so the empty state means literally "no repository". +// The sentinel values contain ":" which is illegal in Git ref names and never +// appears in a repository UUID, so they cannot collide with a real branch name +// or repository id (which would otherwise let resolve*() silently rewrite a +// user's selection). +export const NO_REPOSITORY = "kandev:no-repository"; +export const NO_REPOSITORY_LABEL = "(no repository)"; +export const DEFAULT_BRANCH = "kandev:default-branch"; +export const DEFAULT_BRANCH_LABEL = "(repository default branch)"; + +// Map a repository select value back to the stored id, collapsing the sentinel +// to "" so the payload keeps signalling "no repository". +export function resolveRepositoryId(value: string): string { + return value === NO_REPOSITORY ? "" : value; +} + +// Map a branch select value back to the stored branch, collapsing the sentinel +// to "" so the backend fills the repository's default branch at save time. +export function resolveBaseBranch(value: string): string { + return value === DEFAULT_BRANCH ? "" : value; +} + +// clearWorkspaceScopedForm switches a watcher form to a new workspace and clears +// every field scoped to the previous one (workflow, step, and the repository +// binding) so a stale cross-workspace reference can't be saved. Shared across +// the Linear/Jira/Sentry dialogs to keep this data-loss guard in one place. +export function clearWorkspaceScopedForm< + T extends { + workspaceId: string; + workflowId: string; + workflowStepId: string; + repositoryId: string; + baseBranch: string; + }, +>(prev: T, workspaceId: string): T { + // No-op when the workspace didn't actually change, so re-selecting the current + // workspace doesn't wipe the user's workflow/step/repository choices. + if (prev.workspaceId === workspaceId) return prev; + return { + ...prev, + workspaceId, + workflowId: "", + workflowStepId: "", + repositoryId: "", + baseBranch: "", + }; +} + +// branchPlaceholder mirrors the workflow-step placeholder pattern: it nudges the +// user to pick a repository first, then shows a loading hint while branches +// stream in. +export function branchPlaceholder(repositoryId: string, loading: boolean): string { + if (!repositoryId) return "Pick a repository first"; + if (loading) return "Loading…"; + return DEFAULT_BRANCH_LABEL; +} diff --git a/docs/plans/watcher-repository-binding/plan.md b/docs/plans/watcher-repository-binding/plan.md new file mode 100644 index 0000000000..ce964e1eef --- /dev/null +++ b/docs/plans/watcher-repository-binding/plan.md @@ -0,0 +1,491 @@ +--- +spec: none — self-contained PR plan (see "Problem" and "Design" below) +created: 2026-06-24 +status: draft +area: backend, frontend +related_prs: ["#1094 (watcher self-heal)", "#1124 (use-step-default reset pattern)", "ADR 0013 (multi-branch tasks)", "ADR 0008 (DB upgrade safety)"] +--- + +# Implementation Plan: Issue-Watcher Repository Binding (Linear / Jira / Sentry) + +## Problem + +The Linear, Jira, and Sentry issue-watchers create tasks with **no repository +associated**. As a result their agents launch into a blank scratch git repo +instead of the target codebase ("started with no repository checkout"). + +Validated end-to-end against the current source: + +- The watch models carry no repository field: + - Linear `IssueWatch` — `apps/backend/internal/linear/models.go:190-213` + - Jira `IssueWatch` — `apps/backend/internal/jira/models.go:162-185` + - Sentry `IssueWatch` — `apps/backend/internal/sentry/models.go:133-156` + All three carry only workspace/workflow/step, the per-integration filter + (`Filter SearchFilter` for Linear/Sentry, `JQL string` for Jira), + agent/executor profile, prompt, enabled, poll interval, max-inflight, and + error stamps. (The models even carry an explicit comment: *"Linear/JIRA + issues have no repository affinity"* — `linear/models.go:188-189`, + `jira/models.go:159-161`.) + +- The orchestrator sources build an `IssueTaskRequest` with no `Repositories`: + - `LinearWatcherSource.BuildTaskRequest` — `apps/backend/internal/orchestrator/source_linear.go:60-81` + - `JiraWatcherSource.BuildTaskRequest` — `apps/backend/internal/orchestrator/source_jira.go:57-80` + - `SentryWatcherSource.BuildTaskRequest` — `apps/backend/internal/orchestrator/source_sentry.go:96-118` + +- At launch, the worktree env-preparer branches on whether the launch request + carries repositories. `EnvPrepareRequest.RepoSpecs()` + (`apps/backend/internal/agent/runtime/lifecycle/env_preparer.go:86-91`) and + `manager_launch.go:650-677` take the worktree path only when + `len(req.Repositories) > 0`. With zero repositories the launch falls to + `initGitRepo` (`apps/backend/internal/agent/runtime/lifecycle/manager_launch.go:1250-1300`, + invoked at `manager_launch.go:305`), which `git init`s a blank scratch repo + with an empty `.gitkeep` and an "Initial commit" authored by + `Kandev Quick Chat `. The agent then runs with no + source checkout. + +The `workspace_path` override is **not** a fix: +- `MetaKeyWorkspacePath` (`apps/backend/internal/task/models/models.go:66`) is + only written from `CreateTaskRequest.WorkspacePath` + (`apps/backend/internal/task/service/service_requests.go:47`, applied at + `service_tasks.go:250-254`) — the UI "starting folder" picker. Watchers never + set it. +- When set, it runs the agent **directly in that folder with no per-task + isolation** (`apps/backend/internal/orchestrator/executor/executor_execute.go:271`) + — unacceptable for a fan-out watcher creating many concurrent tasks + (e.g. a Linear watch with `maxInflightTasks: 10`). + +## Goal & Non-Goals + +**Goal.** Let each Linear / Jira / Sentry issue-watch optionally bind a +**repository** (`repository_id`) and a **base branch** (`base_branch`), so +watcher-created tasks carry `Repositories` and take the existing +`len(req.Repositories) > 0` path → an **isolated git worktree per task** cut +from the bound repo at the base branch. This mirrors the GitHub watcher. + +**Non-Goals.** +- No change to GitHub watcher behavior — it is the reference template and is + out of scope to modify. +- No multi-repo / multi-branch binding for watchers. A watch binds **at most + one** `(repository_id, base_branch)`. (Multi-branch tasks — ADR 0013 — remain + a manual/agent-driven flow.) +- No automatic re-pointing of existing repo-less watches; they keep working + exactly as today. +- Deleted-repository self-heal is *scoped* here (see "Self-heal interplay") but + its full implementation may land as a follow-up. + +## Design + +### Reference template — the GitHub watcher + +The transport already exists. `IssueTaskRequest` +(`apps/backend/internal/orchestrator/event_handlers_github.go:83-91`) already +has `Repositories []IssueTaskRepository`, and `IssueTaskRepository` +(`event_handlers_github.go:94-97`) is exactly: + +```go +type IssueTaskRepository struct { + RepositoryID string + BaseBranch string +} +``` + +**No change to `IssueTaskRequest` or `IssueTaskRepository` is required** — the +three watcher sources simply need to populate the existing slice. The GitHub +source resolves owner/name → repo via a resolver; our watchers store a kandev +`repository_id` directly, so they can build the slice with no resolution step. + +### Core change & the invariant + +Each watch gains two optional columns/fields: `repository_id` and +`base_branch`. The single behavioural switch is in each source's +`BuildTaskRequest`: + +```go +// only when bound; otherwise leave Repositories nil (today's behaviour) +if e.RepositoryID != "" { + req.Repositories = []IssueTaskRepository{{ + RepositoryID: e.RepositoryID, + BaseBranch: e.BaseBranch, + }} +} +``` + +> **Backward-compatibility invariant (mandatory).** An empty `repository_id` +> produces `Repositories == nil`, which preserves today's exact behaviour +> (repo-less → blank scratch via `initGitRepo`). Only a non-empty +> `repository_id` switches a watch to the worktree path. This invariant is +> covered by a dedicated test per integration (see Tests). + +### Where `base_branch` is resolved + +Resolve at **create/update time**, store the concrete branch: + +- If `repository_id` is set and `base_branch` is empty, default it to the bound + repository's `DefaultBranch` and persist that concrete value. +- This keeps `BuildTaskRequest` a pure projection of stored state (no I/O in the + orchestrator hot path) and matches how the watcher already pre-resolves + agent/executor at config time. +- Trade-off: if the repo's default branch later changes, the stored value is + stale until the watch is edited. Acceptable and explicit; an empty stored + `base_branch` also remains valid — the worktree manager already falls back to + the repo default branch when `BaseBranch` is empty + (`apps/backend/internal/worktree/manager_base_branch_fallback_test.go` exercises + this), so a stale/blank value degrades safely rather than failing. + +### Repository lookup dependency (validation + default-branch resolution) + +Repositories live in the **task** package, not in the integration packages: +`models.Repository` (`apps/backend/internal/task/models/models.go:757`) carries +`WorkspaceID` and `DefaultBranch`; the store exposes +`GetRepository(ctx, id)` (`apps/backend/internal/task/repository/interface.go:200`) +and `ListRepositories(ctx, workspaceID)` (`interface.go:203`). + +The Linear/Jira/Sentry services must **not** import the task service directly +(import-cycle risk). Mirror the existing post-construction wiring used for +`watchreset.TaskDeleter`: + +- Each service already declares a `taskDeleter` field wired *after* + construction via `SetTaskDeleter` to dodge the cycle + (`linear/service.go:37-53`, `jira/service.go:40-53`, `sentry/service.go:49-57`), + called from `apps/backend/internal/backendapp/helpers.go:520-526`. +- Add a parallel minimal interface, e.g.: + + ```go + // package linear (and jira, sentry) + type RepositoryLookup interface { + GetRepository(ctx context.Context, id string) (workspaceID, defaultBranch string, ok bool) + } + func (s *Service) SetRepositoryLookup(rl RepositoryLookup) { s.repoLookup = rl } + ``` + +- Wire it in `backendapp/helpers.go` (next to `SetTaskDeleter`) with a thin + adapter over `services.Task` (the same `services.Task` already used by the + `taskDeleterAdapter` in `backendapp/main.go:493`). +- When `repoLookup` is nil (e.g. unit tests that don't wire it), validation + skips the membership/default-branch step — preserves testability and + fail-open behaviour. + +## Backend — file-by-file + +The three integrations are near-identical; each bullet applies to all three +unless noted. Per-integration divergences: Jira stores its filter as +`jql TEXT`; Linear & Sentry store `filter_json TEXT`. Sentry's profile fields in +the UI don't use the `STEP_DEFAULT` sentinel (frontend note below). None of +these affect the repository columns. + +### 1. Schema + migration (expand-only, nullable-by-default) + +Add two columns to each watch table, matching the house style of the existing +optional id columns (`agent_profile_id`, `executor_profile_id`), which are +`TEXT NOT NULL DEFAULT ''` — **empty string = unbound**. This satisfies the +"nullable / optional / expand-only" requirement (the column is always present, +defaults to "no binding") while keeping scans `string`-typed (no `*string`). + +- **Linear** — `apps/backend/internal/linear/store.go` + - Add `repository_id TEXT NOT NULL DEFAULT ''` and + `base_branch TEXT NOT NULL DEFAULT ''` to `createTablesSQL` + (table block at `store.go:55-75`). + - Add migration helper `addIssueWatchRepositoryColumns(db)` mirroring + `addMaxInflightTasksColumn` (`store.go:114-129`) and + `addIssueWatchLastErrorColumns` (`store.go:136-152`): use the `tableColumns` + helper (`store.go:262-284`, `PRAGMA table_info`) to add each column only if + missing. + - Register it in `initSchema` (`store.go:94-108`) after the existing + column-add steps. +- **Jira** — `apps/backend/internal/jira/store.go` (table block `59-93`): same + two columns + a `addIssueWatchRepositoryColumns` migration in `initSchema`. +- **Sentry** — `apps/backend/internal/sentry/store.go` (table block `41-71`): + same; Sentry already keeps its incremental column-adds in a dedicated + migration method — extend it. + +Rationale: this is the established Kandev migration pattern (idempotent +`PRAGMA table_info` guard + `ALTER TABLE ADD COLUMN`), and it is observed/logged +through the boot-time `MigrateLogger` snapshot path per ADR 0008. New databases +get the columns straight from `createTablesSQL`. + +### 2. Store read/write columns + +- **INSERT** lists: add `repository_id, base_branch` + - Linear `store.go:80-84` (currently 16 placeholders → 18) and `CreateIssueWatch` (`store.go:94-117`). + - Jira `store.go:400-404`. + - Sentry `store_issue_watch.go:90-94`. +- **SELECT** lists / scan structs: add both columns + - Linear `store.go:86-90` (keep the `COALESCE(...,'')` style for old DBs). + - Jira / Sentry equivalents. +- **UPDATE**: add `repository_id = ?, base_branch = ?` + - Linear `UpdateIssueWatch` (`store.go:194-214`); Jira/Sentry equivalents. + `workspace_id` stays immutable (unchanged). + +### 3. Model + request DTOs + patch + +- Add `RepositoryID string` and `BaseBranch string` to each `IssueWatch` + (`linear/models.go:190-213`, `jira/models.go:162-185`, + `sentry/models.go:133-156`). +- Add the same to `CreateIssueWatchRequest` + (`linear/models.go:248-259`, `jira/models.go:218-229`, `sentry/models.go:189-200`) + and `UpdateIssueWatchRequest` + (`linear/models.go:265-278`, `jira/models.go:235-248`, `sentry/models.go:204-217`) + as pointers (`*string`) for tri-state PATCH, mirroring the existing optional + fields. +- Patch them in `applyIssueWatchPatch` + (`linear/service_issue_watch.go:367-398`, `jira/service.go:772-803`, + `sentry/service_issue_watch.go:324-355`). + +### 4. Validation + default-branch resolution + +Extend `validateIssueWatchCreate` +(`linear/service_issue_watch.go:242-264`, `jira/service.go:729-750`, +`sentry/service_issue_watch.go:244-263`) and the update path: + +- If `repository_id` is non-empty and `repoLookup` is wired: + - Look up the repo; reject (`ErrInvalidConfig`) if it doesn't exist or its + `workspaceID != req.WorkspaceID` (workspace-scoping / IDOR guard). + - If `base_branch` is empty, set it to the repo's `DefaultBranch` before + persisting. +- If `repository_id` is empty: `base_branch` is forced empty (no orphan branch + without a repo) and all checks are skipped — the repo-less invariant. + +### 5. Event payload + +Add `RepositoryID string` and `BaseBranch string` to: +- `NewLinearIssueEvent` (`linear/models.go:232-245`) +- `NewJiraIssueEvent` (`jira/models.go:202-215`) +- `NewSentryIssueEvent` (`sentry/models.go:173-186`) + +Populate them from the watch in the publishers: +- `publishNewLinearIssueEvent` (`linear/service_issue_watch.go:206-228`) +- `publishNewJiraIssueEvent` (`jira/service.go:691-713`) +- `publishNewSentryIssueEvent` (`sentry/service_issue_watch.go:220-242`) + +### 6. Source `BuildTaskRequest` + +In each source, after building the existing request, conditionally set +`Repositories` from the event (the snippet under "Core change"): +- `source_linear.go:60-81` +- `source_jira.go:57-80` +- `source_sentry.go:96-118` + +No change to the `WatcherSource` interface +(`apps/backend/internal/orchestrator/watcher_dispatch.go:107-164`) — its +`BuildTaskRequest(evt any) (*IssueTaskRequest, error)` contract already returns +the struct that carries `Repositories`. + +### 7. HTTP API + +The create/update handlers already bind JSON into the request DTOs, so adding +the fields to the DTOs is most of the work: +- Linear `httpCreateIssueWatch` / `httpUpdateIssueWatch` + (`linear/handlers_issue_watch.go:31-61`). +- Jira routes (`jira/handlers.go:41-48`), Sentry routes + (`sentry/handlers_issue_watch.go:12-21`). +No new endpoints; existing workspace-ownership guards +(`assertWatchInWorkspace` / `workspaceMatches`) are unchanged. + +## Frontend — file-by-file + +### Types & API client (add `repositoryId?: string`, `baseBranch?: string`) + +- Linear: `apps/web/lib/types/linear.ts:122-171` (`LinearIssueWatch`, + `CreateLinearIssueWatchInput`, `UpdateLinearIssueWatchInput`); + client `apps/web/lib/api/domains/linear-api.ts:136-196`. +- Jira: `apps/web/lib/types/jira.ts:101-150`; client + `apps/web/lib/api/domains/jira-api.ts`. +- Sentry: `apps/web/lib/types/sentry.ts:85-125`; client + `apps/web/lib/api/domains/sentry-api.ts`. + +### Watcher dialogs — add a Repository + Base-branch picker + +Place a new optional "Repository" subsection right after the Workflow/Step +("Automation") block in each dialog: +- Linear: `apps/web/components/linear/linear-issue-watch-dialog.tsx` + (after the workflow/step + profile fields, ~`413-457`). +- Jira: `apps/web/components/jira/jira-issue-watch-dialog.tsx` (~`308-352`). +- Sentry: `apps/web/components/sentry/sentry-issue-watch-dialog.tsx` (~`341-375`). + +**Recommended control:** a pair of `SelectField`s — a repository select and a +base-branch select — following the existing **"(use step default)"** reset +pattern from PR #1124 (`apps/web/lib/watcher-profile-default.ts`: +`STEP_DEFAULT`, `STEP_DEFAULT_LABEL`, `resolveProfileId`). A "(no repository — +use step default)" sentinel option maps back to `""`, preserving repo-less +watches. The branch select is disabled until a repository is chosen and +defaults to the repo's default branch. + +- Source the workspace repositories from the workspace store slice + (`apps/web/lib/state/slices/workspace/`) already available to these dialogs + (they render a workspace picker). +- Source branches from the existing + `apps/web/hooks/domains/workspace/use-repository-branches.ts` hook (the same + hook the task-create repo picker uses). +- The heavier multi-repo chip component + (`apps/web/components/task-create-dialog-repo-chips.tsx`, `RepoChipsRow`) is + **not** reused wholesale — it is built for N-repo/N-branch task creation with + on-machine discovery and fresh-branch toggles. A single repo+branch pair is + the right fit; reuse only the `use-repository-branches` hook. + +> Sentry's profile selects currently do **not** use the `STEP_DEFAULT` sentinel +> (they store the id/empty directly). Keep the new repository control consistent +> *within* the Sentry dialog (sentinel optional); unifying Sentry's profile +> fields is out of scope. + +## Backward-compatibility invariant + +Existing repo-less watches must behave exactly as today: +- Migration adds columns defaulting to `''` → every existing row is "unbound". +- Empty `repository_id` ⇒ `Repositories == nil` in `BuildTaskRequest` ⇒ + `initGitRepo` blank-scratch path, unchanged. +- A dedicated regression test per integration asserts an unbound watch yields a + request with empty `Repositories` (Tests §). + +## Self-heal interplay (PR #1094) + +The dispatch coordinator self-heals when the bound **agent profile** is +soft-deleted: `preflightDeletedProfile` +(`apps/backend/internal/orchestrator/watcher_dispatch.go:170-206`) calls +`src.SelfHeal(...)` → `service.DisableIssueWatchWithError` +(`source_linear.go:136`, `source_jira.go:134`, `source_sentry.go:64`). There is +**no repository existence check** in the coordinator today. + +**Decision for this PR:** *fail-open at dispatch, validate at config time.* +- Create/update validation rejects a non-existent or cross-workspace repo, so a + bound repo is valid when stored. +- If a bound repo is later soft-deleted, the launch path already degrades + safely: a missing/empty checkout falls back to repo-default/blank behaviour + rather than crashing the dispatch loop. +- A symmetric "disable watch + stamp `last_error` when the bound repository is + deleted" preflight (mirroring `preflightDeletedProfile`) is **documented as a + follow-up**, not implemented here, to keep this PR focused. Tracked as an open + question below. + +## Tests + +Backend (`*_test.go` beside each source; all three integrations): + +- **Store CRUD round-trip** — bound watch persists/loads `repository_id` + + `base_branch`. *Files:* `linear/store_issue_watch_test.go` (extend the + existing `newTestIssueWatch` helper at `:9-18`), Jira/Sentry equivalents. + *How:* SQLite-backed, table-driven. +- **Migration** — open a DB created from the *old* schema (no repo columns), + run `initSchema`, assert both columns exist and existing rows default to `''`. + *How:* `PRAGMA table_info` assertion, mirroring the existing + `addIssueWatchLastErrorColumns` coverage. +- **Validation** — (a) bound repo in another workspace is rejected; + (b) empty `base_branch` is filled from the repo's `DefaultBranch`; + (c) empty `repository_id` skips all repo checks and forces empty branch. + *How:* service test with a fake `RepositoryLookup`. +- **Source `BuildTaskRequest`** — bound event ⇒ + `Repositories == [{repoID, baseBranch}]`; **unbound event ⇒ + `Repositories == nil`** (the invariant). *Files:* `source_linear_test.go`, + `source_jira_test.go`, `source_sentry_test.go`. +- **Dispatch parity / self-heal unchanged** — extend + `watcher_dispatch_selfheal_test.go:81-113` coverage so a bound repo flows + through `Dispatch` without disturbing the deleted-profile short-circuit. +- **Disable parity** — existing `store_issue_watch_disable_test.go` continues to + pass with the new columns present. + +Frontend: +- Type/compile parity (`pnpm --filter @kandev/web typecheck`); a small unit test + on the create-payload builder asserting `repositoryId`/`baseBranch` are sent + and that the "(no repository)" sentinel maps to `""`. + +## E2E Tests + +- **Scenario:** GIVEN a Linear issue-watch bound to repository R at branch B, + WHEN the watcher creates a task, THEN the task launches in an isolated + worktree of R at B (not a blank scratch repo). + *File:* `apps/web/e2e/tests/integrations/linear-watch-repository.spec.ts` + (mock Linear provider, `KANDEV_MOCK_LINEAR`). *Verify:* created task has a + repository chip / worktree path under the task dir, not the `.gitkeep` + scratch. +- **Scenario (invariant):** GIVEN an unbound watch, WHEN it creates a task, + THEN behaviour is unchanged (repo-less task). Can be asserted at the + source/service layer instead of full E2E if cheaper. +- Jira/Sentry mirror the Linear E2E (mock providers) — at least the bound-repo + happy path for one of them plus unit parity for the others. + +## Rollout + +- **No feature flag.** This is additive and behind an empty default + (`repository_id = ''` = today's behaviour); a flag would only gate a strictly + opt-in field. (Per the repo's flag guidance in `CLAUDE.md`, flags are for + toggling behaviour for *all* users — not the case here.) +- **Migration** is expand-only and idempotent; runs on first boot of the new + binary and is captured by the pre-migration `VACUUM INTO` snapshot (ADR 0008). +- **Order:** ship backend (schema → model/DTO → store → validation/lookup wiring + → event → source → API) and frontend in the same release; the UI degrades + gracefully if pointed at an older backend (extra fields ignored), and an older + UI simply never sends the new fields. + +## Risks + +- **Stale default branch.** Stored `base_branch` can drift from the repo's + default. Mitigated by the worktree manager's empty-branch fallback and by + re-resolution on edit. Low impact. +- **Deleted bound repository.** Until the follow-up preflight lands, a deleted + repo degrades to fallback/blank rather than a clean disable+error. Documented; + fail-open. +- **Cross-package coupling.** New `RepositoryLookup` dependency in three + integration services. Mitigated by the established post-construction + `Set...` wiring pattern (no import cycle) and nil-safe validation. +- **Three-integration drift.** The change is mechanical but triplicated; a + shared test matrix and identical helper signatures reduce the chance of one + integration diverging. +- **Sentry sentinel inconsistency.** Sentry profile fields don't use + `STEP_DEFAULT`; keep the new control self-consistent and avoid scope creep. + +## Implementation Waves + +> Single self-contained plan (no sibling task files). Backend packages are +> independent and can be parallelised per integration; frontend is sequential +> (shared build/types); E2E last. + +``` +Wave 1 — Backend contracts & persistence (parallel per integration): +- [ ] Linear: schema+migration, model/DTO, store, validation+lookup, event, source +- [ ] Jira: schema+migration, model/DTO, store, validation+lookup, event, source +- [ ] Sentry: schema+migration, model/DTO, store, validation+lookup, event, source +- [ ] backendapp wiring: SetRepositoryLookup adapter over services.Task (helpers.go) + +Wave 2 — Frontend (sequential): +- [ ] types + api clients (linear/jira/sentry) +- [ ] repository + base-branch picker in the three watcher dialogs + +Wave 3 — E2E: +- [ ] bound-repo worktree happy path (+ unbound invariant) +``` + +## Verification Commands + +Format first (formatters may split lines and trip the complexity linter): + +```bash +make fmt +``` + +Targeted backend tests: + +```bash +cd apps/backend && go test ./internal/linear/... ./internal/jira/... ./internal/sentry/... ./internal/orchestrator/... +``` + +Frontend: + +```bash +cd apps && pnpm --filter @kandev/web typecheck +cd apps/web && pnpm e2e linear-watch-repository.spec.ts +``` + +Full gate: + +```bash +make typecheck test lint +``` + +## Open Questions + +1. **Deleted-repository self-heal** — implement the symmetric + `preflightDeletedRepository` (disable + `last_error`) in this PR, or ship it + as the documented follow-up? Plan assumes follow-up. +2. **Sentry profile sentinel** — leave Sentry's profile selects without the + `STEP_DEFAULT` sentinel (and add the repo control consistently within that + style), or unify Sentry with Linear/Jira in a separate cleanup? Plan keeps it + out of scope.