diff --git a/apps/backend/internal/github/controller.go b/apps/backend/internal/github/controller.go index 817f3fe806..e21f3c1de7 100644 --- a/apps/backend/internal/github/controller.go +++ b/apps/backend/internal/github/controller.go @@ -289,13 +289,20 @@ func (c *Controller) httpDeletePRWatch(ctx *gin.Context) { ctx.JSON(http.StatusOK, gin.H{"deleted": true}) } +// httpListReviewWatches returns watches scoped to one workspace when +// `workspace_id` is supplied, or every watch across all workspaces when it +// is absent. The integration settings page uses the unscoped form. func (c *Controller) httpListReviewWatches(ctx *gin.Context) { workspaceID := ctx.Query("workspace_id") + var ( + watches []*ReviewWatch + err error + ) if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id query parameter required"}) - return + watches, err = c.service.ListAllReviewWatches(ctx.Request.Context()) + } else { + watches, err = c.service.ListReviewWatches(ctx.Request.Context(), workspaceID) } - watches, err := c.service.ListReviewWatches(ctx.Request.Context(), workspaceID) if err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -540,11 +547,15 @@ func (c *Controller) httpGetStats(ctx *gin.Context) { func (c *Controller) httpListIssueWatches(ctx *gin.Context) { workspaceID := ctx.Query("workspace_id") + var ( + watches []*IssueWatch + err error + ) if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id query parameter required"}) - return + watches, err = c.service.ListAllIssueWatches(ctx.Request.Context()) + } else { + watches, err = c.service.ListIssueWatches(ctx.Request.Context(), workspaceID) } - watches, err := c.service.ListIssueWatches(ctx.Request.Context(), workspaceID) if err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return diff --git a/apps/backend/internal/github/service.go b/apps/backend/internal/github/service.go index 02d25f713c..5130d9b9d4 100644 --- a/apps/backend/internal/github/service.go +++ b/apps/backend/internal/github/service.go @@ -1048,6 +1048,11 @@ func (s *Service) ListReviewWatches(ctx context.Context, workspaceID string) ([] return s.store.ListReviewWatches(ctx, workspaceID) } +// ListAllReviewWatches returns every review watch across all workspaces. +func (s *Service) ListAllReviewWatches(ctx context.Context) ([]*ReviewWatch, error) { + return s.store.ListAllReviewWatches(ctx) +} + // UpdateReviewWatch updates a review watch. func (s *Service) UpdateReviewWatch(ctx context.Context, id string, req *UpdateReviewWatchRequest) error { rw, err := s.store.GetReviewWatch(ctx, id) @@ -1741,6 +1746,11 @@ func (s *Service) ListIssueWatches(ctx context.Context, workspaceID string) ([]* return s.store.ListIssueWatches(ctx, workspaceID) } +// ListAllIssueWatches returns every issue watch across all workspaces. +func (s *Service) ListAllIssueWatches(ctx context.Context) ([]*IssueWatch, error) { + return s.store.ListAllIssueWatches(ctx) +} + // UpdateIssueWatch updates an issue watch. // //nolint:dupl // mirrors UpdateReviewWatch — different types, same structure diff --git a/apps/backend/internal/github/store.go b/apps/backend/internal/github/store.go index d064abbefc..151de13bfe 100644 --- a/apps/backend/internal/github/store.go +++ b/apps/backend/internal/github/store.go @@ -807,6 +807,21 @@ func (s *Store) ListReviewWatches(ctx context.Context, workspaceID string) ([]*R return watches, nil } +// ListAllReviewWatches returns every review watch across all workspaces. Used +// by the install-wide settings UI when no workspace filter is supplied. +func (s *Store) ListAllReviewWatches(ctx context.Context) ([]*ReviewWatch, error) { + var watches []*ReviewWatch + err := s.ro.SelectContext(ctx, &watches, + `SELECT * FROM github_review_watches ORDER BY workspace_id, created_at`) + if err != nil { + return nil, err + } + for _, w := range watches { + hydrateReviewWatchRepos(w) + } + return watches, nil +} + // ListEnabledReviewWatches returns all enabled review watches. func (s *Store) ListEnabledReviewWatches(ctx context.Context) ([]*ReviewWatch, error) { var watches []*ReviewWatch @@ -1131,6 +1146,20 @@ func (s *Store) ListIssueWatches(ctx context.Context, workspaceID string) ([]*Is return watches, nil } +// ListAllIssueWatches returns every issue watch across all workspaces. +func (s *Store) ListAllIssueWatches(ctx context.Context) ([]*IssueWatch, error) { + var watches []*IssueWatch + err := s.ro.SelectContext(ctx, &watches, + `SELECT * FROM github_issue_watches ORDER BY workspace_id, created_at`) + if err != nil { + return nil, err + } + for _, w := range watches { + hydrateIssueWatch(w) + } + return watches, nil +} + // ListEnabledIssueWatches returns all enabled issue watches. func (s *Store) ListEnabledIssueWatches(ctx context.Context) ([]*IssueWatch, error) { var watches []*IssueWatch diff --git a/apps/backend/internal/integrations/healthpoll/healthpoll.go b/apps/backend/internal/integrations/healthpoll/healthpoll.go index c5a853672e..b47c9f0b52 100644 --- a/apps/backend/internal/integrations/healthpoll/healthpoll.go +++ b/apps/backend/internal/integrations/healthpoll/healthpoll.go @@ -28,15 +28,15 @@ const DefaultInterval = 90 * time.Second // itself is single-threaded but Start may be invoked while a previous Stop is // still draining. type Prober interface { - // ListConfiguredWorkspaces returns the IDs of every workspace that has - // credentials stored for this integration. - ListConfiguredWorkspaces(ctx context.Context) ([]string, error) + // HasConfig reports whether the integration has stored credentials worth + // probing. The loop skips RecordAuthHealth when it returns false. + HasConfig(ctx context.Context) (bool, error) - // RecordAuthHealth probes one workspace's credentials and persists the - // result on the integration's config row. Errors are intentionally not - // returned: this is a best-effort health signal, never the source of - // truth for callers, so the implementation handles its own logging. - RecordAuthHealth(ctx context.Context, workspaceID string) + // RecordAuthHealth probes the stored credentials and persists the result + // on the integration's config row. Errors are intentionally not returned: + // this is a best-effort health signal, never the source of truth for + // callers, so the implementation handles its own logging. + RecordAuthHealth(ctx context.Context) } // Poller drives Prober.RecordAuthHealth on a fixed cadence. @@ -116,18 +116,19 @@ func (p *Poller) loop(ctx context.Context) { } } -// probeAll runs one probe pass across every configured workspace. Used by the -// loop and by in-package tests that want to drive a probe deterministically. +// probeAll runs one probe pass when the integration is configured. Used by +// the loop and by in-package tests that want to drive a probe deterministically. func (p *Poller) probeAll(ctx context.Context) { - ids, err := p.prober.ListConfiguredWorkspaces(ctx) + configured, err := p.prober.HasConfig(ctx) if err != nil { - p.logger.Warn(p.name+" poller: list workspaces failed", zap.Error(err)) + p.logger.Warn(p.name+" poller: has-config check failed", zap.Error(err)) return } - for _, id := range ids { - if ctx.Err() != nil { - return - } - p.prober.RecordAuthHealth(ctx, id) + if !configured { + return } + if ctx.Err() != nil { + return + } + p.prober.RecordAuthHealth(ctx) } diff --git a/apps/backend/internal/integrations/healthpoll/healthpoll_test.go b/apps/backend/internal/integrations/healthpoll/healthpoll_test.go index c697137361..40e9f6c52f 100644 --- a/apps/backend/internal/integrations/healthpoll/healthpoll_test.go +++ b/apps/backend/internal/integrations/healthpoll/healthpoll_test.go @@ -12,29 +12,27 @@ import ( ) type fakeProber struct { - mu sync.Mutex - listFn func(ctx context.Context) ([]string, error) - probed []string + mu sync.Mutex + hasFn func(ctx context.Context) (bool, error) + probedCount int } -func (f *fakeProber) ListConfiguredWorkspaces(ctx context.Context) ([]string, error) { - if f.listFn != nil { - return f.listFn(ctx) +func (f *fakeProber) HasConfig(ctx context.Context) (bool, error) { + if f.hasFn != nil { + return f.hasFn(ctx) } - return nil, nil + return false, nil } -func (f *fakeProber) RecordAuthHealth(_ context.Context, workspaceID string) { +func (f *fakeProber) RecordAuthHealth(_ context.Context) { f.mu.Lock() - f.probed = append(f.probed, workspaceID) + f.probedCount++ f.mu.Unlock() } -func TestProbeAll_RecordsEachWorkspace(t *testing.T) { +func TestProbeAll_RecordsWhenConfigured(t *testing.T) { p := &fakeProber{ - listFn: func(_ context.Context) ([]string, error) { - return []string{"ws-a", "ws-b"}, nil - }, + hasFn: func(_ context.Context) (bool, error) { return true, nil }, } poller := New("test", p, logger.Default()) @@ -42,16 +40,29 @@ func TestProbeAll_RecordsEachWorkspace(t *testing.T) { p.mu.Lock() defer p.mu.Unlock() - if len(p.probed) != 2 || p.probed[0] != "ws-a" || p.probed[1] != "ws-b" { - t.Errorf("expected [ws-a ws-b], got %v", p.probed) + if p.probedCount != 1 { + t.Errorf("expected 1 probe, got %d", p.probedCount) } } -func TestProbeAll_ListError_DoesNotPanic(t *testing.T) { +func TestProbeAll_SkipsWhenNotConfigured(t *testing.T) { p := &fakeProber{ - listFn: func(_ context.Context) ([]string, error) { - return nil, errors.New("db down") - }, + hasFn: func(_ context.Context) (bool, error) { return false, nil }, + } + poller := New("test", p, logger.Default()) + + poller.probeAll(context.Background()) + + p.mu.Lock() + defer p.mu.Unlock() + if p.probedCount != 0 { + t.Errorf("expected no probe when not configured, got %d", p.probedCount) + } +} + +func TestProbeAll_HasConfigError_DoesNotPanic(t *testing.T) { + p := &fakeProber{ + hasFn: func(_ context.Context) (bool, error) { return false, errors.New("db down") }, } poller := New("test", p, logger.Default()) @@ -59,16 +70,14 @@ func TestProbeAll_ListError_DoesNotPanic(t *testing.T) { p.mu.Lock() defer p.mu.Unlock() - if len(p.probed) != 0 { - t.Errorf("no probes should run on list error, got %v", p.probed) + if p.probedCount != 0 { + t.Errorf("no probe should run on has-config error, got %d", p.probedCount) } } func TestProbeAll_StopsWhenContextCancelled(t *testing.T) { p := &fakeProber{ - listFn: func(_ context.Context) ([]string, error) { - return []string{"ws-a", "ws-b", "ws-c"}, nil - }, + hasFn: func(_ context.Context) (bool, error) { return true, nil }, } poller := New("test", p, logger.Default()) @@ -78,17 +87,15 @@ func TestProbeAll_StopsWhenContextCancelled(t *testing.T) { p.mu.Lock() defer p.mu.Unlock() - if len(p.probed) != 0 { - t.Errorf("expected no probes after cancel, got %v", p.probed) + if p.probedCount != 0 { + t.Errorf("expected no probe after cancel, got %d", p.probedCount) } } func TestStart_RunsImmediateProbe(t *testing.T) { synctest.Test(t, func(t *testing.T) { p := &fakeProber{ - listFn: func(_ context.Context) ([]string, error) { - return []string{"ws-1"}, nil - }, + hasFn: func(_ context.Context) (bool, error) { return true, nil }, } poller := New("test", p, logger.Default()) @@ -103,7 +110,7 @@ func TestStart_RunsImmediateProbe(t *testing.T) { p.mu.Lock() defer p.mu.Unlock() - if len(p.probed) == 0 { + if p.probedCount == 0 { t.Error("Start did not run an immediate probe") } }) @@ -113,9 +120,9 @@ func TestStart_IsIdempotent(t *testing.T) { synctest.Test(t, func(t *testing.T) { var calls int32 p := &fakeProber{ - listFn: func(_ context.Context) ([]string, error) { + hasFn: func(_ context.Context) (bool, error) { atomic.AddInt32(&calls, 1) - return []string{"ws-1"}, nil + return true, nil }, } poller := New("test", p, logger.Default()) @@ -126,12 +133,12 @@ func TestStart_IsIdempotent(t *testing.T) { poller.Start(ctx) // second call must be a no-op // Wait for the immediate-on-Start probe pass to finish. If the - // second Start spawned a parallel loop we'd see two list calls. + // second Start spawned a parallel loop we'd see two has-config calls. synctest.Wait() poller.Stop() if got := atomic.LoadInt32(&calls); got != 1 { - t.Errorf("expected exactly 1 list call from initial probe, got %d", got) + t.Errorf("expected exactly 1 has-config call from initial probe, got %d", got) } }) } diff --git a/apps/backend/internal/jira/handlers.go b/apps/backend/internal/jira/handlers.go index 9efee14591..c0449dd87b 100644 --- a/apps/backend/internal/jira/handlers.go +++ b/apps/backend/internal/jira/handlers.go @@ -48,12 +48,7 @@ func (c *Controller) RegisterHTTPRoutes(router *gin.Engine) { // --- HTTP handlers --- func (c *Controller) httpGetConfig(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id required"}) - return - } - cfg, err := c.service.GetConfig(ctx.Request.Context(), workspaceID) + cfg, err := c.service.GetConfig(ctx.Request.Context()) if err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -84,12 +79,7 @@ func (c *Controller) httpSetConfig(ctx *gin.Context) { } func (c *Controller) httpDeleteConfig(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id required"}) - return - } - if err := c.service.DeleteConfig(ctx.Request.Context(), workspaceID); err != nil { + if err := c.service.DeleteConfig(ctx.Request.Context()); err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } @@ -111,12 +101,7 @@ func (c *Controller) httpTestConfig(ctx *gin.Context) { } func (c *Controller) httpListProjects(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id required"}) - return - } - projects, err := c.service.ListProjects(ctx.Request.Context(), workspaceID) + projects, err := c.service.ListProjects(ctx.Request.Context()) if err != nil { c.writeClientError(ctx, err) return @@ -125,15 +110,10 @@ func (c *Controller) httpListProjects(ctx *gin.Context) { } func (c *Controller) httpSearchTickets(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id required"}) - return - } jql := ctx.Query("jql") pageToken := ctx.Query("page_token") maxResults, _ := strconv.Atoi(ctx.Query("max_results")) - result, err := c.service.SearchTickets(ctx.Request.Context(), workspaceID, jql, pageToken, maxResults) + result, err := c.service.SearchTickets(ctx.Request.Context(), jql, pageToken, maxResults) if err != nil { c.writeClientError(ctx, err) return @@ -142,13 +122,8 @@ func (c *Controller) httpSearchTickets(ctx *gin.Context) { } func (c *Controller) httpGetTicket(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id required"}) - return - } key := ctx.Param("key") - ticket, err := c.service.GetTicket(ctx.Request.Context(), workspaceID, key) + ticket, err := c.service.GetTicket(ctx.Request.Context(), key) if err != nil { c.writeClientError(ctx, err) return @@ -157,11 +132,6 @@ func (c *Controller) httpGetTicket(ctx *gin.Context) { } func (c *Controller) httpDoTransition(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id required"}) - return - } key := ctx.Param("key") var req struct { TransitionID string `json:"transitionId"` @@ -170,7 +140,7 @@ func (c *Controller) httpDoTransition(ctx *gin.Context) { ctx.JSON(http.StatusBadRequest, gin.H{"error": "transitionId required"}) return } - if err := c.service.DoTransition(ctx.Request.Context(), workspaceID, key, req.TransitionID); err != nil { + if err := c.service.DoTransition(ctx.Request.Context(), key, req.TransitionID); err != nil { c.writeClientError(ctx, err) return } @@ -179,13 +149,21 @@ func (c *Controller) httpDoTransition(ctx *gin.Context) { // --- Issue watch HTTP handlers --- +// httpListIssueWatches returns watches scoped to one workspace when +// `workspace_id` is supplied, or every watch across all workspaces when it +// is absent. The integration settings page uses the unscoped form so the +// table can render a Workspace column without requiring an upfront pick. func (c *Controller) httpListIssueWatches(ctx *gin.Context) { workspaceID := ctx.Query("workspace_id") + var ( + watches []*IssueWatch + err error + ) if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id required"}) - return + watches, err = c.service.ListAllIssueWatches(ctx.Request.Context()) + } else { + watches, err = c.service.ListIssueWatches(ctx.Request.Context(), workspaceID) } - watches, err := c.service.ListIssueWatches(ctx.Request.Context(), workspaceID) if err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -304,9 +282,9 @@ func (c *Controller) writeIssueWatchError(ctx *gin.Context, err error) { c.writeClientError(ctx, err) } -// errCodeJiraNotConfigured is the wire-level code surfaced to the UI when the -// workspace has no saved Jira credentials. The same string is used by both the -// HTTP and WebSocket layers so the frontend can branch on a single value. +// errCodeJiraNotConfigured is the wire-level code surfaced to the UI when +// Jira has no saved credentials. The same string is used by both the HTTP +// and WebSocket layers so the frontend can branch on a single value. const errCodeJiraNotConfigured = "JIRA_NOT_CONFIGURED" // writeClientError maps service-level errors to HTTP responses. ErrNotConfigured @@ -315,7 +293,7 @@ const errCodeJiraNotConfigured = "JIRA_NOT_CONFIGURED" func (c *Controller) writeClientError(ctx *gin.Context, err error) { if errors.Is(err, ErrNotConfigured) { ctx.JSON(http.StatusServiceUnavailable, gin.H{ - "error": "Jira is not configured for this workspace", + "error": "Jira is not configured", "code": errCodeJiraNotConfigured, }) return @@ -370,13 +348,7 @@ func wsFail(msg *ws.Message, err error) (*ws.Message, error) { func wsGetConfig(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { - var p struct { - WorkspaceID string `json:"workspaceId"` - } - if err := msg.ParsePayload(&p); err != nil || p.WorkspaceID == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId required", nil) - } - cfg, err := svc.GetConfig(ctx, p.WorkspaceID) + cfg, err := svc.GetConfig(ctx) if err != nil { return wsFail(msg, err) } @@ -403,13 +375,7 @@ func wsSetConfig(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.M func wsDeleteConfig(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { - var p struct { - WorkspaceID string `json:"workspaceId"` - } - if err := msg.ParsePayload(&p); err != nil || p.WorkspaceID == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId required", nil) - } - if err := svc.DeleteConfig(ctx, p.WorkspaceID); err != nil { + if err := svc.DeleteConfig(ctx); err != nil { return wsFail(msg, err) } return wsReply(msg, gin.H{"deleted": true}) @@ -433,16 +399,15 @@ func wsTestConfig(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws. func wsGetTicket(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var p struct { - WorkspaceID string `json:"workspaceId"` - TicketKey string `json:"ticketKey"` + TicketKey string `json:"ticketKey"` } if err := msg.ParsePayload(&p); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload", nil) } - if p.WorkspaceID == "" || p.TicketKey == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId and ticketKey required", nil) + if p.TicketKey == "" { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "ticketKey required", nil) } - ticket, err := svc.GetTicket(ctx, p.WorkspaceID, p.TicketKey) + ticket, err := svc.GetTicket(ctx, p.TicketKey) if err != nil { return wsFail(msg, err) } @@ -453,17 +418,16 @@ func wsGetTicket(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.M func wsDoTransition(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var p struct { - WorkspaceID string `json:"workspaceId"` TicketKey string `json:"ticketKey"` TransitionID string `json:"transitionId"` } if err := msg.ParsePayload(&p); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload", nil) } - if p.WorkspaceID == "" || p.TicketKey == "" || p.TransitionID == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId, ticketKey, transitionId required", nil) + if p.TicketKey == "" || p.TransitionID == "" { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "ticketKey and transitionId required", nil) } - if err := svc.DoTransition(ctx, p.WorkspaceID, p.TicketKey, p.TransitionID); err != nil { + if err := svc.DoTransition(ctx, p.TicketKey, p.TransitionID); err != nil { return wsFail(msg, err) } return wsReply(msg, gin.H{"transitioned": true}) @@ -472,13 +436,7 @@ func wsDoTransition(svc *Service) func(ctx context.Context, msg *ws.Message) (*w func wsListProjects(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { - var p struct { - WorkspaceID string `json:"workspaceId"` - } - if err := msg.ParsePayload(&p); err != nil || p.WorkspaceID == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId required", nil) - } - projects, err := svc.ListProjects(ctx, p.WorkspaceID) + projects, err := svc.ListProjects(ctx) if err != nil { return wsFail(msg, err) } diff --git a/apps/backend/internal/jira/mock_controller.go b/apps/backend/internal/jira/mock_controller.go index 1f452104bc..fc9ee79e88 100644 --- a/apps/backend/internal/jira/mock_controller.go +++ b/apps/backend/internal/jira/mock_controller.go @@ -51,15 +51,14 @@ func (c *MockController) setAuthResult(ctx *gin.Context) { func (c *MockController) setAuthHealth(ctx *gin.Context) { var req struct { - WorkspaceID string `json:"workspaceId"` - OK bool `json:"ok"` - Error string `json:"error"` + OK bool `json:"ok"` + Error string `json:"error"` } - if err := ctx.ShouldBindJSON(&req); err != nil || req.WorkspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspaceId required"}) + if err := ctx.ShouldBindJSON(&req); err != nil { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"}) return } - if err := c.store.UpdateAuthHealth(ctx.Request.Context(), req.WorkspaceID, req.OK, req.Error, time.Now().UTC()); err != nil { + if err := c.store.UpdateAuthHealth(ctx.Request.Context(), req.OK, req.Error, time.Now().UTC()); err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } diff --git a/apps/backend/internal/jira/mock_controller_test.go b/apps/backend/internal/jira/mock_controller_test.go new file mode 100644 index 0000000000..c982f84293 --- /dev/null +++ b/apps/backend/internal/jira/mock_controller_test.go @@ -0,0 +1,75 @@ +package jira + +import ( + "bytes" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + + "github.com/kandev/kandev/internal/common/logger" +) + +// TestMockController_SetAuthHealth_PersistsWithoutWorkspaceID confirms that +// the mock auth-health route now accepts the install-wide payload (no +// workspaceId field) and forwards it to the singleton store, matching the +// production handler shape after the per-workspace → singleton refactor. +func TestMockController_SetAuthHealth_PersistsWithoutWorkspaceID(t *testing.T) { + store := newTestStore(t) + if err := store.UpsertConfig(t.Context(), &JiraConfig{ + SiteURL: "https://acme.atlassian.net", Email: "u@example.com", AuthMethod: AuthMethodAPIToken, + }); err != nil { + t.Fatalf("seed config: %v", err) + } + mock := NewMockClient() + ctrl := NewMockController(mock, store, logger.Default()) + + gin.SetMode(gin.TestMode) + router := gin.New() + ctrl.RegisterRoutes(router) + + req := httptest.NewRequest(http.MethodPut, "/api/v1/jira/mock/auth-health", + bytes.NewBufferString(`{"ok": false, "error": "session expired"}`)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String()) + } + cfg, err := store.GetConfig(t.Context()) + if err != nil { + t.Fatalf("get config: %v", err) + } + if cfg.LastOk { + t.Error("expected LastOk=false after auth-health write") + } + if cfg.LastError != "session expired" { + t.Errorf("expected error preserved, got %q", cfg.LastError) + } + if cfg.LastCheckedAt == nil { + t.Error("expected LastCheckedAt set") + } +} + +// TestMockController_SetAuthHealth_RejectsInvalidJSON guards the bind step: +// junk in the body should fail fast with a 400 rather than write garbage to +// the store. +func TestMockController_SetAuthHealth_RejectsInvalidJSON(t *testing.T) { + store := newTestStore(t) + mock := NewMockClient() + ctrl := NewMockController(mock, store, logger.Default()) + + gin.SetMode(gin.TestMode) + router := gin.New() + ctrl.RegisterRoutes(router) + + req := httptest.NewRequest(http.MethodPut, "/api/v1/jira/mock/auth-health", + bytes.NewBufferString(`not json`)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", w.Code) + } +} diff --git a/apps/backend/internal/jira/models.go b/apps/backend/internal/jira/models.go index 798fb3c79f..87cca025ee 100644 --- a/apps/backend/internal/jira/models.go +++ b/apps/backend/internal/jira/models.go @@ -1,6 +1,7 @@ -// Package jira implements the Jira/Atlassian Cloud integration: workspace-scoped -// configuration storage, a REST client for tickets and transitions, and the HTTP -// and WebSocket handlers that expose these capabilities to the frontend. +// Package jira implements the Jira/Atlassian Cloud integration: a single +// install-wide configuration plus per-workspace JQL issue watchers, a REST +// client for tickets and transitions, and the HTTP and WebSocket handlers that +// expose these capabilities to the frontend. package jira import "time" @@ -11,11 +12,10 @@ const ( AuthMethodSessionCookie = "session_cookie" ) -// JiraConfig is the workspace-scoped configuration for the Jira integration. -// The secret value (API token or session cookie) is stored separately in the -// encrypted secret store under the key returned by SecretKeyForWorkspace. +// JiraConfig is the install-wide configuration for the Jira integration. The +// secret value (API token or session cookie) is stored separately in the +// encrypted secret store under SecretKey. type JiraConfig struct { - WorkspaceID string `json:"workspaceId" db:"workspace_id"` SiteURL string `json:"siteUrl" db:"site_url"` Email string `json:"email" db:"email"` AuthMethod string `json:"authMethod" db:"auth_method"` @@ -35,11 +35,10 @@ type JiraConfig struct { UpdatedAt time.Time `json:"updatedAt" db:"updated_at"` } -// SetConfigRequest is the payload sent by the UI to create or update the -// workspace's Jira configuration. When Secret is empty on update, the existing -// secret is retained; when non-empty it replaces the stored value. +// SetConfigRequest is the payload sent by the UI to create or update the Jira +// configuration. When Secret is empty on update, the existing secret is +// retained; when non-empty it replaces the stored value. type SetConfigRequest struct { - WorkspaceID string `json:"workspaceId"` SiteURL string `json:"siteUrl"` Email string `json:"email"` AuthMethod string `json:"authMethod"` @@ -113,9 +112,14 @@ type SearchResult struct { NextPageToken string `json:"nextPageToken,omitempty"` } -// SecretKeyForWorkspace returns the secret-store key used for the Jira token of -// a given workspace. Centralised so that the service and the store agree. -func SecretKeyForWorkspace(workspaceID string) string { +// SecretKey is the secret-store key used for the install-wide Jira token. +// Centralised so that the service, store and provider migration agree. +const SecretKey = "jira:singleton:token" + +// LegacySecretKeyForWorkspace returns the pre-singleton per-workspace secret +// key. Only used by the one-shot startup migration in provider.go to copy an +// existing token over to SecretKey. +func LegacySecretKeyForWorkspace(workspaceID string) string { return "jira:" + workspaceID + ":token" } diff --git a/apps/backend/internal/jira/poller.go b/apps/backend/internal/jira/poller.go index c9ca7a3592..35d333c34c 100644 --- a/apps/backend/internal/jira/poller.go +++ b/apps/backend/internal/jira/poller.go @@ -186,10 +186,10 @@ func (p *Poller) fireIssueTickHook() { // interface into Service's public API. type svcProber struct{ svc *Service } -func (s svcProber) ListConfiguredWorkspaces(ctx context.Context) ([]string, error) { - return s.svc.Store().ListConfiguredWorkspaces(ctx) +func (s svcProber) HasConfig(ctx context.Context) (bool, error) { + return s.svc.Store().HasConfig(ctx) } -func (s svcProber) RecordAuthHealth(ctx context.Context, workspaceID string) { - s.svc.RecordAuthHealth(ctx, workspaceID) +func (s svcProber) RecordAuthHealth(ctx context.Context) { + s.svc.RecordAuthHealth(ctx) } diff --git a/apps/backend/internal/jira/poller_issue_watch_test.go b/apps/backend/internal/jira/poller_issue_watch_test.go index 5489b68996..78eea761b6 100644 --- a/apps/backend/internal/jira/poller_issue_watch_test.go +++ b/apps/backend/internal/jira/poller_issue_watch_test.go @@ -49,7 +49,7 @@ func (r *recordingSubscriber) snapshot() []*NewJiraIssueEvent { func TestPoller_CheckIssueWatches_PublishesNewTicketsOnly(t *testing.T) { f := newPollerFixture(t) ctx := context.Background() - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") eb := bus.NewMemoryEventBus(logger.Default()) defer eb.Close() @@ -108,7 +108,7 @@ func TestPoller_CheckIssueWatches_PublishesNewTicketsOnly(t *testing.T) { func TestPoller_CheckIssueWatches_SkipsDisabled(t *testing.T) { f := newPollerFixture(t) ctx := context.Background() - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") eb := bus.NewMemoryEventBus(logger.Default()) defer eb.Close() @@ -183,7 +183,7 @@ func TestIsIssueWatchDue(t *testing.T) { func TestPoller_CheckIssueWatches_RespectsPerWatchInterval(t *testing.T) { f := newPollerFixture(t) ctx := context.Background() - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") eb := bus.NewMemoryEventBus(logger.Default()) defer eb.Close() diff --git a/apps/backend/internal/jira/poller_test.go b/apps/backend/internal/jira/poller_test.go index ab098d58ce..2ebbc5fc81 100644 --- a/apps/backend/internal/jira/poller_test.go +++ b/apps/backend/internal/jira/poller_test.go @@ -36,37 +36,35 @@ func newPollerFixture(t *testing.T) *pollerFixture { return f } -// saveConfig persists a workspace config directly via the store + secret +// saveConfig persists the singleton config directly via the store + secret // fakes. We deliberately avoid Service.SetConfig here because it fires an // async auth probe in a goroutine — fine for production but it would race // against the deterministic RecordAuthHealth calls these tests make. -func (f *pollerFixture) saveConfig(t *testing.T, workspaceID, secret string) { +func (f *pollerFixture) saveConfig(t *testing.T, secret string) { t.Helper() ctx := context.Background() if err := f.store.UpsertConfig(ctx, &JiraConfig{ - WorkspaceID: workspaceID, - SiteURL: "https://" + workspaceID + ".atlassian.net", - Email: workspaceID + "@example.com", - AuthMethod: AuthMethodAPIToken, + SiteURL: "https://acme.atlassian.net", + Email: "user@example.com", + AuthMethod: AuthMethodAPIToken, }); err != nil { - t.Fatalf("save config %s: %v", workspaceID, err) + t.Fatalf("save config: %v", err) } - if err := f.secrets.Set(ctx, SecretKeyForWorkspace(workspaceID), - "jira", secret); err != nil { - t.Fatalf("save secret %s: %v", workspaceID, err) + if err := f.secrets.Set(ctx, SecretKey, "jira", secret); err != nil { + t.Fatalf("save secret: %v", err) } } func TestService_RecordAuthHealth_RecordsSuccess(t *testing.T) { f := newPollerFixture(t) - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") f.client.testAuthFn = func() (*TestConnectionResult, error) { return &TestConnectionResult{OK: true, DisplayName: "Alice"}, nil } - f.svc.RecordAuthHealth(context.Background(), "ws-1") + f.svc.RecordAuthHealth(context.Background()) - cfg, _ := f.store.GetConfig(context.Background(), "ws-1") + cfg, _ := f.store.GetConfig(context.Background()) if cfg == nil { t.Fatal("config disappeared") } @@ -83,16 +81,16 @@ func TestService_RecordAuthHealth_RecordsSuccess(t *testing.T) { func TestService_RecordAuthHealth_RecordsFailure(t *testing.T) { f := newPollerFixture(t) - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") f.client.testAuthFn = func() (*TestConnectionResult, error) { // Service convention: TestAuth returns the failure inside the result // rather than as an error, so the UI can render the message inline. return &TestConnectionResult{OK: false, Error: "step-up auth required"}, nil } - f.svc.RecordAuthHealth(context.Background(), "ws-1") + f.svc.RecordAuthHealth(context.Background()) - cfg, _ := f.store.GetConfig(context.Background(), "ws-1") + cfg, _ := f.store.GetConfig(context.Background()) if cfg.LastOk { t.Error("expected LastOk=false after failed probe") } @@ -103,14 +101,14 @@ func TestService_RecordAuthHealth_RecordsFailure(t *testing.T) { func TestService_RecordAuthHealth_ClientError(t *testing.T) { f := newPollerFixture(t) - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") f.client.testAuthFn = func() (*TestConnectionResult, error) { return nil, errors.New("network timeout") } - f.svc.RecordAuthHealth(context.Background(), "ws-1") + f.svc.RecordAuthHealth(context.Background()) - cfg, _ := f.store.GetConfig(context.Background(), "ws-1") + cfg, _ := f.store.GetConfig(context.Background()) if cfg.LastOk { t.Error("expected LastOk=false on client error") } @@ -119,22 +117,20 @@ func TestService_RecordAuthHealth_ClientError(t *testing.T) { } } -func TestPoller_Start_ProbesEachConfiguredWorkspace(t *testing.T) { - // Smoke test: confirms the prober adapter actually wires - // Service.Store().ListConfiguredWorkspaces → Service.RecordAuthHealth - // when the loop is started, end-to-end. Wrapped in synctest so the - // goroutine the poller spawns runs deterministically against fake time - // instead of relying on a wall-clock deadline. +func TestPoller_Start_ProbesWhenConfigured(t *testing.T) { + // Smoke test: confirms the prober adapter actually wires HasConfig → + // Service.RecordAuthHealth when the loop is started, end-to-end. Wrapped + // in synctest so the goroutine the poller spawns runs deterministically + // against fake time instead of relying on a wall-clock deadline. synctest.Test(t, func(t *testing.T) { f := newPollerFixture(t) - f.saveConfig(t, "ws-a", "tok-a") - f.saveConfig(t, "ws-b", "tok-b") - probed := map[string]bool{} + f.saveConfig(t, "tok") + var probed bool f.client.testAuthFn = func() (*TestConnectionResult, error) { return &TestConnectionResult{OK: true}, nil } - f.svc.SetProbeHook(func(workspaceID string) { - probed[workspaceID] = true + f.svc.SetProbeHook(func() { + probed = true }) ctx, cancel := context.WithCancel(context.Background()) @@ -145,8 +141,8 @@ func TestPoller_Start_ProbesEachConfiguredWorkspace(t *testing.T) { // Wait for the immediate-on-Start probe pass to finish. synctest.Wait() - if !probed["ws-a"] || !probed["ws-b"] { - t.Errorf("expected both ws-a and ws-b probed, got %v", probed) + if !probed { + t.Errorf("expected probe to fire when configured") } }) } diff --git a/apps/backend/internal/jira/provider.go b/apps/backend/internal/jira/provider.go index 74447aae05..d2785fcd71 100644 --- a/apps/backend/internal/jira/provider.go +++ b/apps/backend/internal/jira/provider.go @@ -1,10 +1,13 @@ package jira import ( + "context" "os" + "time" "github.com/gin-gonic/gin" "github.com/jmoiron/sqlx" + "go.uber.org/zap" "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/events/bus" @@ -34,6 +37,7 @@ func Provide(writer, reader *sqlx.DB, secrets SecretStore, eventBus bus.EventBus if err != nil { return nil, nil, err } + migrateLegacySecret(store, secrets, log) clientFn := DefaultClientFactory var mock *MockClient if MockEnabled() { @@ -50,6 +54,34 @@ func Provide(writer, reader *sqlx.DB, secrets SecretStore, eventBus bus.EventBus return svc, cleanup, nil } +// migrateLegacySecret copies the per-workspace token from the pre-singleton +// secret key onto the new install-wide key, then deletes the legacy entry. +// Best-effort: any error is logged and ignored — the worst case is the user +// re-pastes their token in the settings UI. +func migrateLegacySecret(store *Store, secrets SecretStore, log *logger.Logger) { + source := store.MigratedFromWorkspace() + if source == "" || secrets == nil { + return + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if exists, err := secrets.Exists(ctx, SecretKey); err == nil && exists { + return + } + legacyKey := LegacySecretKeyForWorkspace(source) + value, err := secrets.Reveal(ctx, legacyKey) + if err != nil || value == "" { + return + } + if err := secrets.Set(ctx, SecretKey, "Jira token", value); err != nil { + log.Warn("jira: legacy secret migration failed", zap.Error(err)) + return + } + if err := secrets.Delete(ctx, legacyKey); err != nil { + log.Warn("jira: legacy secret cleanup failed", zap.Error(err)) + } +} + // RegisterMockRoutes mounts the mock control routes when the service was built // with a MockClient. No-op otherwise — production builds skip this entirely. func RegisterMockRoutes(router *gin.Engine, svc *Service, log *logger.Logger) { diff --git a/apps/backend/internal/jira/service.go b/apps/backend/internal/jira/service.go index e54ce1431a..87b4fca146 100644 --- a/apps/backend/internal/jira/service.go +++ b/apps/backend/internal/jira/service.go @@ -24,16 +24,16 @@ type SecretStore interface { Exists(ctx context.Context, id string) (bool, error) } -// Service orchestrates Jira config storage, the per-workspace client cache, and -// the fetch/transition operations used by the WebSocket + HTTP handlers. +// Service orchestrates Jira config storage, the cached client, and the +// fetch/transition operations used by the WebSocket + HTTP handlers. type Service struct { store *Store secrets SecretStore log *logger.Logger mu sync.Mutex clientFn ClientFactory - cache map[string]Client // workspaceID → client, cleared on config change. - probeHook func(workspaceID string) + client Client // singleton, cleared on config change. + probeHook func() eventBus bus.EventBus // 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 @@ -67,34 +67,30 @@ func NewService(store *Store, secrets SecretStore, clientFn ClientFactory, log * secrets: secrets, log: log, clientFn: clientFn, - cache: make(map[string]Client), } } -// GetConfig returns the workspace config enriched with a HasSecret flag so the +// GetConfig returns the singleton config enriched with a HasSecret flag so the // UI can distinguish "configured but empty" from "needs credentials". For // session_cookie auth, it also tries to decode the JWT in the stored cookie // and surface its expiry so the UI can warn the user before the session dies. -func (s *Service) GetConfig(ctx context.Context, workspaceID string) (*JiraConfig, error) { - cfg, err := s.store.GetConfig(ctx, workspaceID) +func (s *Service) GetConfig(ctx context.Context) (*JiraConfig, error) { + cfg, err := s.store.GetConfig(ctx) if err != nil || cfg == nil { return cfg, err } if s.secrets == nil { return cfg, nil } - key := SecretKeyForWorkspace(workspaceID) - exists, existsErr := s.secrets.Exists(ctx, key) + exists, existsErr := s.secrets.Exists(ctx, SecretKey) if existsErr != nil { - s.log.Warn("jira: secret exists check failed", - zap.String("workspace_id", workspaceID), zap.Error(existsErr)) + s.log.Warn("jira: secret exists check failed", zap.Error(existsErr)) } cfg.HasSecret = exists if exists && cfg.AuthMethod == AuthMethodSessionCookie { - secret, revealErr := s.secrets.Reveal(ctx, key) + secret, revealErr := s.secrets.Reveal(ctx, SecretKey) if revealErr != nil { - s.log.Warn("jira: secret reveal failed", - zap.String("workspace_id", workspaceID), zap.Error(revealErr)) + s.log.Warn("jira: secret reveal failed", zap.Error(revealErr)) } else { cfg.SecretExpiresAt = parseSessionCookieExpiry(secret) } @@ -103,10 +99,10 @@ func (s *Service) GetConfig(ctx context.Context, workspaceID string) (*JiraConfi } // ErrInvalidConfig is returned by SetConfig when the request fails validation -// (missing workspace, bad auth method, etc.). Callers map it to HTTP 400. +// (missing site URL, bad auth method, etc.). Callers map it to HTTP 400. var ErrInvalidConfig = errors.New("jira: invalid configuration") -// SetConfig is upsert: it writes the workspace row and, when a new secret is +// SetConfig is upsert: it writes the singleton row and, when a new secret is // provided, stores it in the encrypted secret store. An empty Secret means // "keep the existing token" — this lets the UI edit auxiliary fields without // forcing the user to paste the token again. @@ -115,7 +111,6 @@ func (s *Service) SetConfig(ctx context.Context, req *SetConfigRequest) (*JiraCo return nil, fmt.Errorf("%w: %s", ErrInvalidConfig, err.Error()) } cfg := &JiraConfig{ - WorkspaceID: req.WorkspaceID, SiteURL: normalizeSiteURL(req.SiteURL), Email: req.Email, AuthMethod: req.AuthMethod, @@ -125,40 +120,35 @@ func (s *Service) SetConfig(ctx context.Context, req *SetConfigRequest) (*JiraCo return nil, fmt.Errorf("upsert jira config: %w", err) } if req.Secret != "" && s.secrets != nil { - if err := s.secrets.Set(ctx, - SecretKeyForWorkspace(req.WorkspaceID), - "Jira token ("+req.WorkspaceID+")", - req.Secret, - ); err != nil { + if err := s.secrets.Set(ctx, SecretKey, "Jira token", req.Secret); err != nil { return nil, fmt.Errorf("store jira secret: %w", err) } } - s.invalidateClient(req.WorkspaceID) + s.invalidateClient() // Probe asynchronously so a slow Atlassian doesn't stall the save response. // RecordAuthHealth manages its own probe timeout, so a fresh // context.Background() is enough — the request ctx may be cancelled when // this returns, but the probe and the DB write must still complete. - go func(workspaceID string) { - s.RecordAuthHealth(context.Background(), workspaceID) - }(req.WorkspaceID) - return s.GetConfig(ctx, req.WorkspaceID) + go func() { + s.RecordAuthHealth(context.Background()) + }() + return s.GetConfig(ctx) } // DeleteConfig removes both the config row and the stored secret. A failure to // delete the secret is logged (not returned) so the config row deletion still // surfaces success — the orphaned secret is rare and the user can retry by // re-saving + deleting again. -func (s *Service) DeleteConfig(ctx context.Context, workspaceID string) error { - if err := s.store.DeleteConfig(ctx, workspaceID); err != nil { +func (s *Service) DeleteConfig(ctx context.Context) error { + if err := s.store.DeleteConfig(ctx); err != nil { return err } if s.secrets != nil { - if err := s.secrets.Delete(ctx, SecretKeyForWorkspace(workspaceID)); err != nil { - s.log.Warn("jira: secret delete failed", - zap.String("workspace_id", workspaceID), zap.Error(err)) + if err := s.secrets.Delete(ctx, SecretKey); err != nil { + s.log.Warn("jira: secret delete failed", zap.Error(err)) } } - s.invalidateClient(workspaceID) + s.invalidateClient() return nil } @@ -175,13 +165,13 @@ func (s *Service) TestConnection(ctx context.Context, req *SetConfigRequest) (*T return client.TestAuth(ctx) } -// ProbeAuth validates the stored credentials for a workspace by hitting the -// cheapest authenticated endpoint. Used by the background auth-health poller -// to detect expired session cookies / step-up auth before the user clicks -// through to a tab that would 303. Errors are returned as a result so the -// poller can persist them as a failure rather than dropping them. -func (s *Service) ProbeAuth(ctx context.Context, workspaceID string) (*TestConnectionResult, error) { - client, err := s.clientFor(ctx, workspaceID) +// ProbeAuth validates the stored credentials by hitting the cheapest +// authenticated endpoint. Used by the background auth-health poller to detect +// expired session cookies / step-up auth before the user clicks through to a +// tab that would 303. Errors are returned as a result so the poller can +// persist them as a failure rather than dropping them. +func (s *Service) ProbeAuth(ctx context.Context) (*TestConnectionResult, error) { + client, err := s.clientFor(ctx) if err != nil { return &TestConnectionResult{OK: false, Error: err.Error()}, nil } @@ -195,7 +185,7 @@ func (s *Service) Store() *Store { return s.store } -// authProbeTimeout caps a single auth-health probe so a slow workspace can't +// authProbeTimeout caps a single auth-health probe so a slow Atlassian can't // stall the caller. The /myself endpoint typically responds in <500ms. const authProbeTimeout = 15 * time.Second @@ -207,21 +197,21 @@ const authHealthWriteTimeout = 5 * time.Second // SetProbeHook installs a callback fired at the end of each RecordAuthHealth // call (after the DB write). Production code never sets this; tests use it to // synchronize on probe completion without sleep-polling. -func (s *Service) SetProbeHook(fn func(workspaceID string)) { +func (s *Service) SetProbeHook(fn func()) { s.mu.Lock() defer s.mu.Unlock() s.probeHook = fn } -// RecordAuthHealth probes the workspace's stored credentials and writes the -// outcome onto the JiraConfig row. Used both at config-save time (so the UI -// reflects the new credentials immediately) and by the background poller. -// Errors during the persist step are logged but not returned: this is a -// best-effort health signal, never the source of truth for callers. -func (s *Service) RecordAuthHealth(ctx context.Context, workspaceID string) { +// RecordAuthHealth probes the stored credentials and writes the outcome onto +// the JiraConfig row. Used both at config-save time (so the UI reflects the +// new credentials immediately) and by the background poller. Errors during +// the persist step are logged but not returned: this is a best-effort health +// signal, never the source of truth for callers. +func (s *Service) RecordAuthHealth(ctx context.Context) { probeCtx, cancel := context.WithTimeout(ctx, authProbeTimeout) defer cancel() - res, err := s.ProbeAuth(probeCtx, workspaceID) + res, err := s.ProbeAuth(probeCtx) ok := err == nil && res != nil && res.OK errMsg := "" switch { @@ -236,22 +226,20 @@ func (s *Service) RecordAuthHealth(ctx context.Context, workspaceID string) { // failed" until the next poll). writeCtx, writeCancel := context.WithTimeout(context.Background(), authHealthWriteTimeout) defer writeCancel() - if updateErr := s.store.UpdateAuthHealth(writeCtx, workspaceID, ok, errMsg, time.Now().UTC()); updateErr != nil { - s.log.Warn("jira: update auth health failed", - zap.String("workspace_id", workspaceID), zap.Error(updateErr)) + if updateErr := s.store.UpdateAuthHealth(writeCtx, ok, errMsg, time.Now().UTC()); updateErr != nil { + s.log.Warn("jira: update auth health failed", zap.Error(updateErr)) } s.mu.Lock() hook := s.probeHook s.mu.Unlock() if hook != nil { - hook(workspaceID) + hook() } } -// GetTicket loads a Jira ticket by key using the workspace's stored -// credentials. -func (s *Service) GetTicket(ctx context.Context, workspaceID, ticketKey string) (*JiraTicket, error) { - client, err := s.clientFor(ctx, workspaceID) +// GetTicket loads a Jira ticket by key using the stored credentials. +func (s *Service) GetTicket(ctx context.Context, ticketKey string) (*JiraTicket, error) { + client, err := s.clientFor(ctx) if err != nil { return nil, err } @@ -259,8 +247,8 @@ func (s *Service) GetTicket(ctx context.Context, workspaceID, ticketKey string) } // DoTransition applies a transition to a ticket. -func (s *Service) DoTransition(ctx context.Context, workspaceID, ticketKey, transitionID string) error { - client, err := s.clientFor(ctx, workspaceID) +func (s *Service) DoTransition(ctx context.Context, ticketKey, transitionID string) error { + client, err := s.clientFor(ctx) if err != nil { return err } @@ -268,36 +256,37 @@ func (s *Service) DoTransition(ctx context.Context, workspaceID, ticketKey, tran } // ListProjects is used by the settings UI to populate the project selector. -func (s *Service) ListProjects(ctx context.Context, workspaceID string) ([]JiraProject, error) { - client, err := s.clientFor(ctx, workspaceID) +func (s *Service) ListProjects(ctx context.Context) ([]JiraProject, error) { + client, err := s.clientFor(ctx) if err != nil { return nil, err } return client.ListProjects(ctx) } -// SearchTickets runs a JQL search for the workspace, returning a page of -// tickets. pageToken is the cursor returned in the previous page's -// NextPageToken; pass "" for the first page. -func (s *Service) SearchTickets(ctx context.Context, workspaceID, jql, pageToken string, maxResults int) (*SearchResult, error) { - client, err := s.clientFor(ctx, workspaceID) +// SearchTickets runs a JQL search, returning a page of tickets. pageToken is +// the cursor returned in the previous page's NextPageToken; pass "" for the +// first page. +func (s *Service) SearchTickets(ctx context.Context, jql, pageToken string, maxResults int) (*SearchResult, error) { + client, err := s.clientFor(ctx) if err != nil { return nil, err } return client.SearchTickets(ctx, jql, pageToken, maxResults) } -// clientFor returns a cached client, creating one if needed. The cache is +// clientFor returns the cached client, creating it if needed. The cache is // invalidated whenever the config changes so stale credentials never linger. -func (s *Service) clientFor(ctx context.Context, workspaceID string) (Client, error) { +func (s *Service) clientFor(ctx context.Context) (Client, error) { s.mu.Lock() - if c, ok := s.cache[workspaceID]; ok { + if s.client != nil { + c := s.client s.mu.Unlock() return c, nil } s.mu.Unlock() - cfg, err := s.store.GetConfig(ctx, workspaceID) + cfg, err := s.store.GetConfig(ctx) if err != nil { return nil, err } @@ -306,7 +295,7 @@ func (s *Service) clientFor(ctx context.Context, workspaceID string) (Client, er } secret := "" if s.secrets != nil { - secret, err = s.secrets.Reveal(ctx, SecretKeyForWorkspace(workspaceID)) + secret, err = s.secrets.Reveal(ctx, SecretKey) if err != nil { // Don't conflate a transient secret-store failure with "user never // configured Jira". The UI gates on ErrNotConfigured to render a @@ -324,18 +313,18 @@ func (s *Service) clientFor(ctx context.Context, workspaceID string) (Client, er // Re-check: another caller may have populated the cache while we were // fetching the config and secret. Returning the existing client keeps the // cache identity stable so callers comparing pointers don't see flapping. - if existing, ok := s.cache[workspaceID]; ok { - return existing, nil + if s.client != nil { + return s.client, nil } - s.cache[workspaceID] = client + s.client = client return client, nil } -// invalidateClient drops a cached client so the next request rebuilds it with -// fresh credentials. -func (s *Service) invalidateClient(workspaceID string) { +// invalidateClient drops the cached client so the next request rebuilds it +// with fresh credentials. +func (s *Service) invalidateClient() { s.mu.Lock() - delete(s.cache, workspaceID) + s.client = nil s.mu.Unlock() } @@ -344,10 +333,9 @@ func (s *Service) invalidateClient(workspaceID string) { // (post-save re-test). func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) (*JiraConfig, string, error) { cfg := &JiraConfig{ - WorkspaceID: req.WorkspaceID, - SiteURL: normalizeSiteURL(req.SiteURL), - Email: req.Email, - AuthMethod: req.AuthMethod, + SiteURL: normalizeSiteURL(req.SiteURL), + Email: req.Email, + AuthMethod: req.AuthMethod, } if req.Secret != "" { return cfg, req.Secret, nil @@ -355,13 +343,12 @@ func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) if s.secrets == nil { return nil, "", errors.New("no secret store configured") } - secret, err := s.secrets.Reveal(ctx, SecretKeyForWorkspace(req.WorkspaceID)) + secret, err := s.secrets.Reveal(ctx, SecretKey) if err != nil { // Don't conflate a transient secret-store failure with "no token // stored". Surfacing the real error gives the user a path to retry // instead of telling them to paste a token they already have. - s.log.Warn("jira: secret reveal failed", - zap.String("workspace_id", req.WorkspaceID), zap.Error(err)) + s.log.Warn("jira: secret reveal failed", zap.Error(err)) return nil, "", fmt.Errorf("read jira secret: %w", err) } if secret == "" { @@ -369,7 +356,7 @@ func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) } // Merge with persisted config so the test uses saved site/email if the // caller only passed a partial request. - if stored, _ := s.store.GetConfig(ctx, req.WorkspaceID); stored != nil { + if stored, _ := s.store.GetConfig(ctx); stored != nil { if cfg.SiteURL == "" { cfg.SiteURL = stored.SiteURL } @@ -384,9 +371,6 @@ func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) } func validateConfigRequest(req *SetConfigRequest) error { - if req.WorkspaceID == "" { - return errors.New("workspaceId required") - } if req.SiteURL == "" { return errors.New("siteUrl required") } @@ -463,6 +447,11 @@ func (s *Service) ListIssueWatches(ctx context.Context, workspaceID string) ([]* return s.store.ListIssueWatches(ctx, workspaceID) } +// ListAllIssueWatches returns every watch across all workspaces. +func (s *Service) ListAllIssueWatches(ctx context.Context) ([]*IssueWatch, error) { + return s.store.ListAllIssueWatches(ctx) +} + // GetIssueWatch returns a single watch by ID or ErrIssueWatchNotFound. func (s *Service) GetIssueWatch(ctx context.Context, id string) (*IssueWatch, error) { w, err := s.store.GetIssueWatch(ctx, id) @@ -514,7 +503,7 @@ func (s *Service) DeleteIssueWatch(ctx context.Context, id string) error { // so the UI can show liveness. func (s *Service) CheckIssueWatch(ctx context.Context, w *IssueWatch) ([]*JiraTicket, error) { defer s.stampLastPolled(w.ID) - client, err := s.clientFor(ctx, w.WorkspaceID) + client, err := s.clientFor(ctx) if err != nil { return nil, err } diff --git a/apps/backend/internal/jira/service_issue_watch_test.go b/apps/backend/internal/jira/service_issue_watch_test.go index 0babe28e66..95f81909fd 100644 --- a/apps/backend/internal/jira/service_issue_watch_test.go +++ b/apps/backend/internal/jira/service_issue_watch_test.go @@ -148,7 +148,7 @@ func TestService_CheckIssueWatch_FiltersAlreadySeen(t *testing.T) { // Configure JIRA so clientFor succeeds. if _, err := f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "tok", }); err != nil { t.Fatalf("set config: %v", err) @@ -198,7 +198,7 @@ func TestService_CheckIssueWatch_StampsLastPolledOnError(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() if _, err := f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "tok", }); err != nil { t.Fatalf("set config: %v", err) diff --git a/apps/backend/internal/jira/service_test.go b/apps/backend/internal/jira/service_test.go index 1f076e4b7e..dc47394604 100644 --- a/apps/backend/internal/jira/service_test.go +++ b/apps/backend/internal/jira/service_test.go @@ -105,7 +105,7 @@ type svcFixture struct { // atomic so the async probe goroutine spawned by SetConfig can call // clientFor (and the injected factory) without racing the test thread. factoryHit atomic.Int32 - probed chan string + probed chan struct{} } func newSvcFixture(t *testing.T) *svcFixture { @@ -114,15 +114,15 @@ func newSvcFixture(t *testing.T) *svcFixture { store: newTestStore(t), secrets: newFakeSecretStore(), client: &fakeClient{}, - probed: make(chan string, 8), + probed: make(chan struct{}, 8), } f.svc = NewService(f.store, f.secrets, func(_ *JiraConfig, _ string) Client { f.factoryHit.Add(1) return f.client }, logger.Default()) - f.svc.SetProbeHook(func(workspaceID string) { + f.svc.SetProbeHook(func() { select { - case f.probed <- workspaceID: + case f.probed <- struct{}{}: default: } }) @@ -134,11 +134,10 @@ func TestService_SetConfig_UpsertsAndStoresSecret(t *testing.T) { ctx := context.Background() cfg, err := f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", - SiteURL: "https://acme.atlassian.net/", - Email: "u@example.com", - AuthMethod: AuthMethodAPIToken, - Secret: "tok1", + SiteURL: "https://acme.atlassian.net/", + Email: "u@example.com", + AuthMethod: AuthMethodAPIToken, + Secret: "tok1", }) if err != nil { t.Fatalf("set: %v", err) @@ -149,7 +148,7 @@ func TestService_SetConfig_UpsertsAndStoresSecret(t *testing.T) { if !cfg.HasSecret { t.Error("expected HasSecret=true") } - if got, _ := f.secrets.Reveal(ctx, SecretKeyForWorkspace("ws-1")); got != "tok1" { + if got, _ := f.secrets.Reveal(ctx, SecretKey); got != "tok1" { t.Errorf("secret stored = %q", got) } } @@ -160,12 +159,12 @@ func TestService_SetConfig_ProbesAuthImmediately(t *testing.T) { return &TestConnectionResult{OK: true, DisplayName: "Alice"}, nil } if _, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "tok", }); err != nil { t.Fatalf("set: %v", err) } - cfg := waitForAuthProbe(t, f, "ws-1") + cfg := waitForAuthProbe(t, f) if !cfg.LastOk { t.Errorf("expected LastOk=true after async probe, got %+v", cfg) } @@ -180,12 +179,12 @@ func TestService_SetConfig_PersistsProbeFailure(t *testing.T) { return &TestConnectionResult{OK: false, Error: "401 unauthorized"}, nil } if _, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "bad", }); err != nil { t.Fatalf("set: %v", err) } - cfg := waitForAuthProbe(t, f, "ws-1") + cfg := waitForAuthProbe(t, f) if cfg.LastOk { t.Error("expected LastOk=false after failed probe") } @@ -197,23 +196,18 @@ func TestService_SetConfig_PersistsProbeFailure(t *testing.T) { // waitForAuthProbe blocks until the async probe spawned by SetConfig has // completed (signaled via the fixture's probeHook), then returns the persisted // config. A 2s ceiling guards against bugs that prevent the hook from firing. -func waitForAuthProbe(t *testing.T, f *svcFixture, workspaceID string) *JiraConfig { +func waitForAuthProbe(t *testing.T, f *svcFixture) *JiraConfig { t.Helper() - for { - select { - case got := <-f.probed: - if got != workspaceID { - continue - } - cfg, err := f.svc.GetConfig(context.Background(), workspaceID) - if err != nil { - t.Fatalf("get config after probe: %v", err) - } - return cfg - case <-time.After(2 * time.Second): - t.Fatalf("async probe hook did not fire for %q within 2s", workspaceID) - return nil + select { + case <-f.probed: + cfg, err := f.svc.GetConfig(context.Background()) + if err != nil { + t.Fatalf("get config after probe: %v", err) } + return cfg + case <-time.After(2 * time.Second): + t.Fatalf("async probe hook did not fire within 2s") + return nil } } @@ -221,20 +215,20 @@ func TestService_SetConfig_EmptySecret_KeepsExisting(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() _, err := f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "first", }) if err != nil { t.Fatalf("initial: %v", err) } _, err = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "new@x", + SiteURL: "https://a.net", Email: "new@x", AuthMethod: AuthMethodAPIToken, Secret: "", }) if err != nil { t.Fatalf("update: %v", err) } - if got, _ := f.secrets.Reveal(ctx, SecretKeyForWorkspace("ws-1")); got != "first" { + if got, _ := f.secrets.Reveal(ctx, SecretKey); got != "first" { t.Errorf("secret should be preserved, got %q", got) } } @@ -243,18 +237,18 @@ func TestService_SetConfig_InvalidatesClientCache(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() _, _ = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "t", }) // SetConfig spawns an async probe that also builds the client; wait for it // so the rest of the test sees a stable factoryHit count. - waitForAuthProbe(t, f, "ws-1") - if _, err := f.svc.GetTicket(ctx, "ws-1", "A-1"); err != nil { + waitForAuthProbe(t, f) + if _, err := f.svc.GetTicket(ctx, "A-1"); err != nil { t.Fatalf("get1: %v", err) } hits := f.factoryHit.Load() // Second call reuses cached client. - if _, err := f.svc.GetTicket(ctx, "ws-1", "A-2"); err != nil { + if _, err := f.svc.GetTicket(ctx, "A-2"); err != nil { t.Fatalf("get2: %v", err) } if got := f.factoryHit.Load(); got != hits { @@ -264,11 +258,11 @@ func TestService_SetConfig_InvalidatesClientCache(t *testing.T) { // SetConfig to finish before sampling factoryHit so the second probe can't // race with the GetTicket assertion. _, _ = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "t2", }) - waitForAuthProbe(t, f, "ws-1") - if _, err := f.svc.GetTicket(ctx, "ws-1", "A-3"); err != nil { + waitForAuthProbe(t, f) + if _, err := f.svc.GetTicket(ctx, "A-3"); err != nil { t.Fatalf("get3: %v", err) } if got := f.factoryHit.Load(); got <= hits { @@ -283,10 +277,9 @@ func TestService_Validation(t *testing.T) { name string req SetConfigRequest }{ - {"missing ws", SetConfigRequest{SiteURL: "x", AuthMethod: AuthMethodAPIToken, Email: "e"}}, - {"missing site", SetConfigRequest{WorkspaceID: "w", AuthMethod: AuthMethodAPIToken, Email: "e"}}, - {"missing email api_token", SetConfigRequest{WorkspaceID: "w", SiteURL: "x", AuthMethod: AuthMethodAPIToken}}, - {"bad auth", SetConfigRequest{WorkspaceID: "w", SiteURL: "x", AuthMethod: "bogus"}}, + {"missing site", SetConfigRequest{AuthMethod: AuthMethodAPIToken, Email: "e"}}, + {"missing email api_token", SetConfigRequest{SiteURL: "x", AuthMethod: AuthMethodAPIToken}}, + {"bad auth", SetConfigRequest{SiteURL: "x", AuthMethod: "bogus"}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -305,7 +298,7 @@ func TestService_TestConnection_InlineSecret(t *testing.T) { return &TestConnectionResult{OK: true, DisplayName: "Alice"}, nil } res, err := f.svc.TestConnection(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "inline", }) if err != nil || !called { @@ -319,7 +312,7 @@ func TestService_TestConnection_InlineSecret(t *testing.T) { func TestService_TestConnection_NoStoredSecret_ReturnsFailure(t *testing.T) { f := newSvcFixture(t) res, err := f.svc.TestConnection(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-nope", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, }) if err != nil { @@ -334,24 +327,24 @@ func TestService_DeleteConfig_RemovesSecret(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() _, _ = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "t", }) - if err := f.svc.DeleteConfig(ctx, "ws-1"); err != nil { + if err := f.svc.DeleteConfig(ctx); err != nil { t.Fatalf("delete: %v", err) } - if exists, _ := f.secrets.Exists(ctx, SecretKeyForWorkspace("ws-1")); exists { + if exists, _ := f.secrets.Exists(ctx, SecretKey); exists { t.Error("secret should be removed") } - cfg, _ := f.svc.GetConfig(ctx, "ws-1") + cfg, _ := f.svc.GetConfig(ctx) if cfg != nil { t.Errorf("expected config gone, got %+v", cfg) } } -func TestService_GetTicket_UnconfiguredWorkspace(t *testing.T) { +func TestService_GetTicket_Unconfigured(t *testing.T) { f := newSvcFixture(t) - _, err := f.svc.GetTicket(context.Background(), "ws-nope", "X-1") + _, err := f.svc.GetTicket(context.Background(), "X-1") if !errors.Is(err, ErrNotConfigured) { t.Errorf("expected ErrNotConfigured, got %v", err) } @@ -361,12 +354,12 @@ func TestService_GetTicket_RevealError_NotConfusedWithUnconfigured(t *testing.T) f := newSvcFixture(t) ctx := context.Background() if err := f.store.UpsertConfig(ctx, &JiraConfig{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, }); err != nil { t.Fatalf("seed config: %v", err) } - _, err := f.svc.GetTicket(ctx, "ws-1", "X-1") + _, err := f.svc.GetTicket(ctx, "X-1") if err == nil { t.Fatal("expected error") } @@ -379,10 +372,10 @@ func TestService_DoTransition_PassThrough(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() _, _ = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", SiteURL: "https://a.net", Email: "e", + SiteURL: "https://a.net", Email: "e", AuthMethod: AuthMethodAPIToken, Secret: "t", }) - if err := f.svc.DoTransition(ctx, "ws-1", "PROJ-9", "31"); err != nil { + if err := f.svc.DoTransition(ctx, "PROJ-9", "31"); err != nil { t.Fatalf("transition: %v", err) } if len(f.client.transitionLog) != 1 || f.client.transitionLog[0] != "PROJ-9:31" { diff --git a/apps/backend/internal/jira/store.go b/apps/backend/internal/jira/store.go index ed4ba555ae..be8b6aa2fb 100644 --- a/apps/backend/internal/jira/store.go +++ b/apps/backend/internal/jira/store.go @@ -11,11 +11,18 @@ import ( "github.com/jmoiron/sqlx" ) -// Store persists Jira workspace configurations. Secret values are delegated to -// the shared encrypted secret store and not stored here. +// Store persists the Jira configuration. Secret values are delegated to the +// shared encrypted secret store and not stored here. The configuration is a +// singleton — there is one Jira account per install, not one per workspace. type Store struct { db *sqlx.DB ro *sqlx.DB + + // migratedFromWorkspace records the workspace_id of the row that was + // promoted into the singleton during initSchema. Provider reads this to + // migrate the per-workspace secret to the new global key. Empty when no + // migration ran (fresh install or already migrated). + migratedFromWorkspace string } // NewStore creates a new Store and initializes the schema if needed. @@ -27,9 +34,16 @@ func NewStore(writer, reader *sqlx.DB) (*Store, error) { return s, nil } +// MigratedFromWorkspace returns the workspace_id of the row promoted to the +// singleton during the per-workspace → singleton schema migration, or "" when +// no migration ran. Provider uses this to copy the secret over. +func (s *Store) MigratedFromWorkspace() string { + return s.migratedFromWorkspace +} + const createTablesSQL = ` CREATE TABLE IF NOT EXISTS jira_configs ( - workspace_id TEXT PRIMARY KEY, + id TEXT PRIMARY KEY CHECK(id = 'singleton'), site_url TEXT NOT NULL, email TEXT NOT NULL DEFAULT '', auth_method TEXT NOT NULL, @@ -71,43 +85,126 @@ const createTablesSQL = ` ); ` -// addedColumns lists columns introduced after the initial schema. SQLite has no -// portable `ADD COLUMN IF NOT EXISTS`, so we ask sqlite_master for the existing -// CREATE TABLE statement and only ALTER when the column name is missing. -var addedColumns = []struct { - name string - sql string -}{ - {"last_checked_at", "ALTER TABLE jira_configs ADD COLUMN last_checked_at DATETIME"}, - {"last_ok", "ALTER TABLE jira_configs ADD COLUMN last_ok INTEGER NOT NULL DEFAULT 0"}, - {"last_error", "ALTER TABLE jira_configs ADD COLUMN last_error TEXT NOT NULL DEFAULT ''"}, -} +// singletonID is the synthetic primary key of the (only) row in jira_configs. +// The CHECK constraint on the column makes inserting any other id an error, +// so the table can never accidentally grow back to per-workspace rows. +const singletonID = "singleton" func (s *Store) initSchema() error { + if err := s.migrateLegacyPerWorkspaceTable(); err != nil { + return err + } if _, err := s.db.Exec(createTablesSQL); err != nil { return err } - return s.migrateAddedColumns() + return nil } -// migrateAddedColumns applies ALTER TABLE statements for columns introduced -// after the initial schema. Existing databases created with the old CREATE -// TABLE need these columns backfilled; new databases already have them from -// createTablesSQL and the ALTERs are skipped. -func (s *Store) migrateAddedColumns() error { - existing, err := s.tableColumns("jira_configs") +// migrateLegacyPerWorkspaceTable detects the pre-singleton schema (where +// jira_configs was keyed by workspace_id) and rewrites it into the singleton +// shape. Picks the most-recently-updated row as the surviving config and +// records the source workspace_id so the provider can migrate the secret. +// +// Idempotent: a fresh install has no legacy table and falls through to the +// CREATE TABLE IF NOT EXISTS in createTablesSQL. +func (s *Store) migrateLegacyPerWorkspaceTable() error { + cols, err := s.tableColumns("jira_configs") if err != nil { return err } - for _, col := range addedColumns { - if _, ok := existing[col.name]; ok { - continue + if len(cols) == 0 { + return nil + } + if _, hasWorkspace := cols["workspace_id"]; !hasWorkspace { + return nil + } + tx, err := s.db.Begin() + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + + // `last_checked_at`, `last_ok`, and `last_error` were added to the legacy + // schema in a later release via ALTER TABLE. A deployment that upgrades + // from the original schema would have a `workspace_id` column but not + // these — selecting them unconditionally would crash startup. Build the + // SELECT against only columns present in this database. + healthCols := healthColumnsPresent(cols) + selectCols := "workspace_id, site_url, email, auth_method, default_project_key" + if healthCols { + selectCols += ", last_checked_at, last_ok, last_error" + } else { + selectCols += ", NULL AS last_checked_at, 0 AS last_ok, '' AS last_error" + } + selectCols += ", created_at, updated_at" + var sourceWorkspace, siteURL, email, authMethod, defaultProjectKey, lastError sql.NullString + var lastCheckedAt sql.NullTime + var lastOk sql.NullInt64 + var createdAt, updatedAt sql.NullTime + row := tx.QueryRow(`SELECT ` + selectCols + ` FROM jira_configs ORDER BY updated_at DESC LIMIT 1`) + switch err := row.Scan(&sourceWorkspace, &siteURL, &email, &authMethod, &defaultProjectKey, + &lastCheckedAt, &lastOk, &lastError, &createdAt, &updatedAt); { + case errors.Is(err, sql.ErrNoRows): + // Empty legacy table — drop and let createTablesSQL build the new shape. + if _, err := tx.Exec(`DROP TABLE jira_configs`); err != nil { + return err } - if _, err := s.db.Exec(col.sql); err != nil { - return fmt.Errorf("add column %s: %w", col.name, err) + return tx.Commit() + case err != nil: + return err + } + if _, err := tx.Exec(`DROP TABLE jira_configs`); err != nil { + return err + } + if _, err := tx.Exec(` + CREATE TABLE jira_configs ( + id TEXT PRIMARY KEY CHECK(id = 'singleton'), + site_url TEXT NOT NULL, + email TEXT NOT NULL DEFAULT '', + auth_method TEXT NOT NULL, + default_project_key TEXT NOT NULL DEFAULT '', + last_checked_at DATETIME, + last_ok INTEGER NOT NULL DEFAULT 0, + last_error TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + return err + } + if _, err := tx.Exec(` + INSERT INTO jira_configs (id, site_url, email, auth_method, default_project_key, + last_checked_at, last_ok, last_error, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + singletonID, siteURL.String, email.String, authMethod.String, defaultProjectKey.String, + nullableTime(lastCheckedAt), lastOk.Int64, lastError.String, + nullableTime(createdAt), nullableTime(updatedAt)); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return err + } + s.migratedFromWorkspace = sourceWorkspace.String + return nil +} + +// healthColumnsPresent reports whether the legacy jira_configs table has the +// auth-health columns that were added in a later release. When all three are +// missing we fall back to NULL/zero defaults rather than crashing on the +// SELECT. +func healthColumnsPresent(cols map[string]struct{}) bool { + for _, name := range []string{"last_checked_at", "last_ok", "last_error"} { + if _, ok := cols[name]; !ok { + return false } } - return nil + return true +} + +func nullableTime(t sql.NullTime) interface{} { + if !t.Valid { + return nil + } + return t.Time } func (s *Store) tableColumns(table string) (map[string]struct{}, error) { @@ -134,14 +231,14 @@ func (s *Store) tableColumns(table string) (map[string]struct{}, error) { return cols, rows.Err() } -const selectConfigColumns = `workspace_id, site_url, email, auth_method, default_project_key, +const selectConfigColumns = `site_url, email, auth_method, default_project_key, last_checked_at, last_ok, last_error, created_at, updated_at` -// GetConfig returns the Jira config for a workspace, or nil when no row exists. -func (s *Store) GetConfig(ctx context.Context, workspaceID string) (*JiraConfig, error) { +// GetConfig returns the singleton Jira config, or nil when no row exists. +func (s *Store) GetConfig(ctx context.Context) (*JiraConfig, error) { var cfg JiraConfig err := s.ro.GetContext(ctx, &cfg, - `SELECT `+selectConfigColumns+` FROM jira_configs WHERE workspace_id = ?`, workspaceID) + `SELECT `+selectConfigColumns+` FROM jira_configs WHERE id = ?`, singletonID) if errors.Is(err, sql.ErrNoRows) { return nil, nil } @@ -151,10 +248,10 @@ func (s *Store) GetConfig(ctx context.Context, workspaceID string) (*JiraConfig, return &cfg, nil } -// UpsertConfig inserts or updates the config row for a workspace. It never -// touches the secret store — callers must persist the token separately. The -// last_* health columns are deliberately not touched here; the poller owns -// those and writes them via UpdateAuthHealth. +// UpsertConfig inserts or updates the singleton config row. It never touches +// the secret store — callers must persist the token separately. The last_* +// health columns are deliberately not touched here; the poller owns those and +// writes them via UpdateAuthHealth. func (s *Store) UpsertConfig(ctx context.Context, cfg *JiraConfig) error { now := time.Now().UTC() if cfg.CreatedAt.IsZero() { @@ -162,47 +259,47 @@ func (s *Store) UpsertConfig(ctx context.Context, cfg *JiraConfig) error { } cfg.UpdatedAt = now _, err := s.db.ExecContext(ctx, ` - INSERT INTO jira_configs (workspace_id, site_url, email, auth_method, default_project_key, created_at, updated_at) + INSERT INTO jira_configs (id, site_url, email, auth_method, default_project_key, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?) - ON CONFLICT(workspace_id) DO UPDATE SET + ON CONFLICT(id) DO UPDATE SET site_url = excluded.site_url, email = excluded.email, auth_method = excluded.auth_method, default_project_key = excluded.default_project_key, updated_at = excluded.updated_at`, - cfg.WorkspaceID, cfg.SiteURL, cfg.Email, cfg.AuthMethod, cfg.DefaultProjectKey, cfg.CreatedAt, cfg.UpdatedAt) + singletonID, cfg.SiteURL, cfg.Email, cfg.AuthMethod, cfg.DefaultProjectKey, cfg.CreatedAt, cfg.UpdatedAt) return err } -// DeleteConfig removes the Jira config row for a workspace. Secrets must be -// cleared separately by the caller. -func (s *Store) DeleteConfig(ctx context.Context, workspaceID string) error { - _, err := s.db.ExecContext(ctx, `DELETE FROM jira_configs WHERE workspace_id = ?`, workspaceID) +// DeleteConfig removes the singleton config row. Secrets must be cleared +// separately by the caller. +func (s *Store) DeleteConfig(ctx context.Context) error { + _, err := s.db.ExecContext(ctx, `DELETE FROM jira_configs WHERE id = ?`, singletonID) return err } -// ListConfiguredWorkspaces returns the IDs of all workspaces that have a Jira -// config row. Used by the auth-health poller to know which workspaces to probe. -func (s *Store) ListConfiguredWorkspaces(ctx context.Context) ([]string, error) { - var ids []string - err := s.ro.SelectContext(ctx, &ids, - `SELECT workspace_id FROM jira_configs ORDER BY workspace_id`) +// HasConfig reports whether the singleton row exists. Used by the auth-health +// poller to decide whether to probe at all. +func (s *Store) HasConfig(ctx context.Context) (bool, error) { + var present int + err := s.ro.GetContext(ctx, &present, + `SELECT COUNT(*) FROM jira_configs WHERE id = ?`, singletonID) if err != nil { - return nil, err + return false, err } - return ids, nil + return present > 0, nil } -// UpdateAuthHealth records the result of a credential probe for a workspace. -// errMsg is the empty string when ok is true. If the workspace row no longer +// UpdateAuthHealth records the result of a credential probe on the singleton +// config row. errMsg is the empty string when ok is true. If the row no longer // exists (e.g. the user removed the config concurrently with the poller), the // update is a silent no-op rather than an error. -func (s *Store) UpdateAuthHealth(ctx context.Context, workspaceID string, ok bool, errMsg string, checkedAt time.Time) error { +func (s *Store) UpdateAuthHealth(ctx context.Context, ok bool, errMsg string, checkedAt time.Time) error { _, err := s.db.ExecContext(ctx, ` UPDATE jira_configs SET last_checked_at = ?, last_ok = ?, last_error = ? - WHERE workspace_id = ?`, - checkedAt, ok, errMsg, workspaceID) + WHERE id = ?`, + checkedAt, ok, errMsg, singletonID) return err } @@ -260,6 +357,20 @@ func (s *Store) ListIssueWatches(ctx context.Context, workspaceID string) ([]*Is return watches, nil } +// ListAllIssueWatches returns every watch across all workspaces, in insertion +// order. Used by the install-wide settings UI when no workspace filter is +// supplied — the table renders a Workspace column so the user can manage +// watches without first picking a workspace context. +func (s *Store) ListAllIssueWatches(ctx context.Context) ([]*IssueWatch, error) { + var watches []*IssueWatch + err := s.ro.SelectContext(ctx, &watches, + `SELECT `+issueWatchColumns+` FROM jira_issue_watches ORDER BY workspace_id, created_at`) + if err != nil { + return nil, err + } + return watches, nil +} + // ListEnabledIssueWatches returns every enabled watch across all workspaces, // used by the poller to decide what to query each tick. func (s *Store) ListEnabledIssueWatches(ctx context.Context) ([]*IssueWatch, error) { diff --git a/apps/backend/internal/jira/store_test.go b/apps/backend/internal/jira/store_test.go index 1a101e3e5b..f373d815c2 100644 --- a/apps/backend/internal/jira/store_test.go +++ b/apps/backend/internal/jira/store_test.go @@ -32,7 +32,6 @@ func TestStore_UpsertGetDelete(t *testing.T) { ctx := context.Background() cfg := &JiraConfig{ - WorkspaceID: "ws-1", SiteURL: "https://acme.atlassian.net", Email: "user@example.com", AuthMethod: AuthMethodAPIToken, @@ -42,7 +41,7 @@ func TestStore_UpsertGetDelete(t *testing.T) { t.Fatalf("upsert: %v", err) } - got, err := store.GetConfig(ctx, "ws-1") + got, err := store.GetConfig(ctx) if err != nil { t.Fatalf("get: %v", err) } @@ -61,15 +60,15 @@ func TestStore_UpsertGetDelete(t *testing.T) { if err := store.UpsertConfig(ctx, cfg); err != nil { t.Fatalf("update upsert: %v", err) } - got2, _ := store.GetConfig(ctx, "ws-1") + got2, _ := store.GetConfig(ctx) if got2.Email != "other@example.com" { t.Errorf("expected email update, got %q", got2.Email) } - if err := store.DeleteConfig(ctx, "ws-1"); err != nil { + if err := store.DeleteConfig(ctx); err != nil { t.Fatalf("delete: %v", err) } - gone, err := store.GetConfig(ctx, "ws-1") + gone, err := store.GetConfig(ctx) if err != nil { t.Fatalf("get after delete: %v", err) } @@ -80,7 +79,7 @@ func TestStore_UpsertGetDelete(t *testing.T) { func TestStore_GetConfig_Missing(t *testing.T) { store := newTestStore(t) - cfg, err := store.GetConfig(context.Background(), "does-not-exist") + cfg, err := store.GetConfig(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -89,27 +88,29 @@ func TestStore_GetConfig_Missing(t *testing.T) { } } -func TestStore_ListConfiguredWorkspaces(t *testing.T) { +func TestStore_HasConfig(t *testing.T) { store := newTestStore(t) ctx := context.Background() - mustUpsert := func(id string) { - if err := store.UpsertConfig(ctx, &JiraConfig{ - WorkspaceID: id, - SiteURL: "https://" + id + ".atlassian.net", - Email: id + "@example.com", - AuthMethod: AuthMethodAPIToken, - }); err != nil { - t.Fatalf("upsert %s: %v", id, err) - } - } - mustUpsert("ws-b") - mustUpsert("ws-a") - ids, err := store.ListConfiguredWorkspaces(ctx) + has, err := store.HasConfig(ctx) if err != nil { - t.Fatalf("list: %v", err) + t.Fatalf("has-config: %v", err) } - if len(ids) != 2 || ids[0] != "ws-a" || ids[1] != "ws-b" { - t.Errorf("expected sorted [ws-a ws-b], got %v", ids) + if has { + t.Errorf("expected HasConfig=false on empty store, got true") + } + if err := store.UpsertConfig(ctx, &JiraConfig{ + SiteURL: "https://acme.atlassian.net", + Email: "user@example.com", + AuthMethod: AuthMethodAPIToken, + }); err != nil { + t.Fatalf("upsert: %v", err) + } + has, err = store.HasConfig(ctx) + if err != nil { + t.Fatalf("has-config after upsert: %v", err) + } + if !has { + t.Errorf("expected HasConfig=true after upsert") } } @@ -117,16 +118,15 @@ func TestStore_UpdateAuthHealth(t *testing.T) { store := newTestStore(t) ctx := context.Background() if err := store.UpsertConfig(ctx, &JiraConfig{ - WorkspaceID: "ws-1", - SiteURL: "https://acme.atlassian.net", - Email: "u@example.com", - AuthMethod: AuthMethodAPIToken, + SiteURL: "https://acme.atlassian.net", + Email: "u@example.com", + AuthMethod: AuthMethodAPIToken, }); err != nil { t.Fatalf("upsert: %v", err) } // Initial state: no health recorded yet. - cfg, _ := store.GetConfig(ctx, "ws-1") + cfg, _ := store.GetConfig(ctx) if cfg.LastCheckedAt != nil { t.Errorf("expected nil last_checked_at on fresh row, got %v", cfg.LastCheckedAt) } @@ -136,10 +136,10 @@ func TestStore_UpdateAuthHealth(t *testing.T) { // Record success. now := time.Now().UTC().Truncate(time.Second) - if err := store.UpdateAuthHealth(ctx, "ws-1", true, "", now); err != nil { + if err := store.UpdateAuthHealth(ctx, true, "", now); err != nil { t.Fatalf("update ok: %v", err) } - cfg, _ = store.GetConfig(ctx, "ws-1") + cfg, _ = store.GetConfig(ctx) if !cfg.LastOk { t.Error("expected last_ok=true after successful probe") } @@ -152,10 +152,10 @@ func TestStore_UpdateAuthHealth(t *testing.T) { // Record failure — error string is preserved, ok flips back to false. failAt := now.Add(time.Minute) - if err := store.UpdateAuthHealth(ctx, "ws-1", false, "step-up required", failAt); err != nil { + if err := store.UpdateAuthHealth(ctx, false, "step-up required", failAt); err != nil { t.Fatalf("update fail: %v", err) } - cfg, _ = store.GetConfig(ctx, "ws-1") + cfg, _ = store.GetConfig(ctx) if cfg.LastOk { t.Error("expected last_ok=false after failure") } @@ -165,10 +165,170 @@ func TestStore_UpdateAuthHealth(t *testing.T) { if cfg.LastCheckedAt == nil || !cfg.LastCheckedAt.Equal(failAt) { t.Errorf("expected last_checked_at=%v, got %v", failAt, cfg.LastCheckedAt) } +} - // Updating health for a non-existent workspace must not error (silent - // no-op — the row may have been deleted between probe and persist). - if err := store.UpdateAuthHealth(ctx, "missing", true, "", now); err != nil { - t.Errorf("expected no-op for missing row, got %v", err) +func TestStore_MigrateLegacyPerWorkspaceTable(t *testing.T) { + raw, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + raw.SetMaxOpenConns(1) + raw.SetMaxIdleConns(1) + db := sqlx.NewDb(raw, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + + // Build the legacy schema and seed two per-workspace rows; the most + // recently-updated row should win the singleton slot. + if _, err := db.Exec(` + CREATE TABLE jira_configs ( + workspace_id TEXT PRIMARY KEY, + site_url TEXT NOT NULL, + email TEXT NOT NULL DEFAULT '', + auth_method TEXT NOT NULL, + default_project_key TEXT NOT NULL DEFAULT '', + last_checked_at DATETIME, + last_ok INTEGER NOT NULL DEFAULT 0, + last_error TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + t.Fatalf("legacy schema: %v", err) + } + older := time.Now().UTC().Add(-time.Hour) + newer := time.Now().UTC() + if _, err := db.Exec(`INSERT INTO jira_configs + (workspace_id, site_url, email, auth_method, default_project_key, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + "ws-old", "https://old.atlassian.net", "old@example.com", AuthMethodAPIToken, "OLD", older, older); err != nil { + t.Fatalf("seed old row: %v", err) + } + if _, err := db.Exec(`INSERT INTO jira_configs + (workspace_id, site_url, email, auth_method, default_project_key, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + "ws-new", "https://new.atlassian.net", "new@example.com", AuthMethodAPIToken, "NEW", newer, newer); err != nil { + t.Fatalf("seed new row: %v", err) + } + + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("new store: %v", err) + } + if got := store.MigratedFromWorkspace(); got != "ws-new" { + t.Errorf("expected migration source ws-new, got %q", got) + } + cfg, err := store.GetConfig(context.Background()) + if err != nil { + t.Fatalf("get singleton: %v", err) + } + if cfg == nil { + t.Fatal("expected singleton row after migration") + } + if cfg.SiteURL != "https://new.atlassian.net" { + t.Errorf("expected newest row promoted, got SiteURL=%q", cfg.SiteURL) + } +} + +// Covers the sql.ErrNoRows branch: a user who installed a previous version, +// never configured Jira, and then upgrades hits this path. The legacy table +// exists but contains zero rows. +func TestStore_MigrateLegacyPerWorkspaceTable_EmptyTable(t *testing.T) { + raw, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + raw.SetMaxOpenConns(1) + raw.SetMaxIdleConns(1) + db := sqlx.NewDb(raw, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + + if _, err := db.Exec(` + CREATE TABLE jira_configs ( + workspace_id TEXT PRIMARY KEY, + site_url TEXT NOT NULL, + email TEXT NOT NULL DEFAULT '', + auth_method TEXT NOT NULL, + default_project_key TEXT NOT NULL DEFAULT '', + last_checked_at DATETIME, + last_ok INTEGER NOT NULL DEFAULT 0, + last_error TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + t.Fatalf("legacy schema: %v", err) + } + + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("NewStore on empty legacy table: %v", err) + } + if got := store.MigratedFromWorkspace(); got != "" { + t.Errorf("expected empty MigratedFromWorkspace, got %q", got) + } + cfg, err := store.GetConfig(context.Background()) + if err != nil { + t.Fatalf("get singleton: %v", err) + } + if cfg != nil { + t.Errorf("expected nil cfg on empty legacy table, got %+v", cfg) + } +} + +// Covers the original-schema upgrade path: the legacy table exists with +// `workspace_id` but lacks the auth-health columns added in a later release. +// The migration must select only guaranteed-present columns and fall back to +// defaults for the missing ones, otherwise startup crashes with "no such column". +func TestStore_MigrateLegacyPerWorkspaceTable_PreHealthColumns(t *testing.T) { + raw, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + raw.SetMaxOpenConns(1) + raw.SetMaxIdleConns(1) + db := sqlx.NewDb(raw, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + + // Original schema: no last_checked_at / last_ok / last_error. + if _, err := db.Exec(` + CREATE TABLE jira_configs ( + workspace_id TEXT PRIMARY KEY, + site_url TEXT NOT NULL, + email TEXT NOT NULL DEFAULT '', + auth_method TEXT NOT NULL, + default_project_key TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + t.Fatalf("legacy schema: %v", err) + } + now := time.Now().UTC() + if _, err := db.Exec(`INSERT INTO jira_configs + (workspace_id, site_url, email, auth_method, default_project_key, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + "ws-1", "https://acme.atlassian.net", "u@example.com", AuthMethodAPIToken, "PROJ", now, now); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("NewStore on pre-health-columns schema: %v", err) + } + if got := store.MigratedFromWorkspace(); got != "ws-1" { + t.Errorf("expected migration source ws-1, got %q", got) + } + cfg, err := store.GetConfig(context.Background()) + if err != nil { + t.Fatalf("get singleton: %v", err) + } + if cfg == nil { + t.Fatal("expected singleton row after migration") + } + if cfg.SiteURL != "https://acme.atlassian.net" { + t.Errorf("expected SiteURL preserved, got %q", cfg.SiteURL) + } + if cfg.LastOk { + t.Error("expected LastOk=false (default) after migrating from pre-health schema") + } + if cfg.LastCheckedAt != nil { + t.Errorf("expected LastCheckedAt=nil (default), got %v", cfg.LastCheckedAt) } } diff --git a/apps/backend/internal/linear/handlers.go b/apps/backend/internal/linear/handlers.go index a1b52f7796..b119b775e7 100644 --- a/apps/backend/internal/linear/handlers.go +++ b/apps/backend/internal/linear/handlers.go @@ -14,11 +14,6 @@ import ( ws "github.com/kandev/kandev/pkg/websocket" ) -// errMsgWorkspaceIDRequired is the response body returned when a handler is -// invoked without the workspace_id query parameter. Centralised because the -// linter flags repeated string literals at three or more occurrences. -const errMsgWorkspaceIDRequired = "workspace_id required" - // RegisterRoutes wires the Linear HTTP and WebSocket handlers. func RegisterRoutes(router *gin.Engine, dispatcher *ws.Dispatcher, svc *Service, log *logger.Logger) { ctrl := &Controller{service: svc, logger: log} @@ -49,12 +44,7 @@ func (c *Controller) RegisterHTTPRoutes(router *gin.Engine) { // --- HTTP handlers --- func (c *Controller) httpGetConfig(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": errMsgWorkspaceIDRequired}) - return - } - cfg, err := c.service.GetConfig(ctx.Request.Context(), workspaceID) + cfg, err := c.service.GetConfig(ctx.Request.Context()) if err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -85,12 +75,7 @@ func (c *Controller) httpSetConfig(ctx *gin.Context) { } func (c *Controller) httpDeleteConfig(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": errMsgWorkspaceIDRequired}) - return - } - if err := c.service.DeleteConfig(ctx.Request.Context(), workspaceID); err != nil { + if err := c.service.DeleteConfig(ctx.Request.Context()); err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } @@ -112,12 +97,7 @@ func (c *Controller) httpTestConfig(ctx *gin.Context) { } func (c *Controller) httpListTeams(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": errMsgWorkspaceIDRequired}) - return - } - teams, err := c.service.ListTeams(ctx.Request.Context(), workspaceID) + teams, err := c.service.ListTeams(ctx.Request.Context()) if err != nil { c.writeClientError(ctx, err) return @@ -126,13 +106,12 @@ func (c *Controller) httpListTeams(ctx *gin.Context) { } func (c *Controller) httpListStates(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") teamKey := ctx.Query("team_key") - if workspaceID == "" || teamKey == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id and team_key required"}) + if teamKey == "" { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "team_key required"}) return } - states, err := c.service.ListStates(ctx.Request.Context(), workspaceID, teamKey) + states, err := c.service.ListStates(ctx.Request.Context(), teamKey) if err != nil { c.writeClientError(ctx, err) return @@ -141,11 +120,6 @@ func (c *Controller) httpListStates(ctx *gin.Context) { } func (c *Controller) httpSearchIssues(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": errMsgWorkspaceIDRequired}) - return - } filter := SearchFilter{ Query: ctx.Query("query"), TeamKey: ctx.Query("team_key"), @@ -156,7 +130,7 @@ func (c *Controller) httpSearchIssues(ctx *gin.Context) { } pageToken := ctx.Query("page_token") maxResults, _ := strconv.Atoi(ctx.Query("max_results")) - result, err := c.service.SearchIssues(ctx.Request.Context(), workspaceID, filter, pageToken, maxResults) + result, err := c.service.SearchIssues(ctx.Request.Context(), filter, pageToken, maxResults) if err != nil { c.writeClientError(ctx, err) return @@ -165,13 +139,8 @@ func (c *Controller) httpSearchIssues(ctx *gin.Context) { } func (c *Controller) httpGetIssue(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": errMsgWorkspaceIDRequired}) - return - } id := ctx.Param("id") - issue, err := c.service.GetIssue(ctx.Request.Context(), workspaceID, id) + issue, err := c.service.GetIssue(ctx.Request.Context(), id) if err != nil { c.writeClientError(ctx, err) return @@ -180,11 +149,6 @@ func (c *Controller) httpGetIssue(ctx *gin.Context) { } func (c *Controller) httpSetIssueState(ctx *gin.Context) { - workspaceID := ctx.Query("workspace_id") - if workspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": errMsgWorkspaceIDRequired}) - return - } id := ctx.Param("id") var req struct { StateID string `json:"stateId"` @@ -193,7 +157,7 @@ func (c *Controller) httpSetIssueState(ctx *gin.Context) { ctx.JSON(http.StatusBadRequest, gin.H{"error": "stateId required"}) return } - if err := c.service.SetIssueState(ctx.Request.Context(), workspaceID, id, req.StateID); err != nil { + if err := c.service.SetIssueState(ctx.Request.Context(), id, req.StateID); err != nil { c.writeClientError(ctx, err) return } @@ -201,14 +165,14 @@ func (c *Controller) httpSetIssueState(ctx *gin.Context) { } // errCodeLinearNotConfigured is the wire-level code surfaced to the UI when -// the workspace has no saved Linear credentials. +// Linear has no saved credentials. const errCodeLinearNotConfigured = "LINEAR_NOT_CONFIGURED" // writeClientError maps service-level errors to HTTP responses. func (c *Controller) writeClientError(ctx *gin.Context, err error) { if errors.Is(err, ErrNotConfigured) { ctx.JSON(http.StatusServiceUnavailable, gin.H{ - "error": "Linear is not configured for this workspace", + "error": "Linear is not configured", "code": errCodeLinearNotConfigured, }) return @@ -268,13 +232,7 @@ func wsFail(msg *ws.Message, err error) (*ws.Message, error) { func wsGetConfig(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { - var p struct { - WorkspaceID string `json:"workspaceId"` - } - if err := msg.ParsePayload(&p); err != nil || p.WorkspaceID == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId required", nil) - } - cfg, err := svc.GetConfig(ctx, p.WorkspaceID) + cfg, err := svc.GetConfig(ctx) if err != nil { return wsFail(msg, err) } @@ -301,13 +259,7 @@ func wsSetConfig(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.M func wsDeleteConfig(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { - var p struct { - WorkspaceID string `json:"workspaceId"` - } - if err := msg.ParsePayload(&p); err != nil || p.WorkspaceID == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId required", nil) - } - if err := svc.DeleteConfig(ctx, p.WorkspaceID); err != nil { + if err := svc.DeleteConfig(ctx); err != nil { return wsFail(msg, err) } return wsReply(msg, gin.H{"deleted": true}) @@ -331,16 +283,15 @@ func wsTestConfig(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws. func wsGetIssue(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var p struct { - WorkspaceID string `json:"workspaceId"` - Identifier string `json:"identifier"` + Identifier string `json:"identifier"` } if err := msg.ParsePayload(&p); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload", nil) } - if p.WorkspaceID == "" || p.Identifier == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId and identifier required", nil) + if p.Identifier == "" { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "identifier required", nil) } - issue, err := svc.GetIssue(ctx, p.WorkspaceID, p.Identifier) + issue, err := svc.GetIssue(ctx, p.Identifier) if err != nil { return wsFail(msg, err) } @@ -351,17 +302,16 @@ func wsGetIssue(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Me func wsSetIssueState(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var p struct { - WorkspaceID string `json:"workspaceId"` - IssueID string `json:"issueId"` - StateID string `json:"stateId"` + IssueID string `json:"issueId"` + StateID string `json:"stateId"` } if err := msg.ParsePayload(&p); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload", nil) } - if p.WorkspaceID == "" || p.IssueID == "" || p.StateID == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId, issueId, stateId required", nil) + if p.IssueID == "" || p.StateID == "" { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "issueId and stateId required", nil) } - if err := svc.SetIssueState(ctx, p.WorkspaceID, p.IssueID, p.StateID); err != nil { + if err := svc.SetIssueState(ctx, p.IssueID, p.StateID); err != nil { return wsFail(msg, err) } return wsReply(msg, gin.H{"transitioned": true}) @@ -370,13 +320,7 @@ func wsSetIssueState(svc *Service) func(ctx context.Context, msg *ws.Message) (* func wsListTeams(svc *Service) func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { return func(ctx context.Context, msg *ws.Message) (*ws.Message, error) { - var p struct { - WorkspaceID string `json:"workspaceId"` - } - if err := msg.ParsePayload(&p); err != nil || p.WorkspaceID == "" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "workspaceId required", nil) - } - teams, err := svc.ListTeams(ctx, p.WorkspaceID) + teams, err := svc.ListTeams(ctx) if err != nil { return wsFail(msg, err) } diff --git a/apps/backend/internal/linear/handlers_test.go b/apps/backend/internal/linear/handlers_test.go index 6cc0de74ea..478b1e6ccc 100644 --- a/apps/backend/internal/linear/handlers_test.go +++ b/apps/backend/internal/linear/handlers_test.go @@ -103,19 +103,9 @@ func TestWriteClientError_GenericError_Returns500(t *testing.T) { } } -func TestHTTPGetConfig_RequiresWorkspaceID(t *testing.T) { - _, router, _ := newTestController(t) - req := httptest.NewRequest(http.MethodGet, "/api/v1/linear/config", nil) - w := httptest.NewRecorder() - router.ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - func TestHTTPGetConfig_NoConfig_Returns204(t *testing.T) { _, router, _ := newTestController(t) - req := httptest.NewRequest(http.MethodGet, "/api/v1/linear/config?workspace_id=missing", nil) + req := httptest.NewRequest(http.MethodGet, "/api/v1/linear/config", nil) w := httptest.NewRecorder() router.ServeHTTP(w, req) if w.Code != http.StatusNoContent { @@ -123,19 +113,13 @@ func TestHTTPGetConfig_NoConfig_Returns204(t *testing.T) { } } -func TestHTTPListStates_RequiresBothParams(t *testing.T) { +func TestHTTPListStates_RequiresTeamKey(t *testing.T) { _, router, _ := newTestController(t) - for _, url := range []string{ - "/api/v1/linear/states", - "/api/v1/linear/states?workspace_id=ws-1", - "/api/v1/linear/states?team_key=ENG", - } { - req := httptest.NewRequest(http.MethodGet, url, nil) - w := httptest.NewRecorder() - router.ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("%s: status = %d, want 400", url, w.Code) - } + req := httptest.NewRequest(http.MethodGet, "/api/v1/linear/states", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", w.Code) } } @@ -143,12 +127,11 @@ func TestHTTPListStates_RoutesThroughService(t *testing.T) { ctrl, router, client := newTestController(t) ctx := context.Background() if err := ctrl.service.store.UpsertConfig(ctx, &LinearConfig{ - WorkspaceID: "ws-1", - AuthMethod: AuthMethodAPIKey, + AuthMethod: AuthMethodAPIKey, }); err != nil { t.Fatalf("upsert: %v", err) } - if err := ctrl.service.secrets.Set(ctx, SecretKeyForWorkspace("ws-1"), "linear", "tok"); err != nil { + if err := ctrl.service.secrets.Set(ctx, SecretKey, "linear", "tok"); err != nil { t.Fatalf("set secret: %v", err) } var seenTeam string @@ -156,7 +139,7 @@ func TestHTTPListStates_RoutesThroughService(t *testing.T) { seenTeam = teamKey return []LinearWorkflowState{{ID: "s1", Name: "In Progress"}}, nil } - req := httptest.NewRequest(http.MethodGet, "/api/v1/linear/states?workspace_id=ws-1&team_key=ENG", nil) + req := httptest.NewRequest(http.MethodGet, "/api/v1/linear/states?team_key=ENG", nil) w := httptest.NewRecorder() router.ServeHTTP(w, req) if w.Code != http.StatusOK { diff --git a/apps/backend/internal/linear/mock_controller.go b/apps/backend/internal/linear/mock_controller.go index 1d59e1392f..b5bd1f7a5d 100644 --- a/apps/backend/internal/linear/mock_controller.go +++ b/apps/backend/internal/linear/mock_controller.go @@ -49,16 +49,15 @@ func (c *MockController) setAuthResult(ctx *gin.Context) { func (c *MockController) setAuthHealth(ctx *gin.Context) { var req struct { - WorkspaceID string `json:"workspaceId"` - OK bool `json:"ok"` - Error string `json:"error"` - OrgSlug string `json:"orgSlug"` + OK bool `json:"ok"` + Error string `json:"error"` + OrgSlug string `json:"orgSlug"` } - if err := ctx.ShouldBindJSON(&req); err != nil || req.WorkspaceID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspaceId required"}) + if err := ctx.ShouldBindJSON(&req); err != nil { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"}) return } - if err := c.store.UpdateAuthHealth(ctx.Request.Context(), req.WorkspaceID, req.OK, req.Error, req.OrgSlug, time.Now().UTC()); err != nil { + if err := c.store.UpdateAuthHealth(ctx.Request.Context(), req.OK, req.Error, req.OrgSlug, time.Now().UTC()); err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } diff --git a/apps/backend/internal/linear/models.go b/apps/backend/internal/linear/models.go index 82a6a54044..6e21cc53cc 100644 --- a/apps/backend/internal/linear/models.go +++ b/apps/backend/internal/linear/models.go @@ -1,7 +1,6 @@ -// Package linear implements the Linear integration: workspace-scoped -// configuration storage, a GraphQL client for issues and workflow states, and -// the HTTP and WebSocket handlers that expose these capabilities to the -// frontend. +// Package linear implements the Linear integration: a single install-wide +// configuration, a GraphQL client for issues and workflow states, and the HTTP +// and WebSocket handlers that expose these capabilities to the frontend. package linear import "time" @@ -12,11 +11,10 @@ import "time" // and leaves room for OAuth in the future. const AuthMethodAPIKey = "api_key" -// LinearConfig is the workspace-scoped configuration for the Linear -// integration. The API key is stored separately in the encrypted secret store -// under the key returned by SecretKeyForWorkspace. +// LinearConfig is the install-wide configuration for the Linear integration. +// The API key is stored separately in the encrypted secret store under +// SecretKey. type LinearConfig struct { - WorkspaceID string `json:"workspaceId" db:"workspace_id"` AuthMethod string `json:"authMethod" db:"auth_method"` DefaultTeamKey string `json:"defaultTeamKey" db:"default_team_key"` HasSecret bool `json:"hasSecret" db:"-"` @@ -35,10 +33,9 @@ type LinearConfig struct { } // SetConfigRequest is the payload sent by the UI to create or update the -// workspace's Linear configuration. When Secret is empty on update, the -// existing secret is retained; when non-empty it replaces the stored value. +// Linear configuration. When Secret is empty on update, the existing secret +// is retained; when non-empty it replaces the stored value. type SetConfigRequest struct { - WorkspaceID string `json:"workspaceId"` AuthMethod string `json:"authMethod"` DefaultTeamKey string `json:"defaultTeamKey"` Secret string `json:"secret"` @@ -128,8 +125,13 @@ type SearchResult struct { NextPageToken string `json:"nextPageToken,omitempty"` } -// SecretKeyForWorkspace returns the secret-store key used for the Linear API -// key of a given workspace. Centralised so the service and store agree. -func SecretKeyForWorkspace(workspaceID string) string { +// SecretKey is the secret-store key used for the install-wide Linear API key. +// Centralised so the service, store and provider migration agree. +const SecretKey = "linear:singleton:token" + +// LegacySecretKeyForWorkspace returns the pre-singleton per-workspace secret +// key. Only used by the one-shot startup migration in provider.go to copy an +// existing token over to SecretKey. +func LegacySecretKeyForWorkspace(workspaceID string) string { return "linear:" + workspaceID + ":token" } diff --git a/apps/backend/internal/linear/poller.go b/apps/backend/internal/linear/poller.go index d6e0b540ab..f98d85edb9 100644 --- a/apps/backend/internal/linear/poller.go +++ b/apps/backend/internal/linear/poller.go @@ -46,10 +46,10 @@ func (p *Poller) Stop() { // interface into Service's public API. type svcProber struct{ svc *Service } -func (s svcProber) ListConfiguredWorkspaces(ctx context.Context) ([]string, error) { - return s.svc.Store().ListConfiguredWorkspaces(ctx) +func (s svcProber) HasConfig(ctx context.Context) (bool, error) { + return s.svc.Store().HasConfig(ctx) } -func (s svcProber) RecordAuthHealth(ctx context.Context, workspaceID string) { - s.svc.RecordAuthHealth(ctx, workspaceID) +func (s svcProber) RecordAuthHealth(ctx context.Context) { + s.svc.RecordAuthHealth(ctx) } diff --git a/apps/backend/internal/linear/poller_test.go b/apps/backend/internal/linear/poller_test.go index c91a621b69..3b60b2e290 100644 --- a/apps/backend/internal/linear/poller_test.go +++ b/apps/backend/internal/linear/poller_test.go @@ -36,33 +36,31 @@ func newPollerFixture(t *testing.T) *pollerFixture { return f } -// saveConfig persists a workspace config directly via the store + secret +// saveConfig persists the singleton config directly via the store + secret // fakes — bypassing Service.SetConfig avoids racing against its async probe. -func (f *pollerFixture) saveConfig(t *testing.T, workspaceID, secret string) { +func (f *pollerFixture) saveConfig(t *testing.T, secret string) { t.Helper() ctx := context.Background() if err := f.store.UpsertConfig(ctx, &LinearConfig{ - WorkspaceID: workspaceID, - AuthMethod: AuthMethodAPIKey, + AuthMethod: AuthMethodAPIKey, }); err != nil { - t.Fatalf("save config %s: %v", workspaceID, err) + t.Fatalf("save config: %v", err) } - if err := f.secrets.Set(ctx, SecretKeyForWorkspace(workspaceID), - "linear", secret); err != nil { - t.Fatalf("save secret %s: %v", workspaceID, err) + if err := f.secrets.Set(ctx, SecretKey, "linear", secret); err != nil { + t.Fatalf("save secret: %v", err) } } func TestService_RecordAuthHealth_RecordsSuccess(t *testing.T) { f := newPollerFixture(t) - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") f.client.testAuthFn = func() (*TestConnectionResult, error) { return &TestConnectionResult{OK: true, OrgSlug: "acme"}, nil } - f.svc.RecordAuthHealth(context.Background(), "ws-1") + f.svc.RecordAuthHealth(context.Background()) - cfg, _ := f.store.GetConfig(context.Background(), "ws-1") + cfg, _ := f.store.GetConfig(context.Background()) if cfg == nil { t.Fatal("config disappeared") } @@ -76,14 +74,14 @@ func TestService_RecordAuthHealth_RecordsSuccess(t *testing.T) { func TestService_RecordAuthHealth_RecordsFailure(t *testing.T) { f := newPollerFixture(t) - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") f.client.testAuthFn = func() (*TestConnectionResult, error) { return &TestConnectionResult{OK: false, Error: "401 unauthorized"}, nil } - f.svc.RecordAuthHealth(context.Background(), "ws-1") + f.svc.RecordAuthHealth(context.Background()) - cfg, _ := f.store.GetConfig(context.Background(), "ws-1") + cfg, _ := f.store.GetConfig(context.Background()) if cfg.LastOk { t.Error("expected LastOk=false after failed probe") } @@ -94,35 +92,33 @@ func TestService_RecordAuthHealth_RecordsFailure(t *testing.T) { func TestService_RecordAuthHealth_ClientError(t *testing.T) { f := newPollerFixture(t) - f.saveConfig(t, "ws-1", "tok") + f.saveConfig(t, "tok") f.client.testAuthFn = func() (*TestConnectionResult, error) { return nil, errors.New("network timeout") } - f.svc.RecordAuthHealth(context.Background(), "ws-1") + f.svc.RecordAuthHealth(context.Background()) - cfg, _ := f.store.GetConfig(context.Background(), "ws-1") + cfg, _ := f.store.GetConfig(context.Background()) if cfg.LastOk { t.Error("expected LastOk=false on client error") } } -func TestPoller_Start_ProbesEachConfiguredWorkspace(t *testing.T) { - // Smoke test: confirms the prober adapter actually wires - // Service.Store().ListConfiguredWorkspaces → Service.RecordAuthHealth - // when the loop is started, end-to-end. Wrapped in synctest so the - // goroutine the poller spawns runs deterministically against fake time - // instead of relying on a wall-clock deadline. +func TestPoller_Start_ProbesWhenConfigured(t *testing.T) { + // Smoke test: confirms the prober adapter wires HasConfig → + // Service.RecordAuthHealth when the loop is started, end-to-end. Wrapped + // in synctest so the goroutine the poller spawns runs deterministically + // against fake time instead of relying on a wall-clock deadline. synctest.Test(t, func(t *testing.T) { f := newPollerFixture(t) - f.saveConfig(t, "ws-a", "tok-a") - f.saveConfig(t, "ws-b", "tok-b") - probed := map[string]bool{} + f.saveConfig(t, "tok") + var probed bool f.client.testAuthFn = func() (*TestConnectionResult, error) { return &TestConnectionResult{OK: true}, nil } - f.svc.SetProbeHook(func(workspaceID string) { - probed[workspaceID] = true + f.svc.SetProbeHook(func() { + probed = true }) ctx, cancel := context.WithCancel(context.Background()) @@ -133,8 +129,8 @@ func TestPoller_Start_ProbesEachConfiguredWorkspace(t *testing.T) { // Wait for the immediate-on-Start probe pass to finish. synctest.Wait() - if !probed["ws-a"] || !probed["ws-b"] { - t.Errorf("expected both ws-a and ws-b probed, got %v", probed) + if !probed { + t.Errorf("expected probe to fire when configured") } }) } diff --git a/apps/backend/internal/linear/provider.go b/apps/backend/internal/linear/provider.go index e9b091ca98..7eb4e9933f 100644 --- a/apps/backend/internal/linear/provider.go +++ b/apps/backend/internal/linear/provider.go @@ -1,10 +1,13 @@ package linear import ( + "context" "os" + "time" "github.com/gin-gonic/gin" "github.com/jmoiron/sqlx" + "go.uber.org/zap" "github.com/kandev/kandev/internal/common/logger" ) @@ -29,6 +32,7 @@ func Provide(writer, reader *sqlx.DB, secrets SecretStore, log *logger.Logger) ( if err != nil { return nil, nil, err } + migrateLegacySecret(store, secrets, log) clientFn := DefaultClientFactory var mock *MockClient if MockEnabled() { @@ -42,6 +46,34 @@ func Provide(writer, reader *sqlx.DB, secrets SecretStore, log *logger.Logger) ( return svc, cleanup, nil } +// migrateLegacySecret copies the per-workspace API key from the pre-singleton +// secret key onto the new install-wide key, then deletes the legacy entry. +// Best-effort: any error is logged and ignored — the worst case is the user +// re-pastes their key in the settings UI. +func migrateLegacySecret(store *Store, secrets SecretStore, log *logger.Logger) { + source := store.MigratedFromWorkspace() + if source == "" || secrets == nil { + return + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if exists, err := secrets.Exists(ctx, SecretKey); err == nil && exists { + return + } + legacyKey := LegacySecretKeyForWorkspace(source) + value, err := secrets.Reveal(ctx, legacyKey) + if err != nil || value == "" { + return + } + if err := secrets.Set(ctx, SecretKey, "Linear API key", value); err != nil { + log.Warn("linear: legacy secret migration failed", zap.Error(err)) + return + } + if err := secrets.Delete(ctx, legacyKey); err != nil { + log.Warn("linear: legacy secret cleanup failed", zap.Error(err)) + } +} + // RegisterMockRoutes mounts the mock control routes when the service was built // with a MockClient. No-op otherwise. func RegisterMockRoutes(router *gin.Engine, svc *Service, log *logger.Logger) { diff --git a/apps/backend/internal/linear/service.go b/apps/backend/internal/linear/service.go index 31b2ef960b..b42b2df417 100644 --- a/apps/backend/internal/linear/service.go +++ b/apps/backend/internal/linear/service.go @@ -20,16 +20,16 @@ type SecretStore interface { Exists(ctx context.Context, id string) (bool, error) } -// Service orchestrates Linear config storage, the per-workspace client cache, -// and the fetch/transition operations used by the WebSocket + HTTP handlers. +// Service orchestrates Linear config storage, the cached client, and the +// fetch/transition operations used by the WebSocket + HTTP handlers. type Service struct { store *Store secrets SecretStore log *logger.Logger mu sync.Mutex clientFn ClientFactory - cache map[string]Client // workspaceID → client, cleared on config change. - probeHook func(workspaceID string) + client Client // singleton, cleared on config change. + probeHook func() // 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. @@ -61,23 +61,21 @@ func NewService(store *Store, secrets SecretStore, clientFn ClientFactory, log * secrets: secrets, log: log, clientFn: clientFn, - cache: make(map[string]Client), } } -// GetConfig returns the workspace config enriched with a HasSecret flag. -func (s *Service) GetConfig(ctx context.Context, workspaceID string) (*LinearConfig, error) { - cfg, err := s.store.GetConfig(ctx, workspaceID) +// GetConfig returns the singleton config enriched with a HasSecret flag. +func (s *Service) GetConfig(ctx context.Context) (*LinearConfig, error) { + cfg, err := s.store.GetConfig(ctx) if err != nil || cfg == nil { return cfg, err } if s.secrets == nil { return cfg, nil } - exists, existsErr := s.secrets.Exists(ctx, SecretKeyForWorkspace(workspaceID)) + exists, existsErr := s.secrets.Exists(ctx, SecretKey) if existsErr != nil { - s.log.Warn("linear: secret exists check failed", - zap.String("workspace_id", workspaceID), zap.Error(existsErr)) + s.log.Warn("linear: secret exists check failed", zap.Error(existsErr)) } cfg.HasSecret = exists return cfg, nil @@ -92,7 +90,6 @@ func (s *Service) SetConfig(ctx context.Context, req *SetConfigRequest) (*Linear return nil, fmt.Errorf("%w: %s", ErrInvalidConfig, err.Error()) } cfg := &LinearConfig{ - WorkspaceID: req.WorkspaceID, AuthMethod: req.AuthMethod, DefaultTeamKey: req.DefaultTeamKey, } @@ -100,34 +97,29 @@ func (s *Service) SetConfig(ctx context.Context, req *SetConfigRequest) (*Linear return nil, fmt.Errorf("upsert linear config: %w", err) } if req.Secret != "" && s.secrets != nil { - if err := s.secrets.Set(ctx, - SecretKeyForWorkspace(req.WorkspaceID), - "Linear API key ("+req.WorkspaceID+")", - req.Secret, - ); err != nil { + if err := s.secrets.Set(ctx, SecretKey, "Linear API key", req.Secret); err != nil { return nil, fmt.Errorf("store linear secret: %w", err) } } - s.invalidateClient(req.WorkspaceID) + s.invalidateClient() // Probe asynchronously so a slow Linear doesn't stall the save response. - go func(workspaceID string) { - s.RecordAuthHealth(context.Background(), workspaceID) - }(req.WorkspaceID) - return s.GetConfig(ctx, req.WorkspaceID) + go func() { + s.RecordAuthHealth(context.Background()) + }() + return s.GetConfig(ctx) } // DeleteConfig removes both the config row and the stored secret. -func (s *Service) DeleteConfig(ctx context.Context, workspaceID string) error { - if err := s.store.DeleteConfig(ctx, workspaceID); err != nil { +func (s *Service) DeleteConfig(ctx context.Context) error { + if err := s.store.DeleteConfig(ctx); err != nil { return err } if s.secrets != nil { - if err := s.secrets.Delete(ctx, SecretKeyForWorkspace(workspaceID)); err != nil { - s.log.Warn("linear: secret delete failed", - zap.String("workspace_id", workspaceID), zap.Error(err)) + if err := s.secrets.Delete(ctx, SecretKey); err != nil { + s.log.Warn("linear: secret delete failed", zap.Error(err)) } } - s.invalidateClient(workspaceID) + s.invalidateClient() return nil } @@ -142,9 +134,9 @@ func (s *Service) TestConnection(ctx context.Context, req *SetConfigRequest) (*T return client.TestAuth(ctx) } -// ProbeAuth validates the stored credentials for a workspace. -func (s *Service) ProbeAuth(ctx context.Context, workspaceID string) (*TestConnectionResult, error) { - client, err := s.clientFor(ctx, workspaceID) +// ProbeAuth validates the stored credentials. +func (s *Service) ProbeAuth(ctx context.Context) (*TestConnectionResult, error) { + client, err := s.clientFor(ctx) if err != nil { return &TestConnectionResult{OK: false, Error: err.Error()}, nil } @@ -165,17 +157,17 @@ const authHealthWriteTimeout = 5 * time.Second // SetProbeHook installs a callback fired at the end of each RecordAuthHealth // call. Production code never sets this; tests use it to synchronise on probe // completion without sleep-polling. -func (s *Service) SetProbeHook(fn func(workspaceID string)) { +func (s *Service) SetProbeHook(fn func()) { s.mu.Lock() defer s.mu.Unlock() s.probeHook = fn } // RecordAuthHealth probes credentials and writes the outcome onto the row. -func (s *Service) RecordAuthHealth(ctx context.Context, workspaceID string) { +func (s *Service) RecordAuthHealth(ctx context.Context) { probeCtx, cancel := context.WithTimeout(ctx, authProbeTimeout) defer cancel() - res, err := s.ProbeAuth(probeCtx, workspaceID) + res, err := s.ProbeAuth(probeCtx) ok := err == nil && res != nil && res.OK errMsg := "" switch { @@ -192,21 +184,20 @@ func (s *Service) RecordAuthHealth(ctx context.Context, workspaceID string) { // still record the failure. writeCtx, writeCancel := context.WithTimeout(context.Background(), authHealthWriteTimeout) defer writeCancel() - if updateErr := s.store.UpdateAuthHealth(writeCtx, workspaceID, ok, errMsg, orgSlug, time.Now().UTC()); updateErr != nil { - s.log.Warn("linear: update auth health failed", - zap.String("workspace_id", workspaceID), zap.Error(updateErr)) + if updateErr := s.store.UpdateAuthHealth(writeCtx, ok, errMsg, orgSlug, time.Now().UTC()); updateErr != nil { + s.log.Warn("linear: update auth health failed", zap.Error(updateErr)) } s.mu.Lock() hook := s.probeHook s.mu.Unlock() if hook != nil { - hook(workspaceID) + hook() } } // GetIssue loads a Linear issue by identifier (e.g. "ENG-123"). -func (s *Service) GetIssue(ctx context.Context, workspaceID, identifier string) (*LinearIssue, error) { - client, err := s.clientFor(ctx, workspaceID) +func (s *Service) GetIssue(ctx context.Context, identifier string) (*LinearIssue, error) { + client, err := s.clientFor(ctx) if err != nil { return nil, err } @@ -214,8 +205,8 @@ func (s *Service) GetIssue(ctx context.Context, workspaceID, identifier string) } // SetIssueState moves an issue into the requested workflow state. -func (s *Service) SetIssueState(ctx context.Context, workspaceID, issueID, stateID string) error { - client, err := s.clientFor(ctx, workspaceID) +func (s *Service) SetIssueState(ctx context.Context, issueID, stateID string) error { + client, err := s.clientFor(ctx) if err != nil { return err } @@ -223,8 +214,8 @@ func (s *Service) SetIssueState(ctx context.Context, workspaceID, issueID, state } // ListTeams populates the team selector on the settings page. -func (s *Service) ListTeams(ctx context.Context, workspaceID string) ([]LinearTeam, error) { - client, err := s.clientFor(ctx, workspaceID) +func (s *Service) ListTeams(ctx context.Context) ([]LinearTeam, error) { + client, err := s.clientFor(ctx) if err != nil { return nil, err } @@ -232,35 +223,34 @@ func (s *Service) ListTeams(ctx context.Context, workspaceID string) ([]LinearTe } // ListStates returns the workflow states for a team identified by its key. -// Mirrors the other Service methods so handlers never reach into clientFor -// directly. -func (s *Service) ListStates(ctx context.Context, workspaceID, teamKey string) ([]LinearWorkflowState, error) { - client, err := s.clientFor(ctx, workspaceID) +func (s *Service) ListStates(ctx context.Context, teamKey string) ([]LinearWorkflowState, error) { + client, err := s.clientFor(ctx) if err != nil { return nil, err } return client.ListStates(ctx, teamKey) } -// SearchIssues runs a filtered search for the workspace. -func (s *Service) SearchIssues(ctx context.Context, workspaceID string, filter SearchFilter, pageToken string, maxResults int) (*SearchResult, error) { - client, err := s.clientFor(ctx, workspaceID) +// SearchIssues runs a filtered search. +func (s *Service) SearchIssues(ctx context.Context, filter SearchFilter, pageToken string, maxResults int) (*SearchResult, error) { + client, err := s.clientFor(ctx) if err != nil { return nil, err } return client.SearchIssues(ctx, filter, pageToken, maxResults) } -// clientFor returns a cached client, creating one if needed. -func (s *Service) clientFor(ctx context.Context, workspaceID string) (Client, error) { +// clientFor returns the cached client, creating it if needed. +func (s *Service) clientFor(ctx context.Context) (Client, error) { s.mu.Lock() - if c, ok := s.cache[workspaceID]; ok { + if s.client != nil { + c := s.client s.mu.Unlock() return c, nil } s.mu.Unlock() - cfg, err := s.store.GetConfig(ctx, workspaceID) + cfg, err := s.store.GetConfig(ctx) if err != nil { return nil, err } @@ -269,7 +259,7 @@ func (s *Service) clientFor(ctx context.Context, workspaceID string) (Client, er } secret := "" if s.secrets != nil { - secret, err = s.secrets.Reveal(ctx, SecretKeyForWorkspace(workspaceID)) + secret, err = s.secrets.Reveal(ctx, SecretKey) if err != nil { return nil, fmt.Errorf("read linear secret: %w", err) } @@ -280,17 +270,17 @@ func (s *Service) clientFor(ctx context.Context, workspaceID string) (Client, er client := s.clientFn(cfg, secret) s.mu.Lock() defer s.mu.Unlock() - if existing, ok := s.cache[workspaceID]; ok { - return existing, nil + if s.client != nil { + return s.client, nil } - s.cache[workspaceID] = client + s.client = client return client, nil } -// invalidateClient drops a cached client so the next request rebuilds it. -func (s *Service) invalidateClient(workspaceID string) { +// invalidateClient drops the cached client so the next request rebuilds it. +func (s *Service) invalidateClient() { s.mu.Lock() - delete(s.cache, workspaceID) + s.client = nil s.mu.Unlock() } @@ -298,8 +288,7 @@ func (s *Service) invalidateClient(workspaceID string) { // carries a secret, otherwise the stored secret. func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) (*LinearConfig, string, error) { cfg := &LinearConfig{ - WorkspaceID: req.WorkspaceID, - AuthMethod: req.AuthMethod, + AuthMethod: req.AuthMethod, } if req.Secret != "" { return cfg, req.Secret, nil @@ -307,21 +296,19 @@ func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) if s.secrets == nil { return nil, "", errors.New("no secret store configured") } - secret, err := s.secrets.Reveal(ctx, SecretKeyForWorkspace(req.WorkspaceID)) + secret, err := s.secrets.Reveal(ctx, SecretKey) if err != nil { - s.log.Warn("linear: secret reveal failed", - zap.String("workspace_id", req.WorkspaceID), zap.Error(err)) + s.log.Warn("linear: secret reveal failed", zap.Error(err)) return nil, "", fmt.Errorf("read linear secret: %w", err) } if secret == "" { return nil, "", errors.New("no api key stored — paste one to test") } - stored, storeErr := s.store.GetConfig(ctx, req.WorkspaceID) + stored, storeErr := s.store.GetConfig(ctx) if storeErr != nil { // Soft-fail: a transient DB error here only loses the saved-config // fallback values; the inline credentials still work for the test. - s.log.Warn("linear: load stored config for credential resolution failed", - zap.String("workspace_id", req.WorkspaceID), zap.Error(storeErr)) + s.log.Warn("linear: load stored config for credential resolution failed", zap.Error(storeErr)) } if stored != nil && cfg.AuthMethod == "" { cfg.AuthMethod = stored.AuthMethod @@ -330,9 +317,6 @@ func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) } func validateConfigRequest(req *SetConfigRequest) error { - if req.WorkspaceID == "" { - return errors.New("workspaceId required") - } if req.AuthMethod == "" { req.AuthMethod = AuthMethodAPIKey } diff --git a/apps/backend/internal/linear/service_test.go b/apps/backend/internal/linear/service_test.go index 426d53701b..387b5c2d57 100644 --- a/apps/backend/internal/linear/service_test.go +++ b/apps/backend/internal/linear/service_test.go @@ -112,7 +112,7 @@ type svcFixture struct { secrets *fakeSecretStore client *fakeClient factoryHit atomic.Int32 - probed chan string + probed chan struct{} } func newSvcFixture(t *testing.T) *svcFixture { @@ -121,15 +121,15 @@ func newSvcFixture(t *testing.T) *svcFixture { store: newTestStore(t), secrets: newFakeSecretStore(), client: &fakeClient{}, - probed: make(chan string, 8), + probed: make(chan struct{}, 8), } f.svc = NewService(f.store, f.secrets, func(_ *LinearConfig, _ string) Client { f.factoryHit.Add(1) return f.client }, logger.Default()) - f.svc.SetProbeHook(func(workspaceID string) { + f.svc.SetProbeHook(func() { select { - case f.probed <- workspaceID: + case f.probed <- struct{}{}: default: } }) @@ -137,23 +137,18 @@ func newSvcFixture(t *testing.T) *svcFixture { } // waitForAuthProbe blocks until the async probe has fired. -func waitForAuthProbe(t *testing.T, f *svcFixture, workspaceID string) *LinearConfig { +func waitForAuthProbe(t *testing.T, f *svcFixture) *LinearConfig { t.Helper() - for { - select { - case got := <-f.probed: - if got != workspaceID { - continue - } - cfg, err := f.svc.GetConfig(context.Background(), workspaceID) - if err != nil { - t.Fatalf("get config after probe: %v", err) - } - return cfg - case <-time.After(2 * time.Second): - t.Fatalf("async probe hook did not fire for %q within 2s", workspaceID) - return nil + select { + case <-f.probed: + cfg, err := f.svc.GetConfig(context.Background()) + if err != nil { + t.Fatalf("get config after probe: %v", err) } + return cfg + case <-time.After(2 * time.Second): + t.Fatalf("async probe hook did not fire within 2s") + return nil } } @@ -162,7 +157,6 @@ func TestService_SetConfig_UpsertsAndStoresSecret(t *testing.T) { ctx := context.Background() cfg, err := f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, DefaultTeamKey: "ENG", Secret: "lin_api_xyz", @@ -176,7 +170,7 @@ func TestService_SetConfig_UpsertsAndStoresSecret(t *testing.T) { if !cfg.HasSecret { t.Error("expected HasSecret=true") } - if got, _ := f.secrets.Reveal(ctx, SecretKeyForWorkspace("ws-1")); got != "lin_api_xyz" { + if got, _ := f.secrets.Reveal(ctx, SecretKey); got != "lin_api_xyz" { t.Errorf("secret stored = %q", got) } } @@ -187,11 +181,11 @@ func TestService_SetConfig_ProbesAuthImmediately(t *testing.T) { return &TestConnectionResult{OK: true, DisplayName: "Alice", OrgSlug: "acme"}, nil } if _, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, Secret: "tok", + AuthMethod: AuthMethodAPIKey, Secret: "tok", }); err != nil { t.Fatalf("set: %v", err) } - cfg := waitForAuthProbe(t, f, "ws-1") + cfg := waitForAuthProbe(t, f) if !cfg.LastOk { t.Errorf("expected LastOk=true after async probe, got %+v", cfg) } @@ -206,11 +200,11 @@ func TestService_SetConfig_PersistsProbeFailure(t *testing.T) { return &TestConnectionResult{OK: false, Error: "401 unauthorized"}, nil } if _, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, Secret: "bad", + AuthMethod: AuthMethodAPIKey, Secret: "bad", }); err != nil { t.Fatalf("set: %v", err) } - cfg := waitForAuthProbe(t, f, "ws-1") + cfg := waitForAuthProbe(t, f) if cfg.LastOk { t.Error("expected LastOk=false after failed probe") } @@ -223,16 +217,16 @@ func TestService_SetConfig_EmptySecret_KeepsExisting(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() if _, err := f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, Secret: "first", + AuthMethod: AuthMethodAPIKey, Secret: "first", }); err != nil { t.Fatalf("initial: %v", err) } if _, err := f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, DefaultTeamKey: "MOB", + AuthMethod: AuthMethodAPIKey, DefaultTeamKey: "MOB", }); err != nil { t.Fatalf("update: %v", err) } - if got, _ := f.secrets.Reveal(ctx, SecretKeyForWorkspace("ws-1")); got != "first" { + if got, _ := f.secrets.Reveal(ctx, SecretKey); got != "first" { t.Errorf("secret should be preserved, got %q", got) } } @@ -241,24 +235,24 @@ func TestService_SetConfig_InvalidatesClientCache(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() _, _ = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, Secret: "t", + AuthMethod: AuthMethodAPIKey, Secret: "t", }) - waitForAuthProbe(t, f, "ws-1") - if _, err := f.svc.GetIssue(ctx, "ws-1", "A-1"); err != nil { + waitForAuthProbe(t, f) + if _, err := f.svc.GetIssue(ctx, "A-1"); err != nil { t.Fatalf("get1: %v", err) } hits := f.factoryHit.Load() - if _, err := f.svc.GetIssue(ctx, "ws-1", "A-2"); err != nil { + if _, err := f.svc.GetIssue(ctx, "A-2"); err != nil { t.Fatalf("get2: %v", err) } if got := f.factoryHit.Load(); got != hits { t.Errorf("factory should be cached, hits %d→%d", hits, got) } _, _ = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, Secret: "t2", + AuthMethod: AuthMethodAPIKey, Secret: "t2", }) - waitForAuthProbe(t, f, "ws-1") - if _, err := f.svc.GetIssue(ctx, "ws-1", "A-3"); err != nil { + waitForAuthProbe(t, f) + if _, err := f.svc.GetIssue(ctx, "A-3"); err != nil { t.Fatalf("get3: %v", err) } if got := f.factoryHit.Load(); got <= hits { @@ -266,22 +260,12 @@ func TestService_SetConfig_InvalidatesClientCache(t *testing.T) { } } -func TestService_Validation(t *testing.T) { +func TestService_Validation_RejectsBadAuth(t *testing.T) { f := newSvcFixture(t) - ctx := context.Background() - cases := []struct { - name string - req SetConfigRequest - }{ - {"missing ws", SetConfigRequest{AuthMethod: AuthMethodAPIKey}}, - {"bad auth", SetConfigRequest{WorkspaceID: "w", AuthMethod: "bogus"}}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - if _, err := f.svc.SetConfig(ctx, &tc.req); err == nil { - t.Error("expected validation error") - } - }) + if _, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ + AuthMethod: "bogus", + }); err == nil { + t.Error("expected validation error for unknown auth method") } } @@ -289,7 +273,7 @@ func TestService_Validation_DefaultsAuthMethod(t *testing.T) { f := newSvcFixture(t) // Empty AuthMethod is filled with AuthMethodAPIKey rather than rejected. if _, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-1", Secret: "tok", + Secret: "tok", }); err != nil { t.Fatalf("expected default auth method, got error: %v", err) } @@ -303,7 +287,7 @@ func TestService_TestConnection_InlineSecret(t *testing.T) { return &TestConnectionResult{OK: true, DisplayName: "Alice"}, nil } res, err := f.svc.TestConnection(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, Secret: "inline", + AuthMethod: AuthMethodAPIKey, Secret: "inline", }) if err != nil || !called { t.Fatalf("called=%v err=%v", called, err) @@ -316,7 +300,7 @@ func TestService_TestConnection_InlineSecret(t *testing.T) { func TestService_TestConnection_NoStoredSecret_ReturnsFailure(t *testing.T) { f := newSvcFixture(t) res, err := f.svc.TestConnection(context.Background(), &SetConfigRequest{ - WorkspaceID: "ws-nope", AuthMethod: AuthMethodAPIKey, + AuthMethod: AuthMethodAPIKey, }) if err != nil { t.Fatalf("unexpected err: %v", err) @@ -328,7 +312,7 @@ func TestService_TestConnection_NoStoredSecret_ReturnsFailure(t *testing.T) { func TestService_GetIssue_NotConfigured(t *testing.T) { f := newSvcFixture(t) - _, err := f.svc.GetIssue(context.Background(), "missing", "ENG-1") + _, err := f.svc.GetIssue(context.Background(), "ENG-1") if !errors.Is(err, ErrNotConfigured) { t.Errorf("expected ErrNotConfigured, got %v", err) } @@ -338,16 +322,16 @@ func TestService_DeleteConfig_RemovesSecretAndCache(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() _, _ = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, Secret: "t", + AuthMethod: AuthMethodAPIKey, Secret: "t", }) - waitForAuthProbe(t, f, "ws-1") - if err := f.svc.DeleteConfig(ctx, "ws-1"); err != nil { + waitForAuthProbe(t, f) + if err := f.svc.DeleteConfig(ctx); err != nil { t.Fatalf("delete: %v", err) } - if exists, _ := f.secrets.Exists(ctx, SecretKeyForWorkspace("ws-1")); exists { + if exists, _ := f.secrets.Exists(ctx, SecretKey); exists { t.Error("expected secret removed") } - cfg, _ := f.svc.GetConfig(ctx, "ws-1") + cfg, _ := f.svc.GetConfig(ctx) if cfg != nil { t.Errorf("expected config gone, got %+v", cfg) } @@ -357,10 +341,10 @@ func TestService_SetIssueState_Forwards(t *testing.T) { f := newSvcFixture(t) ctx := context.Background() _, _ = f.svc.SetConfig(ctx, &SetConfigRequest{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, Secret: "t", + AuthMethod: AuthMethodAPIKey, Secret: "t", }) - waitForAuthProbe(t, f, "ws-1") - if err := f.svc.SetIssueState(ctx, "ws-1", "ENG-1", "state-id"); err != nil { + waitForAuthProbe(t, f) + if err := f.svc.SetIssueState(ctx, "ENG-1", "state-id"); err != nil { t.Fatalf("set state: %v", err) } if len(f.client.transitionLog) != 1 || f.client.transitionLog[0] != "ENG-1:state-id" { diff --git a/apps/backend/internal/linear/store.go b/apps/backend/internal/linear/store.go index cd1951f0e6..3ace6e0ed4 100644 --- a/apps/backend/internal/linear/store.go +++ b/apps/backend/internal/linear/store.go @@ -10,11 +10,17 @@ import ( "github.com/jmoiron/sqlx" ) -// Store persists Linear workspace configurations. Secret values are delegated -// to the shared encrypted secret store and not stored here. +// Store persists the install-wide Linear configuration. The secret API key is +// delegated to the shared encrypted secret store and not stored here. type Store struct { db *sqlx.DB ro *sqlx.DB + + // migratedFromWorkspace records the workspace_id of the row that was + // promoted into the singleton during initSchema. Provider reads this to + // migrate the per-workspace secret to the new global key. Empty when no + // migration ran. + migratedFromWorkspace string } // NewStore creates a new Store and initializes the schema if needed. @@ -26,9 +32,16 @@ func NewStore(writer, reader *sqlx.DB) (*Store, error) { return s, nil } +// MigratedFromWorkspace returns the workspace_id of the row promoted to the +// singleton during the per-workspace → singleton schema migration, or "" when +// no migration ran. +func (s *Store) MigratedFromWorkspace() string { + return s.migratedFromWorkspace +} + const createTablesSQL = ` CREATE TABLE IF NOT EXISTS linear_configs ( - workspace_id TEXT PRIMARY KEY, + id TEXT PRIMARY KEY CHECK(id = 'singleton'), auth_method TEXT NOT NULL, default_team_key TEXT NOT NULL DEFAULT '', org_slug TEXT NOT NULL DEFAULT '', @@ -40,42 +53,125 @@ const createTablesSQL = ` ); ` -// addedColumns lists columns introduced after the initial schema. Kept for -// parity with the Jira store so future column additions follow the same -// migration pattern; empty for now since the table ships fully-formed. -var addedColumns = []struct { - name string - sql string -}{} +// singletonID is the synthetic primary key of the (only) row in linear_configs. +const singletonID = "singleton" func (s *Store) initSchema() error { + if err := s.migrateLegacyPerWorkspaceTable(); err != nil { + return err + } if _, err := s.db.Exec(createTablesSQL); err != nil { return err } - return s.migrateAddedColumns() + return nil } -// migrateAddedColumns applies ALTER TABLE statements for columns introduced -// after the initial schema. Existing databases need these columns backfilled; -// new databases already have them from createTablesSQL and the ALTERs are -// skipped. -func (s *Store) migrateAddedColumns() error { - if len(addedColumns) == 0 { - return nil - } - existing, err := s.tableColumns("linear_configs") +// migrateLegacyPerWorkspaceTable detects the pre-singleton schema (where +// linear_configs was keyed by workspace_id) and rewrites it into the singleton +// shape. Picks the most-recently-updated row and records the source +// workspace_id so the provider can migrate the secret. +func (s *Store) migrateLegacyPerWorkspaceTable() error { + cols, err := s.tableColumns("linear_configs") if err != nil { return err } - for _, col := range addedColumns { - if _, ok := existing[col.name]; ok { - continue + if len(cols) == 0 { + return nil + } + if _, hasWorkspace := cols["workspace_id"]; !hasWorkspace { + return nil + } + tx, err := s.db.Begin() + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + + // `org_slug`, `last_checked_at`, `last_ok`, and `last_error` were added to + // the legacy schema in later releases. A deployment that upgrades from the + // original schema would have a `workspace_id` column but not these — + // selecting them unconditionally would crash startup. Build the SELECT + // against only columns present in this database. Mirrors the Jira fix. + healthCols := healthColumnsPresent(cols) + _, hasOrgSlug := cols["org_slug"] + selectCols := "workspace_id, auth_method, default_team_key" + if hasOrgSlug { + selectCols += ", org_slug" + } else { + selectCols += ", '' AS org_slug" + } + if healthCols { + selectCols += ", last_checked_at, last_ok, last_error" + } else { + selectCols += ", NULL AS last_checked_at, 0 AS last_ok, '' AS last_error" + } + selectCols += ", created_at, updated_at" + var sourceWorkspace, authMethod, defaultTeamKey, orgSlug, lastError sql.NullString + var lastCheckedAt sql.NullTime + var lastOk sql.NullInt64 + var createdAt, updatedAt sql.NullTime + row := tx.QueryRow(`SELECT ` + selectCols + ` FROM linear_configs ORDER BY updated_at DESC LIMIT 1`) + switch err := row.Scan(&sourceWorkspace, &authMethod, &defaultTeamKey, &orgSlug, + &lastCheckedAt, &lastOk, &lastError, &createdAt, &updatedAt); { + case errors.Is(err, sql.ErrNoRows): + if _, err := tx.Exec(`DROP TABLE linear_configs`); err != nil { + return err } - if _, err := s.db.Exec(col.sql); err != nil { - return fmt.Errorf("add column %s: %w", col.name, err) + return tx.Commit() + case err != nil: + return err + } + if _, err := tx.Exec(`DROP TABLE linear_configs`); err != nil { + return err + } + if _, err := tx.Exec(` + CREATE TABLE linear_configs ( + id TEXT PRIMARY KEY CHECK(id = 'singleton'), + auth_method TEXT NOT NULL, + default_team_key TEXT NOT NULL DEFAULT '', + org_slug TEXT NOT NULL DEFAULT '', + last_checked_at DATETIME, + last_ok INTEGER NOT NULL DEFAULT 0, + last_error TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + return err + } + if _, err := tx.Exec(` + INSERT INTO linear_configs (id, auth_method, default_team_key, org_slug, + last_checked_at, last_ok, last_error, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, + singletonID, authMethod.String, defaultTeamKey.String, orgSlug.String, + nullableTime(lastCheckedAt), lastOk.Int64, lastError.String, + nullableTime(createdAt), nullableTime(updatedAt)); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return err + } + s.migratedFromWorkspace = sourceWorkspace.String + return nil +} + +// healthColumnsPresent reports whether the legacy linear_configs table has the +// auth-health columns that were added in a later release. When all three are +// missing we fall back to NULL/zero defaults rather than crashing on the +// SELECT. Mirrors the helper of the same name in jira/store.go. +func healthColumnsPresent(cols map[string]struct{}) bool { + for _, name := range []string{"last_checked_at", "last_ok", "last_error"} { + if _, ok := cols[name]; !ok { + return false } } - return nil + return true +} + +func nullableTime(t sql.NullTime) interface{} { + if !t.Valid { + return nil + } + return t.Time } func (s *Store) tableColumns(table string) (map[string]struct{}, error) { @@ -102,15 +198,14 @@ func (s *Store) tableColumns(table string) (map[string]struct{}, error) { return cols, rows.Err() } -const selectConfigColumns = `workspace_id, auth_method, default_team_key, org_slug, +const selectConfigColumns = `auth_method, default_team_key, org_slug, last_checked_at, last_ok, last_error, created_at, updated_at` -// GetConfig returns the Linear config for a workspace, or nil when no row -// exists. -func (s *Store) GetConfig(ctx context.Context, workspaceID string) (*LinearConfig, error) { +// GetConfig returns the singleton Linear config, or nil when no row exists. +func (s *Store) GetConfig(ctx context.Context) (*LinearConfig, error) { var cfg LinearConfig err := s.ro.GetContext(ctx, &cfg, - `SELECT `+selectConfigColumns+` FROM linear_configs WHERE workspace_id = ?`, workspaceID) + `SELECT `+selectConfigColumns+` FROM linear_configs WHERE id = ?`, singletonID) if errors.Is(err, sql.ErrNoRows) { return nil, nil } @@ -120,10 +215,9 @@ func (s *Store) GetConfig(ctx context.Context, workspaceID string) (*LinearConfi return &cfg, nil } -// UpsertConfig inserts or updates the config row for a workspace. It never -// touches the secret store — callers must persist the token separately. The -// last_* health columns and org_slug are deliberately not touched here; the -// poller owns those and writes them via UpdateAuthHealth. +// UpsertConfig inserts or updates the singleton config row. The last_* health +// columns and org_slug are deliberately not touched here; the poller owns +// those and writes them via UpdateAuthHealth. func (s *Store) UpsertConfig(ctx context.Context, cfg *LinearConfig) error { now := time.Now().UTC() if cfg.CreatedAt.IsZero() { @@ -131,52 +225,50 @@ func (s *Store) UpsertConfig(ctx context.Context, cfg *LinearConfig) error { } cfg.UpdatedAt = now _, err := s.db.ExecContext(ctx, ` - INSERT INTO linear_configs (workspace_id, auth_method, default_team_key, created_at, updated_at) + INSERT INTO linear_configs (id, auth_method, default_team_key, created_at, updated_at) VALUES (?, ?, ?, ?, ?) - ON CONFLICT(workspace_id) DO UPDATE SET + ON CONFLICT(id) DO UPDATE SET auth_method = excluded.auth_method, default_team_key = excluded.default_team_key, updated_at = excluded.updated_at`, - cfg.WorkspaceID, cfg.AuthMethod, cfg.DefaultTeamKey, cfg.CreatedAt, cfg.UpdatedAt) + singletonID, cfg.AuthMethod, cfg.DefaultTeamKey, cfg.CreatedAt, cfg.UpdatedAt) return err } -// DeleteConfig removes the Linear config row for a workspace. Secrets must be -// cleared separately by the caller. -func (s *Store) DeleteConfig(ctx context.Context, workspaceID string) error { - _, err := s.db.ExecContext(ctx, `DELETE FROM linear_configs WHERE workspace_id = ?`, workspaceID) +// DeleteConfig removes the singleton config row. +func (s *Store) DeleteConfig(ctx context.Context) error { + _, err := s.db.ExecContext(ctx, `DELETE FROM linear_configs WHERE id = ?`, singletonID) return err } -// ListConfiguredWorkspaces returns the IDs of all workspaces that have a -// Linear config row. Used by the auth-health poller. -func (s *Store) ListConfiguredWorkspaces(ctx context.Context) ([]string, error) { - var ids []string - err := s.ro.SelectContext(ctx, &ids, - `SELECT workspace_id FROM linear_configs ORDER BY workspace_id`) +// HasConfig reports whether the singleton row exists. Used by the auth-health +// poller to decide whether to probe at all. +func (s *Store) HasConfig(ctx context.Context) (bool, error) { + var present int + err := s.ro.GetContext(ctx, &present, + `SELECT COUNT(*) FROM linear_configs WHERE id = ?`, singletonID) if err != nil { - return nil, err + return false, err } - return ids, nil + return present > 0, nil } // UpdateAuthHealth records the result of a credential probe. orgSlug is // captured opportunistically from successful probes; pass "" to leave the -// existing slug unchanged. If the workspace row no longer exists, the update -// is a silent no-op. -func (s *Store) UpdateAuthHealth(ctx context.Context, workspaceID string, ok bool, errMsg, orgSlug string, checkedAt time.Time) error { +// existing slug unchanged. +func (s *Store) UpdateAuthHealth(ctx context.Context, ok bool, errMsg, orgSlug string, checkedAt time.Time) error { if orgSlug != "" { _, err := s.db.ExecContext(ctx, ` UPDATE linear_configs SET last_checked_at = ?, last_ok = ?, last_error = ?, org_slug = ? - WHERE workspace_id = ?`, - checkedAt, ok, errMsg, orgSlug, workspaceID) + WHERE id = ?`, + checkedAt, ok, errMsg, orgSlug, singletonID) return err } _, err := s.db.ExecContext(ctx, ` UPDATE linear_configs SET last_checked_at = ?, last_ok = ?, last_error = ? - WHERE workspace_id = ?`, - checkedAt, ok, errMsg, workspaceID) + WHERE id = ?`, + checkedAt, ok, errMsg, singletonID) return err } diff --git a/apps/backend/internal/linear/store_test.go b/apps/backend/internal/linear/store_test.go index 35d33d5ac8..9929555a62 100644 --- a/apps/backend/internal/linear/store_test.go +++ b/apps/backend/internal/linear/store_test.go @@ -32,7 +32,6 @@ func TestStore_UpsertGetDelete(t *testing.T) { ctx := context.Background() cfg := &LinearConfig{ - WorkspaceID: "ws-1", AuthMethod: AuthMethodAPIKey, DefaultTeamKey: "ENG", } @@ -40,7 +39,7 @@ func TestStore_UpsertGetDelete(t *testing.T) { t.Fatalf("upsert: %v", err) } - got, err := store.GetConfig(ctx, "ws-1") + got, err := store.GetConfig(ctx) if err != nil { t.Fatalf("get: %v", err) } @@ -58,15 +57,15 @@ func TestStore_UpsertGetDelete(t *testing.T) { if err := store.UpsertConfig(ctx, cfg); err != nil { t.Fatalf("update upsert: %v", err) } - got2, _ := store.GetConfig(ctx, "ws-1") + got2, _ := store.GetConfig(ctx) if got2.DefaultTeamKey != "MOB" { t.Errorf("expected team update, got %q", got2.DefaultTeamKey) } - if err := store.DeleteConfig(ctx, "ws-1"); err != nil { + if err := store.DeleteConfig(ctx); err != nil { t.Fatalf("delete: %v", err) } - gone, err := store.GetConfig(ctx, "ws-1") + gone, err := store.GetConfig(ctx) if err != nil { t.Fatalf("get after delete: %v", err) } @@ -77,7 +76,7 @@ func TestStore_UpsertGetDelete(t *testing.T) { func TestStore_GetConfig_Missing(t *testing.T) { store := newTestStore(t) - cfg, err := store.GetConfig(context.Background(), "does-not-exist") + cfg, err := store.GetConfig(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -86,25 +85,27 @@ func TestStore_GetConfig_Missing(t *testing.T) { } } -func TestStore_ListConfiguredWorkspaces(t *testing.T) { +func TestStore_HasConfig(t *testing.T) { store := newTestStore(t) ctx := context.Background() - mustUpsert := func(id string) { - if err := store.UpsertConfig(ctx, &LinearConfig{ - WorkspaceID: id, - AuthMethod: AuthMethodAPIKey, - }); err != nil { - t.Fatalf("upsert %s: %v", id, err) - } - } - mustUpsert("ws-b") - mustUpsert("ws-a") - ids, err := store.ListConfiguredWorkspaces(ctx) + has, err := store.HasConfig(ctx) if err != nil { - t.Fatalf("list: %v", err) + t.Fatalf("has-config: %v", err) } - if len(ids) != 2 || ids[0] != "ws-a" || ids[1] != "ws-b" { - t.Errorf("expected sorted [ws-a ws-b], got %v", ids) + if has { + t.Errorf("expected HasConfig=false on empty store") + } + if err := store.UpsertConfig(ctx, &LinearConfig{ + AuthMethod: AuthMethodAPIKey, + }); err != nil { + t.Fatalf("upsert: %v", err) + } + has, err = store.HasConfig(ctx) + if err != nil { + t.Fatalf("has-config after upsert: %v", err) + } + if !has { + t.Errorf("expected HasConfig=true after upsert") } } @@ -112,22 +113,21 @@ func TestStore_UpdateAuthHealth(t *testing.T) { store := newTestStore(t) ctx := context.Background() if err := store.UpsertConfig(ctx, &LinearConfig{ - WorkspaceID: "ws-1", - AuthMethod: AuthMethodAPIKey, + AuthMethod: AuthMethodAPIKey, }); err != nil { t.Fatalf("upsert: %v", err) } - cfg, _ := store.GetConfig(ctx, "ws-1") + cfg, _ := store.GetConfig(ctx) if cfg.LastCheckedAt != nil { t.Errorf("expected nil last_checked_at on fresh row, got %v", cfg.LastCheckedAt) } now := time.Now().UTC().Truncate(time.Second) - if err := store.UpdateAuthHealth(ctx, "ws-1", true, "", "acme", now); err != nil { + if err := store.UpdateAuthHealth(ctx, true, "", "acme", now); err != nil { t.Fatalf("update ok: %v", err) } - cfg, _ = store.GetConfig(ctx, "ws-1") + cfg, _ = store.GetConfig(ctx) if !cfg.LastOk { t.Error("expected last_ok=true after successful probe") } @@ -140,10 +140,10 @@ func TestStore_UpdateAuthHealth(t *testing.T) { // Empty orgSlug should leave the existing slug intact. failAt := now.Add(time.Minute) - if err := store.UpdateAuthHealth(ctx, "ws-1", false, "401 unauthorized", "", failAt); err != nil { + if err := store.UpdateAuthHealth(ctx, false, "401 unauthorized", "", failAt); err != nil { t.Fatalf("update fail: %v", err) } - cfg, _ = store.GetConfig(ctx, "ws-1") + cfg, _ = store.GetConfig(ctx) if cfg.LastOk { t.Error("expected last_ok=false after failure") } @@ -153,9 +153,167 @@ func TestStore_UpdateAuthHealth(t *testing.T) { if cfg.OrgSlug != "acme" { t.Errorf("orgSlug should be preserved across failed probe, got %q", cfg.OrgSlug) } +} - // Update for missing workspace must not error. - if err := store.UpdateAuthHealth(ctx, "missing", true, "", "x", now); err != nil { - t.Errorf("expected no-op for missing row, got %v", err) +func TestStore_MigrateLegacyPerWorkspaceTable(t *testing.T) { + raw, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + raw.SetMaxOpenConns(1) + raw.SetMaxIdleConns(1) + db := sqlx.NewDb(raw, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + + if _, err := db.Exec(` + CREATE TABLE linear_configs ( + workspace_id TEXT PRIMARY KEY, + auth_method TEXT NOT NULL, + default_team_key TEXT NOT NULL DEFAULT '', + org_slug TEXT NOT NULL DEFAULT '', + last_checked_at DATETIME, + last_ok INTEGER NOT NULL DEFAULT 0, + last_error TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + t.Fatalf("legacy schema: %v", err) + } + older := time.Now().UTC().Add(-time.Hour) + newer := time.Now().UTC() + if _, err := db.Exec(`INSERT INTO linear_configs + (workspace_id, auth_method, default_team_key, org_slug, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?)`, + "ws-old", AuthMethodAPIKey, "OLD", "old-slug", older, older); err != nil { + t.Fatalf("seed old row: %v", err) + } + if _, err := db.Exec(`INSERT INTO linear_configs + (workspace_id, auth_method, default_team_key, org_slug, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?)`, + "ws-new", AuthMethodAPIKey, "NEW", "new-slug", newer, newer); err != nil { + t.Fatalf("seed new row: %v", err) + } + + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("new store: %v", err) + } + if got := store.MigratedFromWorkspace(); got != "ws-new" { + t.Errorf("expected migration source ws-new, got %q", got) + } + cfg, err := store.GetConfig(context.Background()) + if err != nil { + t.Fatalf("get singleton: %v", err) + } + if cfg == nil { + t.Fatal("expected singleton row after migration") + } + if cfg.DefaultTeamKey != "NEW" || cfg.OrgSlug != "new-slug" { + t.Errorf("expected newest row promoted, got %+v", cfg) + } +} + +// Covers the original-schema upgrade path: the legacy table exists with +// `workspace_id` but lacks the auth-health columns and `org_slug` added in +// later releases. The migration must select only guaranteed-present columns +// and fall back to defaults for the missing ones, otherwise startup crashes +// with "no such column". Mirrors the Jira test. +func TestStore_MigrateLegacyPerWorkspaceTable_PreHealthColumns(t *testing.T) { + raw, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + raw.SetMaxOpenConns(1) + raw.SetMaxIdleConns(1) + db := sqlx.NewDb(raw, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + + // Original schema: no org_slug / last_checked_at / last_ok / last_error. + if _, err := db.Exec(` + CREATE TABLE linear_configs ( + workspace_id TEXT PRIMARY KEY, + auth_method TEXT NOT NULL, + default_team_key TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + t.Fatalf("legacy schema: %v", err) + } + now := time.Now().UTC() + if _, err := db.Exec(`INSERT INTO linear_configs + (workspace_id, auth_method, default_team_key, created_at, updated_at) + VALUES (?, ?, ?, ?, ?)`, + "ws-1", AuthMethodAPIKey, "ENG", now, now); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("NewStore on pre-health-columns schema: %v", err) + } + if got := store.MigratedFromWorkspace(); got != "ws-1" { + t.Errorf("expected migration source ws-1, got %q", got) + } + cfg, err := store.GetConfig(context.Background()) + if err != nil { + t.Fatalf("get singleton: %v", err) + } + if cfg == nil { + t.Fatal("expected singleton row after migration") + } + if cfg.DefaultTeamKey != "ENG" { + t.Errorf("expected DefaultTeamKey preserved, got %q", cfg.DefaultTeamKey) + } + if cfg.OrgSlug != "" { + t.Errorf("expected OrgSlug='' (default) after migrating from pre-org-slug schema, got %q", cfg.OrgSlug) + } + if cfg.LastOk { + t.Error("expected LastOk=false (default) after migrating from pre-health schema") + } + if cfg.LastCheckedAt != nil { + t.Errorf("expected LastCheckedAt=nil (default), got %v", cfg.LastCheckedAt) + } +} + +// Covers the sql.ErrNoRows branch: a user who installed a previous version, +// never configured Linear, and then upgrades hits this path. +func TestStore_MigrateLegacyPerWorkspaceTable_EmptyTable(t *testing.T) { + raw, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + raw.SetMaxOpenConns(1) + raw.SetMaxIdleConns(1) + db := sqlx.NewDb(raw, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + + if _, err := db.Exec(` + CREATE TABLE linear_configs ( + workspace_id TEXT PRIMARY KEY, + auth_method TEXT NOT NULL, + default_team_key TEXT NOT NULL DEFAULT '', + org_slug TEXT NOT NULL DEFAULT '', + last_checked_at DATETIME, + last_ok INTEGER NOT NULL DEFAULT 0, + last_error TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + t.Fatalf("legacy schema: %v", err) + } + + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("NewStore on empty legacy table: %v", err) + } + if got := store.MigratedFromWorkspace(); got != "" { + t.Errorf("expected empty MigratedFromWorkspace, got %q", got) + } + cfg, err := store.GetConfig(context.Background()) + if err != nil { + t.Fatalf("get singleton: %v", err) + } + if cfg != nil { + t.Errorf("expected nil cfg on empty legacy table, got %+v", cfg) } } diff --git a/apps/web/app/jira/jira-page-client.tsx b/apps/web/app/jira/jira-page-client.tsx index 03088178da..78d370d935 100644 --- a/apps/web/app/jira/jira-page-client.tsx +++ b/apps/web/app/jira/jira-page-client.tsx @@ -35,16 +35,13 @@ type JiraPageClientProps = { steps: WorkflowStep[]; }; -function NotConfiguredNotice({ workspaceId }: { workspaceId?: string }) { +function NotConfiguredNotice() { return (
+ Connect Kandev to third-party services. Credentials are install-wide; per-workspace + watchers and presets live inside each integration page. +
+{description}
++ Connect Kandev to GitHub. Authentication is shared across all workspaces; PR/issue + watchers and presets are configured per workspace. +
+- Configure GitHub PR monitoring, issue tracking, and review queue automation. -
-