diff --git a/apps/backend/AGENTS.md b/apps/backend/AGENTS.md index 140c3e8638..d7489ed9a2 100644 --- a/apps/backend/AGENTS.md +++ b/apps/backend/AGENTS.md @@ -104,7 +104,8 @@ apps/backend/ │ │ ├── engine/ # Typed state-machine engine │ │ ├── models/ # Workflow step, template, and history models │ │ ├── repository/ # Workflow persistence (SQLite) -│ │ └── service/ # Workflow CRUD and step resolution +│ │ └── service/ # Workflow CRUD, step resolution, and sync apply +│ ├── workflowsync/ # GitHub workflow sync (per-workspace repo config, poller, force sync) │ └── worktree/ # Git worktree management for workspace isolation ``` diff --git a/apps/backend/internal/backendapp/e2e_reset.go b/apps/backend/internal/backendapp/e2e_reset.go index fd07f32cab..091c0d7acf 100644 --- a/apps/backend/internal/backendapp/e2e_reset.go +++ b/apps/backend/internal/backendapp/e2e_reset.go @@ -92,6 +92,15 @@ func handleE2EReset( if _, err := repo.DB().ExecContext(ctx, `DELETE FROM github_workspace_settings WHERE workspace_id = ?`, workspaceID); err != nil { log.Warn("e2e reset: GitHub workspace settings cleanup failed", zap.Error(err)) } + // The workflow-sync poller reads these rows globally; delete before + // task/workflow deletion so a mid-reset tick can't resync workflows. + // The table always exists, so a failure is a genuine DB error — abort + // rather than let the poller recreate workflows mid-reset. + if _, err := repo.DB().ExecContext(ctx, `DELETE FROM workflow_sync_configs WHERE workspace_id = ?`, workspaceID); err != nil { + log.Error("e2e reset: workflow sync config cleanup failed", zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{errKey: "workflow sync config cleanup failed"}) + return + } // Reset every agent's routing override to the inherit-markers // shape onboarding writes. Without this, an agent-override test diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index e02ce337c0..7c216b04f4 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -79,6 +79,7 @@ import ( webembedded "github.com/kandev/kandev/internal/webapp/embedded" workflowcontroller "github.com/kandev/kandev/internal/workflow/controller" workflowhandlers "github.com/kandev/kandev/internal/workflow/handlers" + "github.com/kandev/kandev/internal/workflowsync" "github.com/kandev/kandev/internal/worktree" ws "github.com/kandev/kandev/pkg/websocket" ) @@ -926,6 +927,11 @@ func registerSecondaryRoutes( p.log.Debug("Registered Slack handlers (HTTP + WebSocket)") } + if p.services.WorkflowSync != nil { + workflowsync.RegisterRoutes(p.router, p.services.WorkflowSync, p.log) + p.log.Debug("Registered workflow sync handlers (HTTP)") + } + if p.services.Automation != nil { automation.RegisterRoutes(p.router, p.gateway.Dispatcher, p.services.Automation.Service, p.log) p.log.Debug("Registered Automation handlers (HTTP + WebSocket)") diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index e587458324..7a808befe3 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -38,6 +38,7 @@ import ( linearpkg "github.com/kandev/kandev/internal/linear" sentrypkg "github.com/kandev/kandev/internal/sentry" slackpkg "github.com/kandev/kandev/internal/slack" + workflowsyncpkg "github.com/kandev/kandev/internal/workflowsync" // Agent infrastructure "github.com/kandev/kandev/internal/agent/hostutility" @@ -567,6 +568,16 @@ func startAgentInfrastructure( addCleanup(func() error { slackTrigger.Stop(); return nil }) } + // Start workflow-sync poller: periodically pulls workflow definition + // files from each workspace's configured GitHub repo and reconciles the + // workspace's synced workflows with them. + if services.WorkflowSync != nil { + workflowSyncPoller := workflowsyncpkg.NewPoller(services.WorkflowSync, log) + workflowSyncPoller.Start(ctx) + addCleanup(func() error { workflowSyncPoller.Stop(); return nil }) + log.Info("Workflow sync poller started") + } + // Wire automation service into orchestrator for trigger-based task creation. // The Automation subsystem is independent of the Office feature flag — it // has its own cron scheduler, GitHub poller, and webhook handler, and diff --git a/apps/backend/internal/backendapp/services.go b/apps/backend/internal/backendapp/services.go index ea65742b13..fed94744df 100644 --- a/apps/backend/internal/backendapp/services.go +++ b/apps/backend/internal/backendapp/services.go @@ -36,6 +36,7 @@ import ( utilityservice "github.com/kandev/kandev/internal/utility/service" wfmodels "github.com/kandev/kandev/internal/workflow/models" workflowservice "github.com/kandev/kandev/internal/workflow/service" + "github.com/kandev/kandev/internal/workflowsync" ) func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories, dbPool *db.Pool, eventBus bus.EventBus, agentRegistry *registry.Registry, version string) (*Services, *agentsettingscontroller.Controller, error) { @@ -106,6 +107,7 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories linearSvc := initLinearService(dbPool, eventBus, repos.Secrets, log) sentrySvc := initSentryService(dbPool, eventBus, repos.Secrets, log) slackSvc := initSlackService(dbPool, repos.Secrets, log) + workflowSyncSvc := initWorkflowSyncService(dbPool, githubSvc, workflowSvc, taskSvc, log) shareHTTP := initShareHandlers(dbPool, repos.Task, githubSvc, log, version) // Plumb GitHub branch listing into the task service so provider-backed @@ -127,20 +129,21 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories } return &Services{ - Task: taskSvc, - User: userSvc, - Editor: editorSvc, - Prompts: promptSvc, - Utility: utilitySvc, - Workflow: workflowSvc, - GitHub: githubSvc, - GitLab: gitlabSvc, - Jira: jiraSvc, - Linear: linearSvc, - Sentry: sentrySvc, - Slack: slackSvc, - Share: shareHTTP, - Automation: automationComponents, + Task: taskSvc, + User: userSvc, + Editor: editorSvc, + Prompts: promptSvc, + Utility: utilitySvc, + Workflow: workflowSvc, + GitHub: githubSvc, + GitLab: gitlabSvc, + Jira: jiraSvc, + Linear: linearSvc, + Sentry: sentrySvc, + Slack: slackSvc, + WorkflowSync: workflowSyncSvc, + Share: shareHTTP, + Automation: automationComponents, // Office is constructed later in initOfficeServices once all // of its dependencies (config loader, task integrations, etc.) are available. Office: nil, @@ -380,6 +383,22 @@ func initJiraService(dbPool *db.Pool, eventBus bus.EventBus, secretsStore secret return svc } +// initWorkflowSyncService wires the GitHub workflow-sync service. Failures +// are non-fatal; the service is nil when GitHub is unavailable. +func initWorkflowSyncService(dbPool *db.Pool, githubSvc *github.Service, workflowSvc *workflowservice.Service, taskSvc *taskservice.Service, log *logger.Logger) *workflowsync.Service { + if githubSvc == nil { + log.Warn("workflow sync disabled: GitHub service unavailable") + return nil + } + workflowSvc.SetSyncWorkflowOps(taskSvc) + svc, _, err := workflowsync.Provide(dbPool.Writer(), dbPool.Reader(), githubSvc, workflowSvc, log) + if err != nil { + log.Warn("workflow sync service initialization failed (non-fatal)", zap.Error(err)) + return nil + } + return svc +} + // initLinearService wires up the Linear integration. Failures are non-fatal. func initLinearService(dbPool *db.Pool, eventBus bus.EventBus, secretsStore secrets.SecretStore, log *logger.Logger) *linear.Service { svc, _, err := linear.Provide(dbPool.Writer(), dbPool.Reader(), secretadapter.New(secretsStore), eventBus, log) diff --git a/apps/backend/internal/backendapp/types.go b/apps/backend/internal/backendapp/types.go index 64eb0b8816..5a646c9b67 100644 --- a/apps/backend/internal/backendapp/types.go +++ b/apps/backend/internal/backendapp/types.go @@ -32,6 +32,7 @@ import ( utilitystore "github.com/kandev/kandev/internal/utility/store" workflowrepository "github.com/kandev/kandev/internal/workflow/repository" workflowservice "github.com/kandev/kandev/internal/workflow/service" + "github.com/kandev/kandev/internal/workflowsync" "github.com/kandev/kandev/internal/worktree" ) @@ -65,6 +66,9 @@ type Services struct { Linear *linear.Service Sentry *sentry.Service Slack *slack.Service + // WorkflowSync keeps workspace workflows in sync with definition files + // in a configured GitHub repository. Nil when GitHub is unavailable. + WorkflowSync *workflowsync.Service Share *share.HTTPHandlers Office *officeservice.Service OfficeSvcs *office.Services diff --git a/apps/backend/internal/github/client.go b/apps/backend/internal/github/client.go index 2094c10afe..5e686f1658 100644 --- a/apps/backend/internal/github/client.go +++ b/apps/backend/internal/github/client.go @@ -113,4 +113,14 @@ type Client interface { // DeleteGist deletes a gist by ID. A 404 is wrapped in *GitHubAPIError // so callers can distinguish "already gone" from transport failures. DeleteGist(ctx context.Context, gistID string) error + + // ListRepoDirectory lists the entries of a directory in a repository at the + // given ref (branch, tag, or SHA). dir may be "" or "." for the repo root. + // A missing directory returns a *GitHubAPIError with status 404. + ListRepoDirectory(ctx context.Context, owner, repo, dir, ref string) ([]RepoContentEntry, error) + + // GetRepoFileContent fetches the raw decoded content of a single file in a + // repository at the given ref. A missing file returns a *GitHubAPIError with + // status 404. + GetRepoFileContent(ctx context.Context, owner, repo, path, ref string) ([]byte, error) } diff --git a/apps/backend/internal/github/controller_test.go b/apps/backend/internal/github/controller_test.go index b7746ef31d..7faefddb3b 100644 --- a/apps/backend/internal/github/controller_test.go +++ b/apps/backend/internal/github/controller_test.go @@ -123,6 +123,12 @@ func (s *stubClient) CreateGist(context.Context, CreateGistInput) (*GistResponse return nil, nil } func (s *stubClient) DeleteGist(context.Context, string) error { return nil } +func (s *stubClient) ListRepoDirectory(context.Context, string, string, string, string) ([]RepoContentEntry, error) { + return nil, nil +} +func (s *stubClient) GetRepoFileContent(context.Context, string, string, string, string) ([]byte, error) { + return nil, nil +} func newControllerTestLogger() *logger.Logger { log, _ := logger.NewLogger(logger.LoggingConfig{ diff --git a/apps/backend/internal/github/gh_client.go b/apps/backend/internal/github/gh_client.go index 2cf0fa853e..e5167f60aa 100644 --- a/apps/backend/internal/github/gh_client.go +++ b/apps/backend/internal/github/gh_client.go @@ -3,6 +3,7 @@ package github import ( "bytes" "context" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -920,6 +921,75 @@ func (c *GHClient) DeleteGist(ctx context.Context, gistID string) error { return nil } +// ListRepoDirectory lists the entries of a directory in a repository at the +// given ref via `gh api repos/{owner}/{repo}/contents/{path}`. A 404 (missing +// directory) is promoted to a *GitHubAPIError, mirroring PATClient. +func (c *GHClient) ListRepoDirectory(ctx context.Context, owner, repo, dir, ref string) ([]RepoContentEntry, error) { + args := ghContentsArgs(owner, repo, dir, ref) + out, err := c.run(ctx, args...) + if err != nil { + if isNotFoundErr(err) { + return nil, &GitHubAPIError{ + StatusCode: http.StatusNotFound, + Endpoint: fmt.Sprintf("repos/%s/%s/contents/%s", owner, repo, repoContentsPath(dir)), + Body: err.Error(), + } + } + return nil, fmt.Errorf("list repo directory: %w", err) + } + var entries []RepoContentEntry + if err := json.Unmarshal([]byte(out), &entries); err != nil { + return nil, fmt.Errorf("list repo directory: decode: %w", err) + } + return entries, nil +} + +// GetRepoFileContent fetches the raw decoded content of a single file via +// `gh api repos/{owner}/{repo}/contents/{path}`. A 404 (missing file) is +// promoted to a *GitHubAPIError, mirroring PATClient. +func (c *GHClient) GetRepoFileContent(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { + args := ghContentsArgs(owner, repo, path, ref) + out, err := c.run(ctx, args...) + if err != nil { + if isNotFoundErr(err) { + return nil, &GitHubAPIError{ + StatusCode: http.StatusNotFound, + Endpoint: fmt.Sprintf("repos/%s/%s/contents/%s", owner, repo, repoContentsPath(path)), + Body: err.Error(), + } + } + return nil, fmt.Errorf("get repo file content: %w", err) + } + var raw ghRepoFileContent + if err := json.Unmarshal([]byte(out), &raw); err != nil { + return nil, fmt.Errorf("get repo file content: decode: %w", err) + } + if raw.Encoding != "base64" { + return nil, fmt.Errorf("get repo file content: unsupported encoding %q", raw.Encoding) + } + decoded, err := base64.StdEncoding.DecodeString(stripBase64Whitespace(raw.Content)) + if err != nil { + return nil, fmt.Errorf("get repo file content: decode base64: %w", err) + } + return decoded, nil +} + +// ghContentsArgs builds the `gh api` argv for the repo contents endpoint, +// shared by ListRepoDirectory and GetRepoFileContent. ref, when non-empty, is +// passed as a `-f` field (requiring the explicit `-X GET`, since `gh api` +// otherwise switches to POST once any `-f`/`-F` field is supplied). +func ghContentsArgs(owner, repo, path, ref string) []string { + endpoint := "repos/" + owner + "/" + repo + "/contents" + if p := repoContentsPath(path); p != "" { + endpoint += "/" + p + } + args := []string{"api", endpoint, "-X", "GET"} + if ref != "" { + args = append(args, "-f", "ref="+ref) + } + return args +} + const ghCLITimeout = 30 * time.Second // resolveGHExecTimeout returns the per-command exec budget used by diff --git a/apps/backend/internal/github/mock_client.go b/apps/backend/internal/github/mock_client.go index 52d2861677..fbadab1853 100644 --- a/apps/backend/internal/github/mock_client.go +++ b/apps/backend/internal/github/mock_client.go @@ -3,6 +3,7 @@ package github import ( "context" "fmt" + "sort" "strings" "sync" "sync/atomic" @@ -61,6 +62,15 @@ type repoKey struct { Repo string } +// repoFileEntry is one seeded file for MockClient.ListRepoDirectory / +// GetRepoFileContent. Ref "" is a wildcard that matches any requested ref; +// a non-empty Ref only matches that exact ref. +type repoFileEntry struct { + Ref string + Path string + Content []byte +} + // MockClient implements Client with in-memory configurable data for E2E testing. // All data is protected by a sync.RWMutex for thread safety. type MockClient struct { @@ -92,6 +102,7 @@ type MockClient struct { gists map[string]mockGist deletedGists []string nextGistID int + repoFiles map[repoKey][]repoFileEntry // findPRByBranchCalls counts FindPRByBranch invocations so tests can // assert that branch-detection probes are throttled. Atomic because @@ -133,6 +144,7 @@ func NewMockClient() *MockClient { commits: make(map[prKey][]PRCommitInfo), mergeMethods: make(map[repoKey]RepoMergeMethods), gists: make(map[string]mockGist), + repoFiles: make(map[repoKey][]repoFileEntry), } } @@ -426,6 +438,111 @@ func (m *MockClient) ListRepoBranches(_ context.Context, owner, repo string) ([] return out, nil } +// ListRepoDirectory returns the immediate children of dir, derived from the +// paths seeded via SeedRepoFile for owner/repo. Entries seeded with a +// specific ref are only visible when ref matches; entries seeded with ref "" +// are visible for any requested ref. A directory with no matching seeded +// descendants (including an entirely unseeded repo) returns a 404, mirroring +// "missing directory" on the real API. +func (m *MockClient) ListRepoDirectory(_ context.Context, owner, repo, dir, ref string) ([]RepoContentEntry, error) { + m.mu.RLock() + defer m.mu.RUnlock() + cleanDir := repoContentsPath(dir) + children := make(map[string]RepoContentEntry) + for _, e := range m.repoFiles[repoKey{owner, repo}] { + if e.Ref != "" && e.Ref != ref { + continue + } + name, isDir, ok := repoDirChild(cleanDir, e.Path) + if !ok { + continue + } + if _, exists := children[name]; exists { + continue + } + entryType, childPath := RepoContentTypeFile, name + if isDir { + entryType = RepoContentTypeDir + } + if cleanDir != "" { + childPath = cleanDir + "/" + name + } + children[name] = RepoContentEntry{Name: name, Path: childPath, Type: entryType} + } + if len(children) == 0 { + return nil, &GitHubAPIError{ + StatusCode: 404, + Endpoint: fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, cleanDir), + Body: fmt.Sprintf("directory %q not found", dir), + } + } + out := make([]RepoContentEntry, 0, len(children)) + for _, c := range children { + out = append(out, c) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out, nil +} + +// repoDirChild reports the immediate child name of path relative to dir +// ("" meaning root), and whether that child is itself a directory (path has +// further segments beyond the child name). ok is false when path does not +// live under dir. +func repoDirChild(dir, path string) (name string, isDir bool, ok bool) { + rest := path + if dir != "" { + prefix := dir + "/" + if !strings.HasPrefix(path, prefix) { + return "", false, false + } + rest = strings.TrimPrefix(path, prefix) + } + if rest == "" { + return "", false, false + } + if idx := strings.Index(rest, "/"); idx >= 0 { + return rest[:idx], true, true + } + return rest, false, true +} + +// GetRepoFileContent returns the seeded content for owner/repo/path, +// preferring an exact-ref seed over a wildcard (ref "") seed. Returns a 404 +// *GitHubAPIError when no seed matches. +func (m *MockClient) GetRepoFileContent(_ context.Context, owner, repo, path, ref string) ([]byte, error) { + m.mu.RLock() + defer m.mu.RUnlock() + cleanPath := repoContentsPath(path) + var wildcard *repoFileEntry + entries := m.repoFiles[repoKey{owner, repo}] + for i := range entries { + e := &entries[i] + if e.Path != cleanPath { + continue + } + if ref != "" && e.Ref == ref { + return cloneBytes(e.Content), nil + } + if e.Ref == "" { + wildcard = e + } + } + if wildcard != nil { + return cloneBytes(wildcard.Content), nil + } + return nil, &GitHubAPIError{ + StatusCode: 404, + Endpoint: fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, cleanPath), + Body: fmt.Sprintf("file %q not found", path), + } +} + +func cloneBytes(b []byte) []byte { + out := make([]byte, len(b)) + copy(out, b) + return out +} + func (m *MockClient) SubmitReview(_ context.Context, owner, repo string, number int, event, body string) error { m.mu.Lock() defer m.mu.Unlock() @@ -589,6 +706,24 @@ func (m *MockClient) AddBranches(owner, repo string, branches []RepoBranch) { m.branches[repoKey{owner, repo}] = cp } +// SeedRepoFile stores file content for ListRepoDirectory/GetRepoFileContent. +// ref "" matches any requested ref. Re-seeding the same owner/repo/ref/path +// replaces the previous content. +func (m *MockClient) SeedRepoFile(owner, repo, ref, path string, content []byte) { + m.mu.Lock() + defer m.mu.Unlock() + cleanPath := repoContentsPath(path) + k := repoKey{owner, repo} + cp := cloneBytes(content) + for i, e := range m.repoFiles[k] { + if e.Ref == ref && e.Path == cleanPath { + m.repoFiles[k][i].Content = cp + return + } + } + m.repoFiles[k] = append(m.repoFiles[k], repoFileEntry{Ref: ref, Path: cleanPath, Content: cp}) +} + // AddRepos adds repos under a key (an org login OR the authenticated user's // login). The mock's ListUserRepos reads m.repos[m.user], so the same store // backs both SearchOrgRepos and ListUserRepos in tests. @@ -691,6 +826,7 @@ func (m *MockClient) Reset() { m.gists = make(map[string]mockGist) m.deletedGists = nil m.nextGistID = 0 + m.repoFiles = make(map[repoKey][]repoFileEntry) m.findPRByBranchCalls.Store(0) m.probeEntered = nil m.probeRelease = nil diff --git a/apps/backend/internal/github/mock_controller.go b/apps/backend/internal/github/mock_controller.go index a1180c4120..f1c7e8cda4 100644 --- a/apps/backend/internal/github/mock_controller.go +++ b/apps/backend/internal/github/mock_controller.go @@ -44,6 +44,7 @@ func (c *MockController) RegisterRoutes(router *gin.Engine) { api.POST("/files", c.addPRFiles) api.POST("/commits", c.addPRCommits) api.POST("/branches", c.addBranches) + api.POST("/repo-files", c.addRepoFiles) api.POST("/task-prs", c.associateTaskPR) api.POST("/pr-feedback", c.seedPRFeedback) api.PUT("/auth-health", c.setAuthHealth) @@ -244,6 +245,33 @@ func (c *MockController) addBranches(ctx *gin.Context) { ctx.JSON(http.StatusOK, gin.H{"added": len(req.Branches)}) } +// addRepoFiles seeds file content for MockClient.ListRepoDirectory / +// GetRepoFileContent. ref "" seeds a wildcard entry that matches any +// requested ref (see MockClient.SeedRepoFile). +func (c *MockController) addRepoFiles(ctx *gin.Context) { + var req struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Ref string `json:"ref"` + Files []struct { + Path string `json:"path"` + Content string `json:"content"` + } `json:"files"` + } + if err := ctx.ShouldBindJSON(&req); err != nil { + respondInvalidPayload(ctx) + return + } + if req.Owner == "" || req.Repo == "" { + ctx.JSON(http.StatusBadRequest, gin.H{errKey: "owner and repo are required"}) + return + } + for _, f := range req.Files { + c.mock.SeedRepoFile(req.Owner, req.Repo, req.Ref, f.Path, []byte(f.Content)) + } + ctx.JSON(http.StatusOK, gin.H{"added": len(req.Files)}) +} + // associateTaskPRRequest is the JSON body for the mock-controller's // associateTaskPR endpoint. Pointer fields are optional — leave them nil // to skip the corresponding TaskPR column update. diff --git a/apps/backend/internal/github/mock_controller_test.go b/apps/backend/internal/github/mock_controller_test.go index 68d6675d4a..14e56b3060 100644 --- a/apps/backend/internal/github/mock_controller_test.go +++ b/apps/backend/internal/github/mock_controller_test.go @@ -64,6 +64,68 @@ func TestMockControllerAddIssuesInvalidPayload(t *testing.T) { } } +func TestMockControllerAddRepoFiles(t *testing.T) { + router, mock := setupMockControllerTestForAddIssues() + body := bytes.NewBufferString(`{ + "owner":"o", + "repo":"r", + "ref":"main", + "files":[{"path":"workflows/deploy.yaml","content":"name: deploy"}] + }`) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/github/mock/repo-files", body) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var got struct { + Added int `json:"added"` + } + if err := json.NewDecoder(w.Body).Decode(&got); err != nil { + t.Fatalf("decode response: %v", err) + } + if got.Added != 1 { + t.Fatalf("expected added=1, got %d", got.Added) + } + content, err := mock.GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", "main") + if err != nil { + t.Fatalf("get seeded repo file: %v", err) + } + if string(content) != "name: deploy" { + t.Fatalf("expected seeded content, got %q", content) + } +} + +func TestMockControllerAddRepoFilesInvalidPayload(t *testing.T) { + router, _ := setupMockControllerTestForAddIssues() + + req := httptest.NewRequest(http.MethodPost, "/api/v1/github/mock/repo-files", bytes.NewBufferString(`{`)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestMockControllerAddRepoFilesRequiresOwnerAndRepo(t *testing.T) { + router, _ := setupMockControllerTestForAddIssues() + body := bytes.NewBufferString(`{"files":[{"path":"a.yaml","content":"x"}]}`) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/github/mock/repo-files", body) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + func TestEnsureMockPRForRequestCopiesMergeableState(t *testing.T) { mock := NewMockClient() controller := &MockController{mock: mock} diff --git a/apps/backend/internal/github/models.go b/apps/backend/internal/github/models.go index a5a2950149..7cfaa5bb3a 100644 --- a/apps/backend/internal/github/models.go +++ b/apps/backend/internal/github/models.go @@ -392,6 +392,21 @@ type RepoBranch struct { Name string `json:"name"` } +// Repo content entry types, as reported by the GitHub contents API's "type" field. +const ( + RepoContentTypeFile = "file" + RepoContentTypeDir = "dir" +) + +// RepoContentEntry describes one entry returned when listing a repository directory. +type RepoContentEntry struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` // "file" or "dir" + SHA string `json:"sha"` + Size int `json:"size"` +} + // RepoMergeMethods reports which merge methods a repository allows. Mirrors // the allow_*_merge booleans from GET /repos/{owner}/{repo}. type RepoMergeMethods struct { diff --git a/apps/backend/internal/github/noop_client.go b/apps/backend/internal/github/noop_client.go index c2e2e8b17b..655d58721a 100644 --- a/apps/backend/internal/github/noop_client.go +++ b/apps/backend/internal/github/noop_client.go @@ -128,3 +128,11 @@ func (c *NoopClient) CreateGist(context.Context, CreateGistInput) (*GistResponse func (c *NoopClient) DeleteGist(context.Context, string) error { return ErrNoClient } + +func (c *NoopClient) ListRepoDirectory(context.Context, string, string, string, string) ([]RepoContentEntry, error) { + return nil, ErrNoClient +} + +func (c *NoopClient) GetRepoFileContent(context.Context, string, string, string, string) ([]byte, error) { + return nil, ErrNoClient +} diff --git a/apps/backend/internal/github/pat_client.go b/apps/backend/internal/github/pat_client.go index 64eb019d42..4ae68abc60 100644 --- a/apps/backend/internal/github/pat_client.go +++ b/apps/backend/internal/github/pat_client.go @@ -3,6 +3,7 @@ package github import ( "bytes" "context" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -667,6 +668,45 @@ func (c *PATClient) DeleteGist(ctx context.Context, gistID string) error { return c.delete(ctx, "/gists/"+gistID) } +// ListRepoDirectory lists the entries of a directory in a repository at the +// given ref via the REST contents API. A 404 (missing directory) surfaces as +// a *GitHubAPIError via the shared get() helper. +func (c *PATClient) ListRepoDirectory(ctx context.Context, owner, repo, dir, ref string) ([]RepoContentEntry, error) { + endpoint := repoContentsEndpoint(owner, repo, dir, ref) + var entries []RepoContentEntry + if err := c.get(ctx, endpoint, &entries); err != nil { + return nil, fmt.Errorf("list repo directory: %w", err) + } + return entries, nil +} + +// ghRepoFileContent mirrors the GitHub contents API's per-file response +// shape (only the fields GetRepoFileContent needs). +type ghRepoFileContent struct { + Type string `json:"type"` + Encoding string `json:"encoding"` + Content string `json:"content"` +} + +// GetRepoFileContent fetches the raw decoded content of a single file via +// the REST contents API. A 404 (missing file) surfaces as a *GitHubAPIError +// via the shared get() helper. +func (c *PATClient) GetRepoFileContent(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { + endpoint := repoContentsEndpoint(owner, repo, path, ref) + var raw ghRepoFileContent + if err := c.get(ctx, endpoint, &raw); err != nil { + return nil, fmt.Errorf("get repo file content: %w", err) + } + if raw.Encoding != "base64" { + return nil, fmt.Errorf("get repo file content: unsupported encoding %q", raw.Encoding) + } + decoded, err := base64.StdEncoding.DecodeString(stripBase64Whitespace(raw.Content)) + if err != nil { + return nil, fmt.Errorf("get repo file content: decode base64: %w", err) + } + return decoded, nil +} + // gistFileDTO mirrors the GitHub API's per-file body shape. type gistFileDTO struct { Content string `json:"content"` diff --git a/apps/backend/internal/github/repo_contents.go b/apps/backend/internal/github/repo_contents.go new file mode 100644 index 0000000000..0cd0aaeaf4 --- /dev/null +++ b/apps/backend/internal/github/repo_contents.go @@ -0,0 +1,50 @@ +package github + +import ( + "net/url" + "strings" + "unicode" +) + +// repoContentsPath returns the URL-escaped path segment used by the +// `contents` REST/CLI endpoint for dir/path. Each path segment is escaped +// independently so slashes are preserved as path separators while special +// characters within a segment (spaces, '#', etc.) are safely encoded. Returns +// "" for the repository root ("", ".", or all-slash input). +func repoContentsPath(path string) string { + trimmed := strings.Trim(path, "/") + if trimmed == "" || trimmed == "." { + return "" + } + segments := strings.Split(trimmed, "/") + for i, s := range segments { + segments[i] = url.PathEscape(s) + } + return strings.Join(segments, "/") +} + +// repoContentsEndpoint builds the `/repos/{owner}/{repo}/contents[/{path}]` +// REST endpoint (with an optional `?ref=` query param) shared by +// PATClient.ListRepoDirectory / GetRepoFileContent. +func repoContentsEndpoint(owner, repo, path, ref string) string { + endpoint := "/repos/" + owner + "/" + repo + "/contents" + if p := repoContentsPath(path); p != "" { + endpoint += "/" + p + } + if ref != "" { + endpoint += "?ref=" + url.QueryEscape(ref) + } + return endpoint +} + +// stripBase64Whitespace removes all whitespace (including the embedded +// newlines GitHub's contents API wraps base64 content with every 60 +// characters) so the result can be fed directly to base64.StdEncoding. +func stripBase64Whitespace(s string) string { + return strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + return r + }, s) +} diff --git a/apps/backend/internal/github/repo_contents_test.go b/apps/backend/internal/github/repo_contents_test.go new file mode 100644 index 0000000000..f624fb8e59 --- /dev/null +++ b/apps/backend/internal/github/repo_contents_test.go @@ -0,0 +1,442 @@ +package github + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRepoContentsPath(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {name: "empty is root", in: "", want: ""}, + {name: "dot is root", in: ".", want: ""}, + {name: "simple path", in: "workflows", want: "workflows"}, + {name: "nested path preserves slashes", in: "a/b/c.yaml", want: "a/b/c.yaml"}, + {name: "segment with space is escaped", in: "my dir/file.yaml", want: "my%20dir/file.yaml"}, + {name: "leading and trailing slashes trimmed", in: "/workflows/", want: "workflows"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := repoContentsPath(tc.in); got != tc.want { + t.Fatalf("repoContentsPath(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestPATClient_ListRepoDirectory_DecodesEntries(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + wantPath := "/repos/o/r/contents/workflows" + if r.URL.Path != wantPath { + t.Errorf("path = %q, want %q", r.URL.Path, wantPath) + } + if got := r.URL.Query().Get("ref"); got != "main" { + t.Errorf("ref = %q, want main", got) + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`[ + {"name":"deploy.yaml","path":"workflows/deploy.yaml","type":"file","sha":"abc123","size":42}, + {"name":"nested","path":"workflows/nested","type":"dir","sha":"def456","size":0} + ]`)) + })) + t.Cleanup(srv.Close) + c := newPATClientPointingAt(t, srv.URL) + + entries, err := c.ListRepoDirectory(context.Background(), "o", "r", "workflows", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(entries) != 2 { + t.Fatalf("expected 2 entries, got %d", len(entries)) + } + if entries[0].Name != "deploy.yaml" || entries[0].Type != "file" || entries[0].SHA != "abc123" || entries[0].Size != 42 { + t.Fatalf("unexpected first entry: %+v", entries[0]) + } + if entries[1].Name != "nested" || entries[1].Type != "dir" { + t.Fatalf("unexpected second entry: %+v", entries[1]) + } +} + +func TestPATClient_ListRepoDirectory_RootOmitsPathSegment(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + wantPath := "/repos/o/r/contents" + if r.URL.Path != wantPath { + t.Errorf("path = %q, want %q", r.URL.Path, wantPath) + } + if got := r.URL.Query().Get("ref"); got != "" { + t.Errorf("ref = %q, want empty (omitted)", got) + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`[]`)) + })) + t.Cleanup(srv.Close) + c := newPATClientPointingAt(t, srv.URL) + + entries, err := c.ListRepoDirectory(context.Background(), "o", "r", "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(entries) != 0 { + t.Fatalf("expected 0 entries, got %d", len(entries)) + } +} + +func TestPATClient_ListRepoDirectory_404(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"Not Found"}`)) + })) + t.Cleanup(srv.Close) + c := newPATClientPointingAt(t, srv.URL) + + _, err := c.ListRepoDirectory(context.Background(), "o", "r", "missing", "main") + if err == nil { + t.Fatal("expected error, got nil") + } + var apiErr *GitHubAPIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *GitHubAPIError, got %T: %v", err, err) + } + if apiErr.StatusCode != http.StatusNotFound { + t.Fatalf("StatusCode = %d, want 404", apiErr.StatusCode) + } +} + +func TestPATClient_GetRepoFileContent_DecodesBase64(t *testing.T) { + want := []byte("name: deploy\nsteps:\n - run: echo hi\n") + // GitHub wraps base64 content with embedded newlines every 60 chars; + // simulate that here to prove the decoder strips them. + encoded := base64.StdEncoding.EncodeToString(want) + wrapped := encoded[:len(encoded)/2] + "\n" + encoded[len(encoded)/2:] + "\n" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + wantPath := "/repos/o/r/contents/workflows/deploy.yaml" + if r.URL.Path != wantPath { + t.Errorf("path = %q, want %q", r.URL.Path, wantPath) + } + respBody, err := json.Marshal(map[string]string{ + "type": "file", + "encoding": "base64", + "content": wrapped, + }) + if err != nil { + t.Fatalf("marshal response body: %v", err) + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write(respBody) + })) + t.Cleanup(srv.Close) + c := newPATClientPointingAt(t, srv.URL) + + got, err := c.GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != string(want) { + t.Fatalf("content = %q, want %q", got, want) + } +} + +func TestPATClient_GetRepoFileContent_NonBase64EncodingErrors(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"type":"file","encoding":"none","content":"raw text"}`)) + })) + t.Cleanup(srv.Close) + c := newPATClientPointingAt(t, srv.URL) + + _, err := c.GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", "main") + if err == nil { + t.Fatal("expected error for non-base64 encoding, got nil") + } +} + +func TestPATClient_GetRepoFileContent_404(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"Not Found"}`)) + })) + t.Cleanup(srv.Close) + c := newPATClientPointingAt(t, srv.URL) + + _, err := c.GetRepoFileContent(context.Background(), "o", "r", "missing.yaml", "main") + if err == nil { + t.Fatal("expected error, got nil") + } + var apiErr *GitHubAPIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *GitHubAPIError, got %T: %v", err, err) + } + if apiErr.StatusCode != http.StatusNotFound { + t.Fatalf("StatusCode = %d, want 404", apiErr.StatusCode) + } +} + +// --- MockClient repo-file seeding --- + +func TestMockClient_SeedRepoFile_GetRepoFileContent_ExactRef(t *testing.T) { + m := NewMockClient() + m.SeedRepoFile("o", "r", "main", "workflows/deploy.yaml", []byte("on-main")) + m.SeedRepoFile("o", "r", "dev", "workflows/deploy.yaml", []byte("on-dev")) + + got, err := m.GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != "on-main" { + t.Fatalf("content = %q, want on-main", got) + } + + got, err = m.GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", "dev") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != "on-dev" { + t.Fatalf("content = %q, want on-dev", got) + } +} + +func TestMockClient_SeedRepoFile_WildcardRefMatchesAnyRequestedRef(t *testing.T) { + m := NewMockClient() + m.SeedRepoFile("o", "r", "", "workflows/deploy.yaml", []byte("wildcard")) + + for _, ref := range []string{"", "main", "feature/x"} { + got, err := m.GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", ref) + if err != nil { + t.Fatalf("ref %q: unexpected error: %v", ref, err) + } + if string(got) != "wildcard" { + t.Fatalf("ref %q: content = %q, want wildcard", ref, got) + } + } +} + +func TestMockClient_SeedRepoFile_ExactRefPreferredOverWildcard(t *testing.T) { + m := NewMockClient() + m.SeedRepoFile("o", "r", "", "workflows/deploy.yaml", []byte("wildcard")) + m.SeedRepoFile("o", "r", "main", "workflows/deploy.yaml", []byte("exact-main")) + + got, err := m.GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != "exact-main" { + t.Fatalf("content = %q, want exact-main (exact ref should win over wildcard)", got) + } + + // A specific-ref seed must NOT satisfy a different ref, or the default + // (empty) ref, since only the wildcard seed matches those. + got, err = m.GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", "dev") + if err != nil { + t.Fatalf("unexpected error for dev ref: %v", err) + } + if string(got) != "wildcard" { + t.Fatalf("dev ref content = %q, want wildcard", got) + } +} + +func TestMockClient_GetRepoFileContent_404(t *testing.T) { + m := NewMockClient() + _, err := m.GetRepoFileContent(context.Background(), "o", "r", "missing.yaml", "main") + if err == nil { + t.Fatal("expected error, got nil") + } + var apiErr *GitHubAPIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *GitHubAPIError, got %T: %v", err, err) + } + if apiErr.StatusCode != 404 { + t.Fatalf("StatusCode = %d, want 404", apiErr.StatusCode) + } +} + +func TestMockClient_ListRepoDirectory_InfersFilesAndSubdirs(t *testing.T) { + m := NewMockClient() + m.SeedRepoFile("o", "r", "main", "workflows/deploy.yaml", []byte("a")) + m.SeedRepoFile("o", "r", "main", "workflows/nested/inner.yaml", []byte("b")) + m.SeedRepoFile("o", "r", "main", "README.md", []byte("c")) + + entries, err := m.ListRepoDirectory(context.Background(), "o", "r", "", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + byName := make(map[string]RepoContentEntry, len(entries)) + for _, e := range entries { + byName[e.Name] = e + } + if len(entries) != 2 { + t.Fatalf("expected 2 root entries, got %d: %+v", len(entries), entries) + } + if got := byName["README.md"]; got.Type != "file" || got.Path != "README.md" { + t.Fatalf("README.md entry = %+v, want file at README.md", got) + } + if got := byName["workflows"]; got.Type != "dir" || got.Path != "workflows" { + t.Fatalf("workflows entry = %+v, want dir at workflows", got) + } + + nested, err := m.ListRepoDirectory(context.Background(), "o", "r", "workflows", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + nestedByName := make(map[string]RepoContentEntry, len(nested)) + for _, e := range nested { + nestedByName[e.Name] = e + } + if len(nested) != 2 { + t.Fatalf("expected 2 entries under workflows, got %d: %+v", len(nested), nested) + } + if got := nestedByName["deploy.yaml"]; got.Type != "file" || got.Path != "workflows/deploy.yaml" { + t.Fatalf("deploy.yaml entry = %+v", got) + } + if got := nestedByName["nested"]; got.Type != "dir" || got.Path != "workflows/nested" { + t.Fatalf("nested entry = %+v", got) + } +} + +func TestMockClient_ListRepoDirectory_404OnMissingDir(t *testing.T) { + m := NewMockClient() + m.SeedRepoFile("o", "r", "main", "workflows/deploy.yaml", []byte("a")) + + _, err := m.ListRepoDirectory(context.Background(), "o", "r", "does-not-exist", "main") + if err == nil { + t.Fatal("expected error, got nil") + } + var apiErr *GitHubAPIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *GitHubAPIError, got %T: %v", err, err) + } + if apiErr.StatusCode != 404 { + t.Fatalf("StatusCode = %d, want 404", apiErr.StatusCode) + } +} + +func TestMockClient_ListRepoDirectory_RefScoping(t *testing.T) { + m := NewMockClient() + m.SeedRepoFile("o", "r", "main", "only-on-main.yaml", []byte("a")) + + _, err := m.ListRepoDirectory(context.Background(), "o", "r", "", "dev") + if err == nil { + t.Fatal("expected 404 for a ref with no matching seeded files, got nil") + } + + entries, err := m.ListRepoDirectory(context.Background(), "o", "r", "", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(entries) != 1 || entries[0].Name != "only-on-main.yaml" { + t.Fatalf("unexpected entries: %+v", entries) + } +} + +// --- GHClient (gh CLI) --- + +// installFakeGH writes a fake `gh` shell script onto PATH for the duration of +// the test, logging every invocation's argv to a file the test can inspect. +// script receives the raw argv (via "$*") and stdout/stderr as it sees fit. +func installFakeGH(t *testing.T, script string) (argsLogPath string) { + t.Helper() + binDir := t.TempDir() + logPath := filepath.Join(t.TempDir(), "gh-args.log") + ghPath := filepath.Join(binDir, "gh") + if err := os.WriteFile(ghPath, []byte(script), 0o755); err != nil { + t.Fatalf("write fake gh: %v", err) + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("GH_ARGS_LOG", logPath) + return logPath +} + +func TestGHClient_ListRepoDirectory_DecodesEntries(t *testing.T) { + logPath := installFakeGH(t, `#!/bin/sh +printf '%s\n' "$*" >> "$GH_ARGS_LOG" +case "$*" in + *contents/workflows*) + printf '%s\n' '[{"name":"deploy.yaml","path":"workflows/deploy.yaml","type":"file","sha":"abc123","size":42}]' + ;; + *) printf '%s\n' 'gh: HTTP 404: Not Found' >&2; exit 1 ;; +esac +`) + + entries, err := NewGHClient().ListRepoDirectory(context.Background(), "o", "r", "workflows", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(entries) != 1 || entries[0].Name != "deploy.yaml" || entries[0].Type != "file" { + t.Fatalf("unexpected entries: %+v", entries) + } + logged, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read gh args log: %v", err) + } + if !strings.Contains(string(logged), "repos/o/r/contents/workflows") { + t.Fatalf("expected contents endpoint in gh args, got:\n%s", logged) + } + if !strings.Contains(string(logged), "-f ref=main") { + t.Fatalf("expected ref field in gh args, got:\n%s", logged) + } +} + +func TestGHClient_ListRepoDirectory_404(t *testing.T) { + installFakeGH(t, `#!/bin/sh +printf '%s\n' 'gh: HTTP 404: Not Found' >&2 +exit 1 +`) + + _, err := NewGHClient().ListRepoDirectory(context.Background(), "o", "r", "missing", "main") + if err == nil { + t.Fatal("expected error, got nil") + } + var apiErr *GitHubAPIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *GitHubAPIError, got %T: %v", err, err) + } + if apiErr.StatusCode != http.StatusNotFound { + t.Fatalf("StatusCode = %d, want 404", apiErr.StatusCode) + } +} + +func TestGHClient_GetRepoFileContent_DecodesBase64(t *testing.T) { + want := []byte("name: deploy\n") + encoded := base64.StdEncoding.EncodeToString(want) + installFakeGH(t, `#!/bin/sh +printf '{"type":"file","encoding":"base64","content":"`+encoded+`"}\n' +`) + + got, err := NewGHClient().GetRepoFileContent(context.Background(), "o", "r", "workflows/deploy.yaml", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != string(want) { + t.Fatalf("content = %q, want %q", got, want) + } +} + +func TestGHClient_GetRepoFileContent_404(t *testing.T) { + installFakeGH(t, `#!/bin/sh +printf '%s\n' 'gh: HTTP 404: Not Found' >&2 +exit 1 +`) + + _, err := NewGHClient().GetRepoFileContent(context.Background(), "o", "r", "missing.yaml", "main") + if err == nil { + t.Fatal("expected error, got nil") + } + var apiErr *GitHubAPIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *GitHubAPIError, got %T: %v", err, err) + } + if apiErr.StatusCode != http.StatusNotFound { + t.Fatalf("StatusCode = %d, want 404", apiErr.StatusCode) + } +} diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index 75cb18601b..4632bacaa7 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -18,9 +18,13 @@ type WorkflowDTO struct { Hidden bool `json:"hidden,omitempty"` // Style is a Phase 2 (ADR-0004) UX hint read by the frontend ONLY. // Allowed values: "kanban" | "office" | "custom". - Style string `json:"style,omitempty"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + Style string `json:"style,omitempty"` + // Source records where the workflow definition came from ("manual" | + // "github"); SourcePath is the repo-relative file for synced workflows. + Source string `json:"source,omitempty"` + SourcePath string `json:"source_path,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } type WorkspaceDTO struct { @@ -407,6 +411,8 @@ func FromWorkflow(workflow *models.Workflow) WorkflowDTO { SortOrder: workflow.SortOrder, Hidden: workflow.Hidden, Style: workflow.Style, + Source: workflow.Source, + SourcePath: workflow.SourcePath, CreatedAt: workflow.CreatedAt, UpdatedAt: workflow.UpdatedAt, } diff --git a/apps/backend/internal/task/handlers/workflow_handlers.go b/apps/backend/internal/task/handlers/workflow_handlers.go index 865977de8c..40da6be4b7 100644 --- a/apps/backend/internal/task/handlers/workflow_handlers.go +++ b/apps/backend/internal/task/handlers/workflow_handlers.go @@ -174,6 +174,35 @@ func (h *WorkflowHandlers) httpCreateWorkflow(c *gin.Context) { c.JSON(http.StatusCreated, dto.FromWorkflow(workflow)) } +// workflowReadOnlyMsg is returned when a UI mutation targets a workflow whose +// definition is owned by workflow sync. The sync applier writes through the +// service layer directly and never hits these handlers. +const workflowReadOnlyMsg = "workflow is managed by GitHub sync and is read-only; edit its definition in the synced repository" + +// isWorkflowReadOnly reports whether the workflow is managed by workflow +// sync. Lookup errors surface as (false, err) so callers keep their existing +// not-found handling. +func (h *WorkflowHandlers) isWorkflowReadOnly(ctx context.Context, id string) (bool, error) { + workflow, err := h.service.GetWorkflow(ctx, id) + if err != nil { + return false, err + } + return workflow.Source == models.WorkflowSourceGitHub, nil +} + +func (h *WorkflowHandlers) rejectReadOnlyWorkflowHTTP(c *gin.Context, id string) bool { + readOnly, err := h.isWorkflowReadOnly(c.Request.Context(), id) + if err != nil { + handleNotFound(c, h.logger, err, "workflow not found") + return true + } + if readOnly { + c.JSON(http.StatusConflict, gin.H{"error": workflowReadOnlyMsg}) + return true + } + return false +} + type httpUpdateWorkflowRequest struct { Name *string `json:"name"` Description *string `json:"description"` @@ -187,6 +216,9 @@ func (h *WorkflowHandlers) httpUpdateWorkflow(c *gin.Context) { return } id := c.Param("id") + if h.rejectReadOnlyWorkflowHTTP(c, id) { + return + } workflow, err := h.service.UpdateWorkflow(c.Request.Context(), id, &service.UpdateWorkflowRequest{ Name: body.Name, Description: body.Description, @@ -201,6 +233,9 @@ func (h *WorkflowHandlers) httpUpdateWorkflow(c *gin.Context) { func (h *WorkflowHandlers) httpDeleteWorkflow(c *gin.Context) { id := c.Param("id") + if h.rejectReadOnlyWorkflowHTTP(c, id) { + return + } if err := h.service.DeleteWorkflow(c.Request.Context(), id); err != nil { handleNotFound(c, h.logger, err, "workflow not found") return @@ -406,6 +441,9 @@ func (h *WorkflowHandlers) wsUpdateWorkflow(ctx context.Context, msg *ws.Message if req.ID == "" { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "id is required", nil) } + if readOnly, roErr := h.isWorkflowReadOnly(ctx, req.ID); roErr == nil && readOnly { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeConflict, workflowReadOnlyMsg, nil) + } workflow, err := h.service.UpdateWorkflow(ctx, req.ID, &service.UpdateWorkflowRequest{ Name: req.Name, @@ -420,6 +458,16 @@ func (h *WorkflowHandlers) wsUpdateWorkflow(ctx context.Context, msg *ws.Message } func (h *WorkflowHandlers) wsDeleteWorkflow(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + // Check read-only before the generic ID-request helper so the client gets + // a CONFLICT with the actual reason instead of a generic internal error. + var req struct { + ID string `json:"id"` + } + if err := msg.ParsePayload(&req); err == nil && req.ID != "" { + if readOnly, roErr := h.isWorkflowReadOnly(ctx, req.ID); roErr == nil && readOnly { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeConflict, workflowReadOnlyMsg, nil) + } + } return wsHandleIDRequest(ctx, msg, h.logger, "failed to delete workflow", func(ctx context.Context, id string) (any, error) { if err := h.service.DeleteWorkflow(ctx, id); err != nil { diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index 2c4e5b4d71..2e1b72b16d 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -399,6 +399,15 @@ const ( WorkflowStyleCustom = "custom" ) +// WorkflowSource values are persisted in workflows.source and record where a +// workflow definition came from. Manual workflows are user-managed; GitHub +// workflows are owned by the workflow-sync poller and may be overwritten or +// removed on each sync. +const ( + WorkflowSourceManual = "manual" + WorkflowSourceGitHub = "github" +) + // Workflow represents a task workflow type Workflow struct { ID string `json:"id"` @@ -408,6 +417,11 @@ type Workflow struct { AgentProfileID string `json:"agent_profile_id,omitempty"` WorkflowTemplateID *string `json:"workflow_template_id,omitempty"` SortOrder int `json:"sort_order"` + // Source records where the workflow definition came from ("manual" or + // "github"). SourcePath is the repo-relative file path the definition + // was synced from; empty for manual workflows. + Source string `json:"source,omitempty"` + SourcePath string `json:"source_path,omitempty"` // Hidden workflows are excluded from management and picker UIs by default. // Used by system-only flows like Improve Kandev. Hidden bool `json:"hidden,omitempty"` diff --git a/apps/backend/internal/task/repository/sqlite/base_migrations.go b/apps/backend/internal/task/repository/sqlite/base_migrations.go index 95494837f1..07bb070999 100644 --- a/apps/backend/internal/task/repository/sqlite/base_migrations.go +++ b/apps/backend/internal/task/repository/sqlite/base_migrations.go @@ -149,6 +149,12 @@ func (r *Repository) runMigrations() error { // presentation for existing workflows. r.migrate.Apply("workflows.style", `ALTER TABLE workflows ADD COLUMN style TEXT NOT NULL DEFAULT 'kanban'`) + // Workflow-sync provenance: which system owns the workflow definition + // ("manual" | "github") and, for synced workflows, the repo-relative + // file path it was synced from. + r.migrate.Apply("workflows.source", `ALTER TABLE workflows ADD COLUMN source TEXT NOT NULL DEFAULT 'manual'`) + r.migrate.Apply("workflows.source_path", `ALTER TABLE workflows ADD COLUMN source_path TEXT NOT NULL DEFAULT ''`) + // ADR 0005 Wave F — ensure the runner-projection tables exist so // task SELECTs that reference them via correlated subquery don't // fail. Required for tests and any environment where the workflow diff --git a/apps/backend/internal/task/repository/sqlite/workflow.go b/apps/backend/internal/task/repository/sqlite/workflow.go index 1be376c31b..a6a6fbd9a2 100644 --- a/apps/backend/internal/task/repository/sqlite/workflow.go +++ b/apps/backend/internal/task/repository/sqlite/workflow.go @@ -52,13 +52,23 @@ func (r *Repository) CreateWorkflow(ctx context.Context, workflow *models.Workfl workflow.SortOrder = maxOrder + 1 _, err = r.db.ExecContext(ctx, r.db.Rebind(` - INSERT INTO workflows (id, workspace_id, name, description, agent_profile_id, workflow_template_id, sort_order, hidden, style, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `), workflow.ID, workflow.WorkspaceID, workflow.Name, workflow.Description, workflow.AgentProfileID, workflow.WorkflowTemplateID, workflow.SortOrder, dialect.BoolToInt(workflow.Hidden), normalizeWorkflowStyle(workflow.Style), workflow.CreatedAt, workflow.UpdatedAt) + INSERT INTO workflows (id, workspace_id, name, description, agent_profile_id, workflow_template_id, sort_order, hidden, style, source, source_path, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `), workflow.ID, workflow.WorkspaceID, workflow.Name, workflow.Description, workflow.AgentProfileID, workflow.WorkflowTemplateID, workflow.SortOrder, dialect.BoolToInt(workflow.Hidden), normalizeWorkflowStyle(workflow.Style), normalizeWorkflowSource(workflow.Source), workflow.SourcePath, workflow.CreatedAt, workflow.UpdatedAt) return err } +// normalizeWorkflowSource returns a value fit for the workflows.source column. +// Empty or unknown sources collapse to the manual default. +func normalizeWorkflowSource(source string) string { + switch source { + case models.WorkflowSourceManual, models.WorkflowSourceGitHub: + return source + } + return models.WorkflowSourceManual +} + // normalizeWorkflowStyle returns a value fit for the workflows.style column. // Empty or unknown styles collapse to the kanban default so existing callers // stay schema-compliant without having to set the field explicitly. @@ -72,7 +82,7 @@ func normalizeWorkflowStyle(style string) string { const workflowSelectColumns = ` id, workspace_id, name, description, agent_profile_id, - workflow_template_id, sort_order, hidden, style, created_at, updated_at + workflow_template_id, sort_order, hidden, style, source, source_path, created_at, updated_at ` type workflowScanner interface { @@ -81,7 +91,7 @@ type workflowScanner interface { func scanWorkflowRow(scanner workflowScanner) (*models.Workflow, error) { workflow := &models.Workflow{} - var workflowTemplateID, agentProfileID, style sql.NullString + var workflowTemplateID, agentProfileID, style, source, sourcePath sql.NullString var hidden int if err := scanner.Scan( &workflow.ID, @@ -93,6 +103,8 @@ func scanWorkflowRow(scanner workflowScanner) (*models.Workflow, error) { &workflow.SortOrder, &hidden, &style, + &source, + &sourcePath, &workflow.CreatedAt, &workflow.UpdatedAt, ); err != nil { @@ -110,6 +122,10 @@ func scanWorkflowRow(scanner workflowScanner) (*models.Workflow, error) { } else { workflow.Style = models.WorkflowStyleKanban } + workflow.Source = normalizeWorkflowSource(source.String) + if sourcePath.Valid { + workflow.SourcePath = sourcePath.String + } return workflow, nil } @@ -145,8 +161,8 @@ func (r *Repository) UpdateWorkflow(ctx context.Context, workflow *models.Workfl workflow.UpdatedAt = time.Now().UTC() result, err := r.db.ExecContext(ctx, r.db.Rebind(` - UPDATE workflows SET name = ?, description = ?, agent_profile_id = ?, workflow_template_id = ?, hidden = ?, style = ?, updated_at = ? WHERE id = ? - `), workflow.Name, workflow.Description, workflow.AgentProfileID, workflow.WorkflowTemplateID, dialect.BoolToInt(workflow.Hidden), normalizeWorkflowStyle(workflow.Style), workflow.UpdatedAt, workflow.ID) + UPDATE workflows SET name = ?, description = ?, agent_profile_id = ?, workflow_template_id = ?, hidden = ?, style = ?, source = ?, source_path = ?, updated_at = ? WHERE id = ? + `), workflow.Name, workflow.Description, workflow.AgentProfileID, workflow.WorkflowTemplateID, dialect.BoolToInt(workflow.Hidden), normalizeWorkflowStyle(workflow.Style), normalizeWorkflowSource(workflow.Source), workflow.SourcePath, workflow.UpdatedAt, workflow.ID) if err != nil { return err } diff --git a/apps/backend/internal/task/repository/sqlite/workflow_source_test.go b/apps/backend/internal/task/repository/sqlite/workflow_source_test.go new file mode 100644 index 0000000000..e6a033f366 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/workflow_source_test.go @@ -0,0 +1,139 @@ +package sqlite + +import ( + "context" + "path/filepath" + "testing" + + "github.com/jmoiron/sqlx" + + "github.com/kandev/kandev/internal/db" + "github.com/kandev/kandev/internal/task/models" +) + +func newRepoForWorkflowSourceTests(t *testing.T) *Repository { + t.Helper() + dbPath := filepath.Join(t.TempDir(), "workflow-source-test.db") + dbConn, err := db.OpenSQLite(dbPath) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + sqlxDB := sqlx.NewDb(dbConn, "sqlite3") + repo, err := NewWithDB(sqlxDB, sqlxDB, nil) + if err != nil { + t.Fatalf("new repo: %v", err) + } + t.Cleanup(func() { _ = sqlxDB.Close() }) + return repo +} + +// TestWorkflowSource_RoundTrip verifies the workflow-sync provenance columns +// survive create, get, list, and update. +func TestWorkflowSource_RoundTrip(t *testing.T) { + repo := newRepoForWorkflowSourceTests(t) + ctx := context.Background() + + wf := &models.Workflow{ + WorkspaceID: "ws-1", + Name: "Synced Flow", + Source: models.WorkflowSourceGitHub, + SourcePath: "flows/dev.yml", + } + if err := repo.CreateWorkflow(ctx, wf); err != nil { + t.Fatalf("create workflow: %v", err) + } + + got, err := repo.GetWorkflow(ctx, wf.ID) + if err != nil { + t.Fatalf("get workflow: %v", err) + } + if got.Source != models.WorkflowSourceGitHub || got.SourcePath != "flows/dev.yml" { + t.Fatalf("get: source roundtrip failed: %q %q", got.Source, got.SourcePath) + } + + listed, err := repo.ListWorkflows(ctx, "ws-1", true) + if err != nil { + t.Fatalf("list workflows: %v", err) + } + if len(listed) != 1 || listed[0].Source != models.WorkflowSourceGitHub { + t.Fatalf("list: source roundtrip failed: %+v", listed) + } + + got.SourcePath = "flows/renamed.yml" + if err := repo.UpdateWorkflow(ctx, got); err != nil { + t.Fatalf("update workflow: %v", err) + } + updated, err := repo.GetWorkflow(ctx, wf.ID) + if err != nil { + t.Fatalf("get updated workflow: %v", err) + } + if updated.SourcePath != "flows/renamed.yml" { + t.Fatalf("update: source_path not persisted: %q", updated.SourcePath) + } +} + +// TestWorkflowSource_SchemaReplay verifies the source/source_path migrations +// are idempotent: opening the repository a second time on the same database +// (which replays initSchema + runMigrations) must succeed and keep the data. +func TestWorkflowSource_SchemaReplay(t *testing.T) { + dbPath := filepath.Join(t.TempDir(), "workflow-source-replay.db") + ctx := context.Background() + + openRepo := func() (*Repository, *sqlx.DB) { + dbConn, err := db.OpenSQLite(dbPath) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + sqlxDB := sqlx.NewDb(dbConn, "sqlite3") + repo, err := NewWithDB(sqlxDB, sqlxDB, nil) + if err != nil { + t.Fatalf("new repo: %v", err) + } + return repo, sqlxDB + } + + repo, conn := openRepo() + wf := &models.Workflow{ + WorkspaceID: "ws-1", + Name: "Synced Flow", + Source: models.WorkflowSourceGitHub, + SourcePath: "flows/dev.yml", + } + if err := repo.CreateWorkflow(ctx, wf); err != nil { + t.Fatalf("create workflow: %v", err) + } + _ = conn.Close() + + repo, conn = openRepo() + defer func() { _ = conn.Close() }() + got, err := repo.GetWorkflow(ctx, wf.ID) + if err != nil { + t.Fatalf("get workflow after replay: %v", err) + } + if got.Source != models.WorkflowSourceGitHub || got.SourcePath != "flows/dev.yml" { + t.Fatalf("replay lost provenance: %q %q", got.Source, got.SourcePath) + } +} + +// TestWorkflowSource_DefaultsToManual verifies workflows created without an +// explicit source read back as manual (both fresh inserts and the scan-side +// normalization of the migration default). +func TestWorkflowSource_DefaultsToManual(t *testing.T) { + repo := newRepoForWorkflowSourceTests(t) + ctx := context.Background() + + wf := &models.Workflow{WorkspaceID: "ws-1", Name: "Personal Flow"} + if err := repo.CreateWorkflow(ctx, wf); err != nil { + t.Fatalf("create workflow: %v", err) + } + got, err := repo.GetWorkflow(ctx, wf.ID) + if err != nil { + t.Fatalf("get workflow: %v", err) + } + if got.Source != models.WorkflowSourceManual { + t.Fatalf("expected manual source default, got %q", got.Source) + } + if got.SourcePath != "" { + t.Fatalf("expected empty source_path, got %q", got.SourcePath) + } +} diff --git a/apps/backend/internal/task/service/service_events.go b/apps/backend/internal/task/service/service_events.go index c56e39a334..41cca68c5d 100644 --- a/apps/backend/internal/task/service/service_events.go +++ b/apps/backend/internal/task/service/service_events.go @@ -299,6 +299,8 @@ func (s *Service) publishWorkflowEvent(ctx context.Context, eventType string, wo "description": workflow.Description, "agent_profile_id": workflow.AgentProfileID, "hidden": workflow.Hidden, + "source": workflow.Source, + "source_path": workflow.SourcePath, "created_at": workflow.CreatedAt.Format(time.RFC3339), "updated_at": workflow.UpdatedAt.Format(time.RFC3339), } diff --git a/apps/backend/internal/task/service/service_resources.go b/apps/backend/internal/task/service/service_resources.go index 99141f9d59..f86689c017 100644 --- a/apps/backend/internal/task/service/service_resources.go +++ b/apps/backend/internal/task/service/service_resources.go @@ -441,6 +441,28 @@ func (s *Service) SetWorkflowHidden(ctx context.Context, id string, hidden bool) return nil } +// SetWorkflowSource stamps a workflow's provenance ("manual" | "github") and +// the repo-relative file path it was synced from. Used by workflow-sync to +// mark workflows it owns. +func (s *Service) SetWorkflowSource(ctx context.Context, id, source, sourcePath string) error { + workflow, err := s.workflows.GetWorkflow(ctx, id) + if err != nil { + return err + } + if workflow.Source == source && workflow.SourcePath == sourcePath { + return nil + } + workflow.Source = source + workflow.SourcePath = sourcePath + workflow.UpdatedAt = time.Now().UTC() + if err := s.workflows.UpdateWorkflow(ctx, workflow); err != nil { + s.logger.Error("failed to update workflow source", zap.String("workflow_id", id), zap.Error(err)) + return err + } + s.publishWorkflowEvent(ctx, events.WorkflowUpdated, workflow) + return nil +} + // DeleteWorkflow deletes a workflow, archiving its remaining tasks first so // they do not linger as orphan rows pointing at a workflow_id that no longer // exists (the tasks.workflow_id FK was dropped to support empty workflow_id diff --git a/apps/backend/internal/workflow/controller/controller.go b/apps/backend/internal/workflow/controller/controller.go index 0f8224da02..25ef81d186 100644 --- a/apps/backend/internal/workflow/controller/controller.go +++ b/apps/backend/internal/workflow/controller/controller.go @@ -91,6 +91,9 @@ func (c *Controller) GetStep(ctx context.Context, id string) (*GetStepResponse, } func (c *Controller) CreateStepsFromTemplate(ctx context.Context, req CreateStepsFromTemplateRequest) error { + if err := c.svc.EnsureWorkflowMutable(ctx, req.WorkflowID); err != nil { + return err + } return c.svc.CreateStepsFromTemplate(ctx, req.WorkflowID, req.TemplateID) } @@ -112,6 +115,9 @@ type CreateStepRequest struct { // CreateStep creates a new workflow step. func (c *Controller) CreateStep(ctx context.Context, req CreateStepRequest) (*GetStepResponse, error) { + if err := c.svc.EnsureWorkflowMutable(ctx, req.WorkflowID); err != nil { + return nil, err + } step := &models.WorkflowStep{ WorkflowID: req.WorkflowID, Name: req.Name, @@ -177,6 +183,9 @@ func (c *Controller) UpdateStep(ctx context.Context, req UpdateStepRequest) (*Ge if err != nil { return nil, err } + if err := c.svc.EnsureWorkflowMutable(ctx, step.WorkflowID); err != nil { + return nil, err + } if req.Name != nil { step.Name = *req.Name } @@ -276,6 +285,13 @@ func (c *Controller) validatePullFromStepAcyclic(ctx context.Context, stepID str // DeleteStep deletes a workflow step. func (c *Controller) DeleteStep(ctx context.Context, id string) error { + step, err := c.svc.GetStep(ctx, id) + if err != nil { + return err + } + if err := c.svc.EnsureWorkflowMutable(ctx, step.WorkflowID); err != nil { + return err + } return c.svc.DeleteStep(ctx, id) } @@ -287,6 +303,9 @@ type ReorderStepsRequest struct { // ReorderSteps reorders workflow steps for a workflow. func (c *Controller) ReorderSteps(ctx context.Context, req ReorderStepsRequest) error { + if err := c.svc.EnsureWorkflowMutable(ctx, req.WorkflowID); err != nil { + return err + } return c.svc.ReorderSteps(ctx, req.WorkflowID, req.StepIDs) } diff --git a/apps/backend/internal/workflow/handlers/handlers.go b/apps/backend/internal/workflow/handlers/handlers.go index 79a03b1454..d9ace2fb7c 100644 --- a/apps/backend/internal/workflow/handlers/handlers.go +++ b/apps/backend/internal/workflow/handlers/handlers.go @@ -2,6 +2,7 @@ package handlers import ( "context" + "errors" "io" "net/http" "strings" @@ -13,6 +14,7 @@ import ( "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/workflow/controller" "github.com/kandev/kandev/internal/workflow/models" + "github.com/kandev/kandev/internal/workflow/service" ws "github.com/kandev/kandev/pkg/websocket" ) @@ -143,7 +145,7 @@ func (h *Handlers) httpCreateStepsFromTemplate(c *gin.Context) { TemplateID: req.TemplateID, }); err != nil { h.logger.Error("failed to create steps from template", zap.Error(err)) - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create steps"}) + h.writeStepMutationError(c, err) return } c.JSON(http.StatusCreated, gin.H{"success": true}) @@ -185,6 +187,10 @@ func (h *Handlers) httpUpdateStep(c *gin.Context) { } func (h *Handlers) writeStepMutationError(c *gin.Context, err error) { + if errors.Is(err, service.ErrWorkflowReadOnly) { + c.JSON(http.StatusConflict, gin.H{"error": err.Error()}) + return + } msg := strings.ToLower(err.Error()) switch { case isStepValidationError(msg): @@ -206,7 +212,7 @@ func isStepValidationError(msg string) bool { func (h *Handlers) httpDeleteStep(c *gin.Context) { if err := h.controller.DeleteStep(c.Request.Context(), c.Param("id")); err != nil { h.logger.Error("failed to delete step", zap.Error(err)) - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete step"}) + h.writeStepMutationError(c, err) return } c.JSON(http.StatusOK, gin.H{"success": true}) @@ -227,7 +233,7 @@ func (h *Handlers) httpReorderSteps(c *gin.Context) { StepIDs: req.StepIDs, }); err != nil { h.logger.Error("failed to reorder steps", zap.Error(err)) - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to reorder steps"}) + h.writeStepMutationError(c, err) return } c.JSON(http.StatusOK, gin.H{"success": true}) diff --git a/apps/backend/internal/workflow/service/readonly.go b/apps/backend/internal/workflow/service/readonly.go new file mode 100644 index 0000000000..6df174ec6b --- /dev/null +++ b/apps/backend/internal/workflow/service/readonly.go @@ -0,0 +1,35 @@ +package service + +import ( + "context" + "errors" + + taskmodels "github.com/kandev/kandev/internal/task/models" +) + +// ErrWorkflowReadOnly rejects UI mutations of workflows managed by workflow +// sync. The sync applier itself writes through the repo/provider directly and +// never hits the guarded controller/handler paths. +var ErrWorkflowReadOnly = errors.New("workflow is managed by GitHub sync and is read-only; edit its definition in the synced repository") + +// EnsureWorkflowMutable returns ErrWorkflowReadOnly when the workflow's +// definition is owned by workflow sync (source == "github"). UI-facing +// mutation paths (step CRUD, template application) call this before writing. +// +// The guard fails open when the workflow can't be resolved: its only job is +// to block GitHub-managed workflows, the underlying mutation surfaces real +// not-found errors itself, and callers like the MCP handlers operate on +// workflows the wired provider may not track. +func (s *Service) EnsureWorkflowMutable(ctx context.Context, workflowID string) error { + if s.workflowProvider == nil { + return nil + } + wf, err := s.workflowProvider.GetWorkflow(ctx, workflowID) + if err != nil { + return nil //nolint:nilerr // fail open: guard only blocks resolved github-sourced workflows + } + if wf.Source == taskmodels.WorkflowSourceGitHub { + return ErrWorkflowReadOnly + } + return nil +} diff --git a/apps/backend/internal/workflow/service/service.go b/apps/backend/internal/workflow/service/service.go index 7bd2024d9b..50a94bdd4b 100644 --- a/apps/backend/internal/workflow/service/service.go +++ b/apps/backend/internal/workflow/service/service.go @@ -30,6 +30,7 @@ type Service struct { workflowProvider WorkflowProvider resolveProfile models.AgentProfileResolver matchProfile models.AgentProfileMatcher + syncOps SyncWorkflowOps } // SetWorkflowProvider wires the workflow provider (set during service init to break circular deps). @@ -493,7 +494,7 @@ func (s *Service) ImportWorkflows(ctx context.Context, workspaceID string, expor result.Skipped = append(result.Skipped, pw.Name) continue } - if err := s.importSingleWorkflow(ctx, workspaceID, pw); err != nil { + if _, err := s.importSingleWorkflow(ctx, workspaceID, pw); err != nil { return nil, fmt.Errorf("failed to import workflow %q: %w", pw.Name, err) } result.Created = append(result.Created, pw.Name) @@ -506,10 +507,10 @@ func (s *Service) ImportWorkflows(ctx context.Context, workspaceID string, expor return result, nil } -func (s *Service) importSingleWorkflow(ctx context.Context, workspaceID string, pw models.WorkflowPortable) error { +func (s *Service) importSingleWorkflow(ctx context.Context, workspaceID string, pw models.WorkflowPortable) (*taskmodels.Workflow, error) { wf, err := s.workflowProvider.CreateWorkflow(ctx, workspaceID, pw.Name, pw.Description) if err != nil { - return fmt.Errorf("create workflow: %w", err) + return nil, fmt.Errorf("create workflow: %w", err) } // Match workflow-level agent profile if present. @@ -517,7 +518,7 @@ func (s *Service) importSingleWorkflow(ctx context.Context, workspaceID string, if profileID := s.matchProfile(pw.AgentProfile.AgentName, pw.AgentProfile.Model, pw.AgentProfile.Mode); profileID != "" { wf.AgentProfileID = profileID if err := s.workflowProvider.UpdateWorkflow(ctx, wf); err != nil { - return fmt.Errorf("set workflow agent profile: %w", err) + return nil, fmt.Errorf("set workflow agent profile: %w", err) } } } @@ -530,29 +531,36 @@ func (s *Service) importSingleWorkflow(ctx context.Context, workspaceID string, // Create each step with remapped events. for _, sp := range pw.Steps { - step := &models.WorkflowStep{ - ID: posToID[sp.Position], - WorkflowID: wf.ID, - Name: sp.Name, - Position: sp.Position, - Color: sp.Color, - Prompt: sp.Prompt, - Events: models.ConvertPositionToStepID(sp.Events, posToID), - IsStartStep: sp.IsStartStep, - ShowInCommandPanel: sp.ShowInCommandPanel, - AllowManualMove: sp.AllowManualMove, - AutoArchiveAfterHours: sp.AutoArchiveAfterHours, - AutoAdvanceRequiresSignal: sp.AutoAdvanceRequiresSignal, - WIPLimit: sp.WIPLimit, - PullFromStepID: sp.PullFromStepID(posToID), - } - // Match step-level agent profile if present. - if sp.AgentProfile != nil && s.matchProfile != nil { - step.AgentProfileID = s.matchProfile(sp.AgentProfile.AgentName, sp.AgentProfile.Model, sp.AgentProfile.Mode) - } + step := s.stepFromPortable(wf.ID, sp, posToID) if err := s.repo.CreateStep(ctx, step); err != nil { - return fmt.Errorf("create step %q: %w", sp.Name, err) + return nil, fmt.Errorf("create step %q: %w", sp.Name, err) } } - return nil + return wf, nil +} + +// stepFromPortable builds a WorkflowStep from its portable form, remapping +// position-based references to the step IDs in posToID and matching the +// step-level agent profile when a matcher is wired. +func (s *Service) stepFromPortable(workflowID string, sp models.StepPortable, posToID map[int]string) *models.WorkflowStep { + step := &models.WorkflowStep{ + ID: posToID[sp.Position], + WorkflowID: workflowID, + Name: sp.Name, + Position: sp.Position, + Color: sp.Color, + Prompt: sp.Prompt, + Events: models.ConvertPositionToStepID(sp.Events, posToID), + IsStartStep: sp.IsStartStep, + ShowInCommandPanel: sp.ShowInCommandPanel, + AllowManualMove: sp.AllowManualMove, + AutoArchiveAfterHours: sp.AutoArchiveAfterHours, + AutoAdvanceRequiresSignal: sp.AutoAdvanceRequiresSignal, + WIPLimit: sp.WIPLimit, + PullFromStepID: sp.PullFromStepID(posToID), + } + if sp.AgentProfile != nil && s.matchProfile != nil { + step.AgentProfileID = s.matchProfile(sp.AgentProfile.AgentName, sp.AgentProfile.Model, sp.AgentProfile.Mode) + } + return step } diff --git a/apps/backend/internal/workflow/service/sync_apply.go b/apps/backend/internal/workflow/service/sync_apply.go new file mode 100644 index 0000000000..e50ca0113d --- /dev/null +++ b/apps/backend/internal/workflow/service/sync_apply.go @@ -0,0 +1,324 @@ +package service + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "sort" + + "github.com/google/uuid" + "go.uber.org/zap" + + taskmodels "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/workflow/models" +) + +// SyncWorkflowOps is the extra task-domain surface the workflow-sync applier +// needs beyond WorkflowProvider. Satisfied by the task service. +type SyncWorkflowOps interface { + DeleteWorkflow(ctx context.Context, id string) error + CountTasksByWorkflow(ctx context.Context, workflowID string) (int, error) + CountTasksByWorkflowStep(ctx context.Context, stepID string) (int, error) + SetWorkflowSource(ctx context.Context, id, source, sourcePath string) error +} + +// SetSyncWorkflowOps wires the task-domain operations used when applying a +// workflow sync (set during service init to break circular deps). +func (s *Service) SetSyncWorkflowOps(ops SyncWorkflowOps) { + s.syncOps = ops +} + +// SyncFileExport is one parsed, validated workflow definition file from a +// sync source (e.g. a GitHub repo). Path is the repo-relative file path. A +// nil Export marks a file that exists but could not be parsed: workflows +// previously synced from it are left untouched instead of being treated as +// removed. +type SyncFileExport struct { + Path string + Export *models.WorkflowExport +} + +// SyncApplyResult reports what applying a workflow sync changed. Warnings +// describe definitions that could not be applied and need user attention. +type SyncApplyResult struct { + Created []string `json:"created"` + Updated []string `json:"updated"` + Deleted []string `json:"deleted"` + Warnings []string `json:"warnings"` +} + +func syncKey(sourcePath, name string) string { + return sourcePath + "\x00" + name +} + +// ApplySyncedWorkflows reconciles the workspace's GitHub-sourced workflows +// with the given definition files. Workflows are matched by (source file +// path, workflow name): matches are updated in place (step identity is +// preserved by step name so tasks keep their step assignment), new +// definitions are created, and previously-synced workflows that disappeared +// from the source are deleted only when they no longer hold tasks. Manual +// workflows are never touched. Every export must already be validated. +func (s *Service) ApplySyncedWorkflows(ctx context.Context, workspaceID string, files []SyncFileExport) (*SyncApplyResult, error) { + if s.workflowProvider == nil || s.syncOps == nil { + return nil, fmt.Errorf("workflow sync is not wired") + } + existing, err := s.workflowProvider.ListWorkflows(ctx, workspaceID, true) + if err != nil { + return nil, fmt.Errorf("failed to list existing workflows: %w", err) + } + synced := make(map[string]*taskmodels.Workflow) + for _, wf := range existing { + if wf.Source == taskmodels.WorkflowSourceGitHub { + synced[syncKey(wf.SourcePath, wf.Name)] = wf + } + } + + result := &SyncApplyResult{} + desired := make(map[string]bool) + frozenPaths := make(map[string]bool) + for _, f := range files { + if f.Export == nil { + frozenPaths[f.Path] = true + continue + } + for _, pw := range f.Export.Workflows { + key := syncKey(f.Path, pw.Name) + if desired[key] { + result.Warnings = append(result.Warnings, fmt.Sprintf("%s: duplicate workflow %q ignored", f.Path, pw.Name)) + continue + } + desired[key] = true + if wf, ok := synced[key]; ok { + s.updateSyncedWorkflow(ctx, wf, pw, result) + } else { + s.createSyncedWorkflow(ctx, workspaceID, pw, f.Path, result) + } + } + } + s.deleteVanishedWorkflows(ctx, synced, desired, frozenPaths, result) + + sort.Strings(result.Created) + sort.Strings(result.Updated) + sort.Strings(result.Deleted) + s.logger.Info("applied workflow sync", + zap.String("workspace_id", workspaceID), + zap.Int("created", len(result.Created)), + zap.Int("updated", len(result.Updated)), + zap.Int("deleted", len(result.Deleted)), + zap.Int("warnings", len(result.Warnings))) + return result, nil +} + +func (s *Service) createSyncedWorkflow(ctx context.Context, workspaceID string, pw models.WorkflowPortable, path string, result *SyncApplyResult) { + wf, err := s.importSingleWorkflow(ctx, workspaceID, pw) + if err != nil { + result.Warnings = append(result.Warnings, fmt.Sprintf("%s: failed to create workflow %q: %v", path, pw.Name, err)) + return + } + if err := s.syncOps.SetWorkflowSource(ctx, wf.ID, taskmodels.WorkflowSourceGitHub, path); err != nil { + // Roll the creation back: an unstamped workflow would be left as an + // editable manual copy and the next sync would create a duplicate. + if delErr := s.syncOps.DeleteWorkflow(ctx, wf.ID); delErr != nil { + s.logger.Warn("failed to roll back unstamped synced workflow", + zap.String("workflow_id", wf.ID), zap.Error(delErr)) + } + result.Warnings = append(result.Warnings, fmt.Sprintf("%s: failed to mark workflow %q as synced: %v", path, pw.Name, err)) + return + } + result.Created = append(result.Created, pw.Name) +} + +// updateSyncedWorkflow reconciles an existing synced workflow with its +// definition. Steps are matched by name so their IDs — referenced by +// tasks.workflow_step_id — survive the update, and steps that already match +// the definition are left untouched so a no-drift sync writes (and +// broadcasts) nothing. The workflow is skipped with a warning when steps +// can't be matched unambiguously or when a removed step still holds tasks. +func (s *Service) updateSyncedWorkflow(ctx context.Context, wf *taskmodels.Workflow, pw models.WorkflowPortable, result *SyncApplyResult) { + steps, err := s.repo.ListStepsByWorkflow(ctx, wf.ID) + if err != nil { + result.Warnings = append(result.Warnings, fmt.Sprintf("workflow %q: failed to list steps: %v", wf.Name, err)) + return + } + existingByName, toDelete, warning := s.planStepChanges(ctx, wf, pw, steps) + if warning != "" { + result.Warnings = append(result.Warnings, warning) + return + } + + // Position → step ID: matched steps keep their ID, new steps get one. + posToID := make(map[int]string, len(pw.Steps)) + for _, sp := range pw.Steps { + if st, ok := existingByName[sp.Name]; ok { + posToID[sp.Position] = st.ID + } else { + posToID[sp.Position] = uuid.New().String() + } + } + + changed, err := s.applyWorkflowFields(ctx, wf, pw) + if err != nil { + result.Warnings = append(result.Warnings, fmt.Sprintf("workflow %q: failed to update: %v", wf.Name, err)) + return + } + for _, sp := range pw.Steps { + step := s.stepFromPortable(wf.ID, sp, posToID) + if existing, ok := existingByName[sp.Name]; ok { + step.CreatedAt = existing.CreatedAt + step.StageType = existing.StageType // not carried by the portable format + if stepMatchesDefinition(existing, step) { + continue + } + err = s.repo.UpdateStep(ctx, step) + } else { + err = s.repo.CreateStep(ctx, step) + } + if err != nil { + result.Warnings = append(result.Warnings, fmt.Sprintf("workflow %q: failed to apply step %q: %v", wf.Name, sp.Name, err)) + return + } + changed = true + } + for _, st := range toDelete { + if err := s.DeleteStep(ctx, st.ID); err != nil { + result.Warnings = append(result.Warnings, fmt.Sprintf("workflow %q: failed to remove step %q: %v", wf.Name, st.Name, err)) + return + } + changed = true + } + if changed { + result.Updated = append(result.Updated, wf.Name) + } +} + +// stepMatchesDefinition reports whether an existing step already equals its +// desired definition (timestamps and stage type aside — stage type is copied +// from the existing step). Events are compared via their JSON encoding to +// normalize YAML/JSON numeric type differences in action configs. +func stepMatchesDefinition(existing, desired *models.WorkflowStep) bool { + return existing.Name == desired.Name && + existing.Position == desired.Position && + existing.Color == desired.Color && + existing.Prompt == desired.Prompt && + existing.AllowManualMove == desired.AllowManualMove && + existing.IsStartStep == desired.IsStartStep && + existing.ShowInCommandPanel == desired.ShowInCommandPanel && + existing.AutoArchiveAfterHours == desired.AutoArchiveAfterHours && + existing.AgentProfileID == desired.AgentProfileID && + existing.WIPLimit == desired.WIPLimit && + existing.PullFromStepID == desired.PullFromStepID && + existing.AutoAdvanceRequiresSignal == desired.AutoAdvanceRequiresSignal && + eventsEqual(existing.Events, desired.Events) +} + +func eventsEqual(a, b models.StepEvents) bool { + aj, errA := json.Marshal(a) + bj, errB := json.Marshal(b) + return errA == nil && errB == nil && bytes.Equal(aj, bj) +} + +// planStepChanges matches existing steps to the portable definition by name +// and decides which steps would be removed. It returns a non-empty warning +// (and the update must be skipped) when names are ambiguous or a removed step +// still holds tasks. +func (s *Service) planStepChanges(ctx context.Context, wf *taskmodels.Workflow, pw models.WorkflowPortable, steps []*models.WorkflowStep) (map[string]*models.WorkflowStep, []*models.WorkflowStep, string) { + existingByName := make(map[string]*models.WorkflowStep, len(steps)) + for _, st := range steps { + if _, dup := existingByName[st.Name]; dup { + return nil, nil, fmt.Sprintf("workflow %q has multiple steps named %q; rename them, then sync again", wf.Name, st.Name) + } + existingByName[st.Name] = st + } + desiredNames := make(map[string]bool, len(pw.Steps)) + for _, sp := range pw.Steps { + if desiredNames[sp.Name] { + return nil, nil, fmt.Sprintf("workflow %q defines multiple steps named %q; step names must be unique to sync", wf.Name, sp.Name) + } + desiredNames[sp.Name] = true + } + var toDelete []*models.WorkflowStep + for _, st := range steps { + if desiredNames[st.Name] { + continue + } + count, err := s.syncOps.CountTasksByWorkflowStep(ctx, st.ID) + if err != nil { + return nil, nil, fmt.Sprintf("workflow %q: failed to count tasks in step %q: %v", wf.Name, st.Name, err) + } + if count > 0 { + return nil, nil, fmt.Sprintf("workflow %q not updated: step %q was removed from the definition but still has %d task(s); move or archive them, then sync again", wf.Name, st.Name, count) + } + toDelete = append(toDelete, st) + } + return existingByName, toDelete, "" +} + +func (s *Service) applyWorkflowFields(ctx context.Context, wf *taskmodels.Workflow, pw models.WorkflowPortable) (bool, error) { + profileID := "" + if pw.AgentProfile != nil && s.matchProfile != nil { + profileID = s.matchProfile(pw.AgentProfile.AgentName, pw.AgentProfile.Model, pw.AgentProfile.Mode) + } + if wf.Description == pw.Description && wf.AgentProfileID == profileID { + return false, nil + } + wf.Description = pw.Description + wf.AgentProfileID = profileID + return true, s.workflowProvider.UpdateWorkflow(ctx, wf) +} + +// ReleaseSyncedWorkflows converts every GitHub-sourced workflow in the +// workspace back to a manual, editable workflow. Called when the workspace's +// sync configuration is removed so users aren't locked out of orphaned +// read-only workflows. +func (s *Service) ReleaseSyncedWorkflows(ctx context.Context, workspaceID string) ([]string, error) { + if s.workflowProvider == nil || s.syncOps == nil { + return nil, fmt.Errorf("workflow sync is not wired") + } + workflows, err := s.workflowProvider.ListWorkflows(ctx, workspaceID, true) + if err != nil { + return nil, fmt.Errorf("failed to list workflows: %w", err) + } + var released []string + for _, wf := range workflows { + if wf.Source != taskmodels.WorkflowSourceGitHub { + continue + } + if err := s.syncOps.SetWorkflowSource(ctx, wf.ID, taskmodels.WorkflowSourceManual, ""); err != nil { + return released, fmt.Errorf("failed to release workflow %q: %w", wf.Name, err) + } + released = append(released, wf.Name) + } + sort.Strings(released) + return released, nil +} + +// deleteVanishedWorkflows removes previously-synced workflows whose +// definition disappeared from the source, unless they still hold tasks or +// come from a file that failed to parse this round. +// +// NOTE: a workflow rename in the source (same source_path, new name) is +// treated as delete-old + create-new, mirroring the step-rename behavior. +// Workflow-level references (e.g. workspaces.office_workflow_id) will point +// at the old ID until updated. +func (s *Service) deleteVanishedWorkflows(ctx context.Context, synced map[string]*taskmodels.Workflow, desired, frozenPaths map[string]bool, result *SyncApplyResult) { + for key, wf := range synced { + if desired[key] || frozenPaths[wf.SourcePath] { + continue + } + count, err := s.syncOps.CountTasksByWorkflow(ctx, wf.ID) + if err != nil { + result.Warnings = append(result.Warnings, fmt.Sprintf("workflow %q: failed to count tasks: %v", wf.Name, err)) + continue + } + if count > 0 { + result.Warnings = append(result.Warnings, fmt.Sprintf("workflow %q was removed from the sync source but still has %d task(s); move or archive them, then sync again", wf.Name, count)) + continue + } + if err := s.syncOps.DeleteWorkflow(ctx, wf.ID); err != nil { + result.Warnings = append(result.Warnings, fmt.Sprintf("workflow %q: failed to delete: %v", wf.Name, err)) + continue + } + result.Deleted = append(result.Deleted, wf.Name) + } +} diff --git a/apps/backend/internal/workflow/service/sync_apply_test.go b/apps/backend/internal/workflow/service/sync_apply_test.go new file mode 100644 index 0000000000..2e44f27e67 --- /dev/null +++ b/apps/backend/internal/workflow/service/sync_apply_test.go @@ -0,0 +1,403 @@ +package service + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + taskmodels "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/workflow/models" +) + +// mockSyncOps implements SyncWorkflowOps on top of a mockWorkflowProvider. +type mockSyncOps struct { + provider *mockWorkflowProvider + tasksPerWorkflow map[string]int + tasksPerStep map[string]int + deleted []string +} + +func newMockSyncOps(provider *mockWorkflowProvider) *mockSyncOps { + return &mockSyncOps{ + provider: provider, + tasksPerWorkflow: map[string]int{}, + tasksPerStep: map[string]int{}, + } +} + +func (m *mockSyncOps) DeleteWorkflow(_ context.Context, id string) error { + for i, wf := range m.provider.workflows { + if wf.ID == id { + m.provider.workflows = append(m.provider.workflows[:i], m.provider.workflows[i+1:]...) + m.deleted = append(m.deleted, id) + return nil + } + } + return fmt.Errorf("workflow %s not found", id) +} + +func (m *mockSyncOps) CountTasksByWorkflow(_ context.Context, workflowID string) (int, error) { + return m.tasksPerWorkflow[workflowID], nil +} + +func (m *mockSyncOps) CountTasksByWorkflowStep(_ context.Context, stepID string) (int, error) { + return m.tasksPerStep[stepID], nil +} + +func (m *mockSyncOps) SetWorkflowSource(ctx context.Context, id, source, sourcePath string) error { + wf, err := m.provider.GetWorkflow(ctx, id) + if err != nil { + return err + } + wf.Source = source + wf.SourcePath = sourcePath + return nil +} + +func setupSyncService(t *testing.T) (*Service, *mockWorkflowProvider, *mockSyncOps) { + svc, _ := setupTestService(t) + provider := &mockWorkflowProvider{} + svc.SetWorkflowProvider(provider) + ops := newMockSyncOps(provider) + svc.SetSyncWorkflowOps(ops) + return svc, provider, ops +} + +func portableWorkflow(name string, stepNames ...string) models.WorkflowPortable { + steps := make([]models.StepPortable, 0, len(stepNames)) + for i, sn := range stepNames { + steps = append(steps, models.StepPortable{ + Name: sn, + Position: i, + Color: "#aabbcc", + IsStartStep: i == 0, + }) + } + return models.WorkflowPortable{Name: name, Description: "synced " + name, Steps: steps} +} + +func exportOf(workflows ...models.WorkflowPortable) *models.WorkflowExport { + return &models.WorkflowExport{ + Version: models.ExportVersion, + Type: models.ExportType, + Workflows: workflows, + } +} + +func addSyncedWorkflow(provider *mockWorkflowProvider, id, workspaceID, name, sourcePath string) *taskmodels.Workflow { + provider.addWorkflow(id, workspaceID, name) + wf := provider.workflows[len(provider.workflows)-1] + wf.Source = taskmodels.WorkflowSourceGitHub + wf.SourcePath = sourcePath + return wf +} + +func TestApplySyncedWorkflows_CreatesNewWorkflows(t *testing.T) { + svc, provider, _ := setupSyncService(t) + ctx := context.Background() + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(portableWorkflow("Dev Flow", "Todo", "Doing", "Done"))}, + }) + require.NoError(t, err) + assert.Equal(t, []string{"Dev Flow"}, result.Created) + assert.Empty(t, result.Warnings) + + wf, err := provider.GetWorkflow(ctx, "imported-Dev Flow") + require.NoError(t, err) + assert.Equal(t, taskmodels.WorkflowSourceGitHub, wf.Source) + assert.Equal(t, "flows/dev.yml", wf.SourcePath) + + steps, err := svc.ListStepsByWorkflow(ctx, wf.ID) + require.NoError(t, err) + require.Len(t, steps, 3) + assert.Equal(t, "Todo", steps[0].Name) + assert.True(t, steps[0].IsStartStep) +} + +func TestApplySyncedWorkflows_UpdatesMatchedWorkflowPreservingStepIDs(t *testing.T) { + svc, provider, _ := setupSyncService(t) + ctx := context.Background() + wf := addSyncedWorkflow(provider, "wf-1", "ws-1", "Dev Flow", "flows/dev.yml") + + createStep(t, svc, &models.WorkflowStep{ID: "step-todo", WorkflowID: wf.ID, Name: "Todo", Position: 0, IsStartStep: true}) + createStep(t, svc, &models.WorkflowStep{ID: "step-done", WorkflowID: wf.ID, Name: "Done", Position: 1}) + + // New definition: Todo keeps its identity but moves color/position, + // "Review" is inserted, "Done" survives at position 2. + pw := portableWorkflow("Dev Flow", "Todo", "Review", "Done") + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(pw)}, + }) + require.NoError(t, err) + assert.Equal(t, []string{"Dev Flow"}, result.Updated) + assert.Empty(t, result.Created) + assert.Empty(t, result.Warnings) + assert.Equal(t, "synced Dev Flow", wf.Description) + + steps, err := svc.ListStepsByWorkflow(ctx, wf.ID) + require.NoError(t, err) + require.Len(t, steps, 3) + byName := map[string]*models.WorkflowStep{} + for _, st := range steps { + byName[st.Name] = st + } + assert.Equal(t, "step-todo", byName["Todo"].ID, "matched step keeps its ID") + assert.Equal(t, "step-done", byName["Done"].ID, "matched step keeps its ID") + assert.Equal(t, 1, byName["Review"].Position) + assert.Equal(t, 2, byName["Done"].Position) +} + +func TestApplySyncedWorkflows_RemovedStepWithTasksBlocksUpdate(t *testing.T) { + svc, provider, ops := setupSyncService(t) + ctx := context.Background() + wf := addSyncedWorkflow(provider, "wf-1", "ws-1", "Dev Flow", "flows/dev.yml") + createStep(t, svc, &models.WorkflowStep{ID: "step-todo", WorkflowID: wf.ID, Name: "Todo", Position: 0, IsStartStep: true}) + createStep(t, svc, &models.WorkflowStep{ID: "step-legacy", WorkflowID: wf.ID, Name: "Legacy", Position: 1}) + ops.tasksPerStep["step-legacy"] = 2 + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(portableWorkflow("Dev Flow", "Todo"))}, + }) + require.NoError(t, err) + assert.Empty(t, result.Updated) + require.Len(t, result.Warnings, 1) + assert.Contains(t, result.Warnings[0], "Legacy") + assert.Contains(t, result.Warnings[0], "2 task(s)") + + steps, err := svc.ListStepsByWorkflow(ctx, wf.ID) + require.NoError(t, err) + assert.Len(t, steps, 2, "workflow left untouched") +} + +func TestApplySyncedWorkflows_RemovedStepWithoutTasksIsDeleted(t *testing.T) { + svc, provider, _ := setupSyncService(t) + ctx := context.Background() + wf := addSyncedWorkflow(provider, "wf-1", "ws-1", "Dev Flow", "flows/dev.yml") + createStep(t, svc, &models.WorkflowStep{ID: "step-todo", WorkflowID: wf.ID, Name: "Todo", Position: 0, IsStartStep: true}) + createStep(t, svc, &models.WorkflowStep{ID: "step-legacy", WorkflowID: wf.ID, Name: "Legacy", Position: 1}) + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(portableWorkflow("Dev Flow", "Todo"))}, + }) + require.NoError(t, err) + assert.Equal(t, []string{"Dev Flow"}, result.Updated) + assert.Empty(t, result.Warnings) + + steps, err := svc.ListStepsByWorkflow(ctx, wf.ID) + require.NoError(t, err) + require.Len(t, steps, 1) + assert.Equal(t, "Todo", steps[0].Name) +} + +func TestApplySyncedWorkflows_DeletesVanishedWorkflowWithoutTasks(t *testing.T) { + svc, provider, ops := setupSyncService(t) + ctx := context.Background() + addSyncedWorkflow(provider, "wf-gone", "ws-1", "Old Flow", "flows/old.yml") + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", nil) + require.NoError(t, err) + assert.Equal(t, []string{"Old Flow"}, result.Deleted) + assert.Equal(t, []string{"wf-gone"}, ops.deleted) +} + +func TestApplySyncedWorkflows_KeepsVanishedWorkflowWithTasks(t *testing.T) { + svc, provider, ops := setupSyncService(t) + ctx := context.Background() + addSyncedWorkflow(provider, "wf-gone", "ws-1", "Old Flow", "flows/old.yml") + ops.tasksPerWorkflow["wf-gone"] = 3 + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", nil) + require.NoError(t, err) + assert.Empty(t, result.Deleted) + require.Len(t, result.Warnings, 1) + assert.Contains(t, result.Warnings[0], "Old Flow") + assert.Empty(t, ops.deleted) +} + +func TestApplySyncedWorkflows_BrokenFileFreezesItsWorkflows(t *testing.T) { + svc, provider, ops := setupSyncService(t) + ctx := context.Background() + addSyncedWorkflow(provider, "wf-frozen", "ws-1", "Frozen Flow", "flows/broken.yml") + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{ + {Path: "flows/broken.yml", Export: nil}, + }) + require.NoError(t, err) + assert.Empty(t, result.Deleted) + assert.Empty(t, ops.deleted, "workflows from unparseable files must not be deleted") +} + +func TestApplySyncedWorkflows_LeavesManualWorkflowsAlone(t *testing.T) { + svc, provider, ops := setupSyncService(t) + ctx := context.Background() + provider.addWorkflow("wf-manual", "ws-1", "Dev Flow") // manual workflow, same name + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(portableWorkflow("Dev Flow", "Todo"))}, + }) + require.NoError(t, err) + // A synced sibling is created; the manual workflow is neither updated nor deleted. + assert.Equal(t, []string{"Dev Flow"}, result.Created) + assert.Empty(t, ops.deleted) + + manual, err := provider.GetWorkflow(ctx, "wf-manual") + require.NoError(t, err) + assert.Equal(t, "", manual.Source) + assert.Equal(t, "", manual.Description) +} + +func TestApplySyncedWorkflows_DuplicateStepNamesInDefinitionWarn(t *testing.T) { + svc, provider, _ := setupSyncService(t) + ctx := context.Background() + wf := addSyncedWorkflow(provider, "wf-1", "ws-1", "Dev Flow", "flows/dev.yml") + createStep(t, svc, &models.WorkflowStep{ID: "step-todo", WorkflowID: wf.ID, Name: "Todo", Position: 0, IsStartStep: true}) + + pw := models.WorkflowPortable{ + Name: "Dev Flow", + Steps: []models.StepPortable{ + {Name: "Todo", Position: 0}, + {Name: "Todo", Position: 1}, + }, + } + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(pw)}, + }) + require.NoError(t, err) + assert.Empty(t, result.Updated) + require.Len(t, result.Warnings, 1) + assert.Contains(t, result.Warnings[0], "must be unique") +} + +func TestApplySyncedWorkflows_RemapsStepEventPositions(t *testing.T) { + svc, _, _ := setupSyncService(t) + ctx := context.Background() + + pw := models.WorkflowPortable{ + Name: "Flow", + Steps: []models.StepPortable{ + { + Name: "Start", Position: 0, IsStartStep: true, + Events: models.StepEvents{ + OnTurnComplete: []models.OnTurnCompleteAction{ + {Type: models.OnTurnCompleteMoveToStep, Config: map[string]any{"step_position": 1}}, + }, + }, + }, + {Name: "End", Position: 1}, + }, + } + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{ + {Path: "flows/flow.yml", Export: exportOf(pw)}, + }) + require.NoError(t, err) + require.Equal(t, []string{"Flow"}, result.Created) + + steps, err := svc.ListStepsByWorkflow(ctx, "imported-Flow") + require.NoError(t, err) + require.Len(t, steps, 2) + require.Len(t, steps[0].Events.OnTurnComplete, 1) + assert.Equal(t, steps[1].ID, steps[0].Events.OnTurnComplete[0].Config["step_id"]) +} + +func TestApplySyncedWorkflows_SecondApplyIsNoOp(t *testing.T) { + svc, _, _ := setupSyncService(t) + ctx := context.Background() + files := []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(portableWorkflow("Dev Flow", "Todo", "Done"))}, + } + + first, err := svc.ApplySyncedWorkflows(ctx, "ws-1", files) + require.NoError(t, err) + require.Equal(t, []string{"Dev Flow"}, first.Created) + + second, err := svc.ApplySyncedWorkflows(ctx, "ws-1", files) + require.NoError(t, err) + assert.Empty(t, second.Created) + assert.Empty(t, second.Updated, "no drift means nothing is rewritten or broadcast") + assert.Empty(t, second.Deleted) + assert.Empty(t, second.Warnings) +} + +func TestApplySyncedWorkflows_RepairsLocalStepEdit(t *testing.T) { + svc, _, _ := setupSyncService(t) + ctx := context.Background() + files := []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(portableWorkflow("Dev Flow", "Todo", "Done"))}, + } + _, err := svc.ApplySyncedWorkflows(ctx, "ws-1", files) + require.NoError(t, err) + + // A user recolors a synced step in the UI. + steps, err := svc.ListStepsByWorkflow(ctx, "imported-Dev Flow") + require.NoError(t, err) + steps[0].Color = "#123456" + require.NoError(t, svc.repo.UpdateStep(ctx, steps[0])) + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", files) + require.NoError(t, err) + assert.Equal(t, []string{"Dev Flow"}, result.Updated, "local drift is repaired") + + steps, err = svc.ListStepsByWorkflow(ctx, "imported-Dev Flow") + require.NoError(t, err) + assert.Equal(t, "#aabbcc", steps[0].Color, "definition wins over the local edit") +} + +func TestApplySyncedWorkflows_RecreatesLocallyDeletedStep(t *testing.T) { + svc, _, _ := setupSyncService(t) + ctx := context.Background() + files := []SyncFileExport{ + {Path: "flows/dev.yml", Export: exportOf(portableWorkflow("Dev Flow", "Todo", "Done"))}, + } + _, err := svc.ApplySyncedWorkflows(ctx, "ws-1", files) + require.NoError(t, err) + + steps, err := svc.ListStepsByWorkflow(ctx, "imported-Dev Flow") + require.NoError(t, err) + require.Len(t, steps, 2) + require.NoError(t, svc.DeleteStep(ctx, steps[1].ID)) + + result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", files) + require.NoError(t, err) + assert.Equal(t, []string{"Dev Flow"}, result.Updated) + + steps, err = svc.ListStepsByWorkflow(ctx, "imported-Dev Flow") + require.NoError(t, err) + require.Len(t, steps, 2, "locally deleted step is recreated from the definition") + assert.Equal(t, "Done", steps[1].Name) +} + +func TestEnsureWorkflowMutable(t *testing.T) { + svc, provider, _ := setupSyncService(t) + ctx := context.Background() + addSyncedWorkflow(provider, "wf-synced", "ws-1", "Synced Flow", "flows/dev.yml") + provider.addWorkflow("wf-manual", "ws-1", "Manual Flow") + + assert.ErrorIs(t, svc.EnsureWorkflowMutable(ctx, "wf-synced"), ErrWorkflowReadOnly) + assert.NoError(t, svc.EnsureWorkflowMutable(ctx, "wf-manual")) + assert.NoError(t, svc.EnsureWorkflowMutable(ctx, "wf-missing"), + "guard fails open on lookup errors; the mutation itself surfaces not-found") +} + +func TestReleaseSyncedWorkflows(t *testing.T) { + svc, provider, _ := setupSyncService(t) + ctx := context.Background() + addSyncedWorkflow(provider, "wf-a", "ws-1", "Flow A", "flows/a.yml") + addSyncedWorkflow(provider, "wf-b", "ws-1", "Flow B", "flows/b.yml") + provider.addWorkflow("wf-manual", "ws-1", "Manual Flow") + + released, err := svc.ReleaseSyncedWorkflows(ctx, "ws-1") + require.NoError(t, err) + assert.Equal(t, []string{"Flow A", "Flow B"}, released) + + for _, id := range []string{"wf-a", "wf-b"} { + wf, err := provider.GetWorkflow(ctx, id) + require.NoError(t, err) + assert.Equal(t, taskmodels.WorkflowSourceManual, wf.Source, "released workflows become manual") + assert.Empty(t, wf.SourcePath) + assert.NoError(t, svc.EnsureWorkflowMutable(ctx, id), "released workflows are editable again") + } +} diff --git a/apps/backend/internal/workflowsync/goleak_test.go b/apps/backend/internal/workflowsync/goleak_test.go new file mode 100644 index 0000000000..710d05bc7e --- /dev/null +++ b/apps/backend/internal/workflowsync/goleak_test.go @@ -0,0 +1,15 @@ +package workflowsync + +import ( + "testing" + + "go.uber.org/goleak" +) + +// TestMain enforces no goroutine leaks across the workflowsync package — the +// Poller owns a sync loop, lifecycle-managed via Start/Stop. Regressions +// where Stop forgets to cancel the context or drain the WaitGroup surface +// here. +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} diff --git a/apps/backend/internal/workflowsync/handlers.go b/apps/backend/internal/workflowsync/handlers.go new file mode 100644 index 0000000000..a7b69b177d --- /dev/null +++ b/apps/backend/internal/workflowsync/handlers.go @@ -0,0 +1,128 @@ +package workflowsync + +import ( + "errors" + "net/http" + "strings" + + "github.com/gin-gonic/gin" + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" +) + +// Controller holds HTTP route handlers for workflow sync. +type Controller struct { + service *Service + logger *logger.Logger +} + +// RegisterRoutes wires the workflow sync HTTP endpoints. +func RegisterRoutes(router *gin.Engine, svc *Service, log *logger.Logger) { + ctrl := &Controller{service: svc, logger: log} + api := router.Group("/api/v1/workflow-sync") + api.GET("/config", ctrl.httpGetConfig) + api.POST("/config", ctrl.httpSetConfig) + api.DELETE("/config", ctrl.httpDeleteConfig) + api.POST("/sync", ctrl.httpForceSync) +} + +func (c *Controller) workspaceID(ctx *gin.Context) string { + return strings.TrimSpace(ctx.Query("workspace_id")) +} + +func (c *Controller) requireWorkspaceID(ctx *gin.Context) (string, bool) { + id := c.workspaceID(ctx) + if id == "" { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id is required"}) + return "", false + } + return id, true +} + +func (c *Controller) httpGetConfig(ctx *gin.Context) { + workspaceID, ok := c.requireWorkspaceID(ctx) + if !ok { + return + } + cfg, err := c.service.GetConfigForWorkspace(ctx.Request.Context(), workspaceID) + if err != nil { + c.internalError(ctx, "failed to load workflow sync config", err) + return + } + if cfg == nil { + ctx.Status(http.StatusNoContent) + return + } + ctx.JSON(http.StatusOK, cfg) +} + +// internalError logs the underlying failure and returns a generic message so +// driver/query details never leak to clients. +func (c *Controller) internalError(ctx *gin.Context, msg string, err error) { + c.logger.Error(msg, zap.Error(err)) + ctx.JSON(http.StatusInternalServerError, gin.H{"error": msg}) +} + +func (c *Controller) httpSetConfig(ctx *gin.Context) { + workspaceID, ok := c.requireWorkspaceID(ctx) + if !ok { + return + } + var req SetConfigRequest + if err := ctx.ShouldBindJSON(&req); err != nil { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"}) + return + } + cfg, err := c.service.SetConfigForWorkspace(ctx.Request.Context(), workspaceID, &req) + if errors.Is(err, ErrInvalidConfig) { + ctx.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + if err != nil { + c.internalError(ctx, "failed to save workflow sync config", err) + return + } + ctx.JSON(http.StatusOK, cfg) +} + +func (c *Controller) httpDeleteConfig(ctx *gin.Context) { + workspaceID, ok := c.requireWorkspaceID(ctx) + if !ok { + return + } + if err := c.service.DeleteConfigForWorkspace(ctx.Request.Context(), workspaceID); err != nil { + c.internalError(ctx, "failed to remove workflow sync config", err) + return + } + ctx.JSON(http.StatusOK, gin.H{"deleted": true}) +} + +// httpForceSync runs a sync immediately instead of waiting for the poller +// (every sync — manual or periodic — reconciles, repairing local drift). Sync +// failures still return 200 with the error embedded — the outcome is also +// recorded on the config row, which is returned for the status banner. +func (c *Controller) httpForceSync(ctx *gin.Context) { + workspaceID, ok := c.requireWorkspaceID(ctx) + if !ok { + return + } + result, syncErr := c.service.SyncWorkspace(ctx.Request.Context(), workspaceID) + if errors.Is(syncErr, ErrNotConfigured) { + ctx.JSON(http.StatusNotFound, gin.H{"error": syncErr.Error()}) + return + } + cfg, err := c.service.GetConfigForWorkspace(ctx.Request.Context(), workspaceID) + if err != nil { + c.internalError(ctx, "failed to load workflow sync config", err) + return + } + response := gin.H{"config": cfg} + if syncErr != nil { + response["error"] = syncErr.Error() + } + if result != nil { + response["result"] = result + } + ctx.JSON(http.StatusOK, response) +} diff --git a/apps/backend/internal/workflowsync/models.go b/apps/backend/internal/workflowsync/models.go new file mode 100644 index 0000000000..14a5349755 --- /dev/null +++ b/apps/backend/internal/workflowsync/models.go @@ -0,0 +1,119 @@ +// Package workflowsync keeps workspace workflows in sync with definition +// files stored in a configured GitHub repository. A background poller fetches +// the configured directory on an interval and applies changes through the +// workflow service; users can also force a sync from the settings UI. Synced +// workflows coexist with manually-created ones — see workflows.source. +package workflowsync + +import ( + "errors" + "fmt" + "strings" + "time" + + "github.com/kandev/kandev/internal/common/securityutil" +) + +// Defaults for optional config fields. +const ( + DefaultBranch = "main" + DefaultPath = ".kandev/workflows" + DefaultIntervalSeconds = 300 + MinIntervalSeconds = 60 + // MaxIntervalSeconds caps the poll interval at 30 days — far beyond any + // sensible cadence, and safely below time.Duration overflow territory. + MaxIntervalSeconds = 30 * 24 * 60 * 60 +) + +// ErrInvalidConfig marks validation failures on SetConfigRequest so handlers +// can map them to 400. +var ErrInvalidConfig = errors.New("invalid workflow sync config") + +// ErrNotConfigured is returned when a sync is requested for a workspace that +// has no workflow sync config. +var ErrNotConfigured = errors.New("workflow sync is not configured for this workspace") + +// Config is the per-workspace workflow sync configuration plus the status of +// the most recent sync attempt (written by the poller and force syncs). +type Config struct { + WorkspaceID string `json:"workspace_id"` + RepoOwner string `json:"repo_owner"` + RepoName string `json:"repo_name"` + Branch string `json:"branch"` + Path string `json:"path"` + IntervalSeconds int `json:"interval_seconds"` + PollEnabled bool `json:"poll_enabled"` + LastSyncedAt *time.Time `json:"last_synced_at,omitempty"` + LastOk bool `json:"last_ok"` + LastError string `json:"last_error,omitempty"` + LastWarnings []string `json:"last_warnings,omitempty"` + LastHash string `json:"-"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// SetConfigRequest is the payload for creating or updating a workspace's +// workflow sync config. Branch, Path, and IntervalSeconds fall back to +// defaults when empty/zero. +type SetConfigRequest struct { + RepoOwner string `json:"repo_owner"` + RepoName string `json:"repo_name"` + Branch string `json:"branch"` + Path string `json:"path"` + IntervalSeconds int `json:"interval_seconds"` + // PollEnabled controls the background polling loop; nil defaults to + // true. When false the workspace only syncs via "Sync now". + PollEnabled *bool `json:"poll_enabled"` +} + +// Normalize validates the request and fills defaults. It returns a wrapped +// ErrInvalidConfig on bad input. +func (r *SetConfigRequest) Normalize() error { + r.RepoOwner = strings.TrimSpace(r.RepoOwner) + r.RepoName = strings.TrimSpace(r.RepoName) + r.Branch = strings.TrimSpace(r.Branch) + r.Path = strings.Trim(strings.TrimSpace(r.Path), "/") + if r.RepoOwner == "" || strings.ContainsAny(r.RepoOwner, "/ ") { + return fmt.Errorf("%w: repo_owner is required and cannot contain slashes or spaces", ErrInvalidConfig) + } + if r.RepoName == "" || strings.ContainsAny(r.RepoName, "/ ") { + return fmt.Errorf("%w: repo_name is required and cannot contain slashes or spaces", ErrInvalidConfig) + } + if r.Branch == "" { + r.Branch = DefaultBranch + } + if !securityutil.IsValidBranchName(r.Branch) { + return fmt.Errorf("%w: branch is not a valid git branch name", ErrInvalidConfig) + } + if r.Path == "" { + r.Path = DefaultPath + } + for _, segment := range strings.Split(r.Path, "/") { + if segment == ".." { + return fmt.Errorf("%w: path cannot contain \"..\"", ErrInvalidConfig) + } + } + if r.IntervalSeconds == 0 { + r.IntervalSeconds = DefaultIntervalSeconds + } + if r.IntervalSeconds < MinIntervalSeconds { + return fmt.Errorf("%w: interval_seconds must be at least %d", ErrInvalidConfig, MinIntervalSeconds) + } + if r.IntervalSeconds > MaxIntervalSeconds { + return fmt.Errorf("%w: interval_seconds must be at most %d", ErrInvalidConfig, MaxIntervalSeconds) + } + if r.PollEnabled == nil { + enabled := true + r.PollEnabled = &enabled + } + return nil +} + +// SyncResult reports the outcome of one sync run for a workspace. +type SyncResult struct { + Created []string `json:"created"` + Updated []string `json:"updated"` + Deleted []string `json:"deleted"` + Warnings []string `json:"warnings"` + Unchanged bool `json:"unchanged"` +} diff --git a/apps/backend/internal/workflowsync/models_test.go b/apps/backend/internal/workflowsync/models_test.go new file mode 100644 index 0000000000..697412e130 --- /dev/null +++ b/apps/backend/internal/workflowsync/models_test.go @@ -0,0 +1,62 @@ +package workflowsync + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSetConfigRequestNormalize_AppliesDefaults(t *testing.T) { + req := &SetConfigRequest{RepoOwner: " acme ", RepoName: "flows"} + require.NoError(t, req.Normalize()) + assert.Equal(t, "acme", req.RepoOwner) + assert.Equal(t, DefaultBranch, req.Branch) + assert.Equal(t, DefaultPath, req.Path) + assert.Equal(t, DefaultIntervalSeconds, req.IntervalSeconds) +} + +func TestSetConfigRequestNormalize_TrimsPathSlashes(t *testing.T) { + req := &SetConfigRequest{RepoOwner: "acme", RepoName: "flows", Path: "/workflows/prod/"} + require.NoError(t, req.Normalize()) + assert.Equal(t, "workflows/prod", req.Path) +} + +func TestSetConfigRequestNormalize_Rejections(t *testing.T) { + cases := map[string]*SetConfigRequest{ + "missing owner": {RepoName: "flows"}, + "owner with slash": {RepoOwner: "acme/evil", RepoName: "flows"}, + "missing repo": {RepoOwner: "acme"}, + "repo with space": {RepoOwner: "acme", RepoName: "my flows"}, + "path traversal": {RepoOwner: "acme", RepoName: "flows", Path: "a/../../etc"}, + "interval below min": {RepoOwner: "acme", RepoName: "flows", IntervalSeconds: 30}, + } + for name, req := range cases { + t.Run(name, func(t *testing.T) { + err := req.Normalize() + require.Error(t, err) + assert.True(t, errors.Is(err, ErrInvalidConfig)) + }) + } +} + +func TestSetConfigRequestNormalize_PollEnabledDefaultsTrue(t *testing.T) { + req := &SetConfigRequest{RepoOwner: "acme", RepoName: "flows"} + require.NoError(t, req.Normalize()) + require.NotNil(t, req.PollEnabled) + assert.True(t, *req.PollEnabled) + + disabled := false + req = &SetConfigRequest{RepoOwner: "acme", RepoName: "flows", PollEnabled: &disabled} + require.NoError(t, req.Normalize()) + assert.False(t, *req.PollEnabled, "explicit false survives normalization") +} + +func TestSetConfigRequestNormalize_IntervalAndBranchBounds(t *testing.T) { + req := &SetConfigRequest{RepoOwner: "acme", RepoName: "flows", IntervalSeconds: MaxIntervalSeconds + 1} + assert.ErrorIs(t, req.Normalize(), ErrInvalidConfig, "interval above the 30-day cap is rejected") + + req = &SetConfigRequest{RepoOwner: "acme", RepoName: "flows", Branch: "bad..ref"} + assert.ErrorIs(t, req.Normalize(), ErrInvalidConfig, "invalid git branch names are rejected") +} diff --git a/apps/backend/internal/workflowsync/poller.go b/apps/backend/internal/workflowsync/poller.go new file mode 100644 index 0000000000..a3d3817a03 --- /dev/null +++ b/apps/backend/internal/workflowsync/poller.go @@ -0,0 +1,81 @@ +package workflowsync + +import ( + "context" + "sync" + "time" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" +) + +// PollInterval is the outer ticker cadence. Each tick only syncs workspaces +// whose own interval_seconds has elapsed (see Service.SyncDueConfigs). +const PollInterval = 60 * time.Second + +// Poller periodically syncs workflow definitions for every configured +// workspace. Owns a single goroutine with Start/Stop lifecycle semantics. +type Poller struct { + svc *Service + logger *logger.Logger + interval time.Duration + + mu sync.Mutex + cancel context.CancelFunc + wg sync.WaitGroup + started bool +} + +// NewPoller creates a workflow sync poller at the default interval. +func NewPoller(svc *Service, log *logger.Logger) *Poller { + return &Poller{ + svc: svc, + logger: log.WithFields(zap.String("component", "workflowsync-poller")), + interval: PollInterval, + } +} + +// Start launches the polling loop. Idempotent. +func (p *Poller) Start(ctx context.Context) { + p.mu.Lock() + defer p.mu.Unlock() + if p.started { + return + } + p.started = true + ctx, p.cancel = context.WithCancel(ctx) + p.wg.Add(1) + go p.loop(ctx) +} + +// Stop cancels the polling loop and waits for it to drain. Idempotent. The +// mutex is held through the wait so a concurrent Start cannot register a new +// loop on the WaitGroup mid-shutdown (the loop itself never takes the mutex, +// so holding it here cannot deadlock). +func (p *Poller) Stop() { + p.mu.Lock() + defer p.mu.Unlock() + if !p.started { + return + } + p.started = false + p.cancel() + p.wg.Wait() +} + +// loop waits a full interval before the first sync so boot doesn't hammer +// the GitHub API, then syncs due configs on every tick. +func (p *Poller) loop(ctx context.Context) { + defer p.wg.Done() + ticker := time.NewTicker(p.interval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + p.svc.SyncDueConfigs(ctx) + } + } +} diff --git a/apps/backend/internal/workflowsync/poller_test.go b/apps/backend/internal/workflowsync/poller_test.go new file mode 100644 index 0000000000..59b5e83df6 --- /dev/null +++ b/apps/backend/internal/workflowsync/poller_test.go @@ -0,0 +1,48 @@ +package workflowsync + +import ( + "context" + "testing" + "testing/synctest" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPoller_StartStopIdempotent(t *testing.T) { + svc, _ := setupTestService(t, seededMockClient()) + p := NewPoller(svc, svc.logger) + + p.Start(context.Background()) + t.Cleanup(p.Stop) + p.Start(context.Background()) // second start is a no-op + p.Stop() + p.Stop() // second stop is a no-op +} + +func TestPoller_SyncsDueConfigsOnTick(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + svc, applier := setupTestService(t, seededMockClient()) + configureWorkspace(t, svc, "ws-1") + + p := NewPoller(svc, svc.logger) + p.Start(context.Background()) + t.Cleanup(p.Stop) + + // No sync before the first tick: the loop waits a full interval so + // boot doesn't hammer the GitHub API. + synctest.Wait() + assert.Zero(t, applier.callCount()) + + time.Sleep(PollInterval + time.Second) + synctest.Wait() + p.Stop() + + require.Equal(t, 1, applier.callCount()) + cfg, err := svc.GetConfigForWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + assert.True(t, cfg.LastOk) + assert.NotNil(t, cfg.LastSyncedAt) + }) +} diff --git a/apps/backend/internal/workflowsync/provider.go b/apps/backend/internal/workflowsync/provider.go new file mode 100644 index 0000000000..e8fc4e5e1b --- /dev/null +++ b/apps/backend/internal/workflowsync/provider.go @@ -0,0 +1,20 @@ +package workflowsync + +import ( + "github.com/jmoiron/sqlx" + + "github.com/kandev/kandev/internal/common/logger" +) + +// Provide builds the workflow sync service. Cleanup is a no-op today but the +// signature mirrors other integration providers so callers can register it +// uniformly. +func Provide(writer, reader *sqlx.DB, clients ClientProvider, applier Applier, log *logger.Logger) (*Service, func() error, error) { + store, err := NewStore(writer, reader) + if err != nil { + return nil, nil, err + } + svc := NewService(store, clients, applier, log) + cleanup := func() error { return nil } + return svc, cleanup, nil +} diff --git a/apps/backend/internal/workflowsync/service.go b/apps/backend/internal/workflowsync/service.go new file mode 100644 index 0000000000..607cf51d38 --- /dev/null +++ b/apps/backend/internal/workflowsync/service.go @@ -0,0 +1,281 @@ +package workflowsync + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "sort" + "strings" + "sync" + "time" + + "go.uber.org/zap" + "gopkg.in/yaml.v3" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/github" + workflowmodels "github.com/kandev/kandev/internal/workflow/models" + workflowservice "github.com/kandev/kandev/internal/workflow/service" +) + +// Applier applies parsed workflow definition files to a workspace and +// releases synced workflows back to manual ownership when syncing stops. +// Satisfied by the workflow service. +type Applier interface { + ApplySyncedWorkflows(ctx context.Context, workspaceID string, files []workflowservice.SyncFileExport) (*workflowservice.SyncApplyResult, error) + ReleaseSyncedWorkflows(ctx context.Context, workspaceID string) ([]string, error) +} + +// ClientProvider hands out the GitHub client used to read the sync repo. +// Satisfied by the GitHub service; Client() may return nil when GitHub is not +// authenticated. +type ClientProvider interface { + Client() github.Client +} + +// Service owns workflow sync configuration and sync execution. +type Service struct { + store *Store + clients ClientProvider + applier Applier + logger *logger.Logger + // locks serializes syncs and config mutations per workspace so a force + // sync cannot interleave with a config delete/replace and apply stale + // definitions (or re-stamp workflows that were just released). The lock + // is deliberately held across the GitHub fetch too: a config change or + // delete for that workspace waits (bounded by the HTTP client timeout) + // rather than racing an in-flight apply. + locks sync.Map // workspaceID → *sync.Mutex +} + +func (s *Service) workspaceLock(workspaceID string) *sync.Mutex { + lock, _ := s.locks.LoadOrStore(workspaceID, &sync.Mutex{}) + return lock.(*sync.Mutex) +} + +// NewService creates a workflow sync service. +func NewService(store *Store, clients ClientProvider, applier Applier, log *logger.Logger) *Service { + return &Service{ + store: store, + clients: clients, + applier: applier, + logger: log.WithFields(zap.String("component", "workflowsync-service")), + } +} + +// Store exposes the config store (e2e reset cascade). +func (s *Service) Store() *Store { + return s.store +} + +// GetConfigForWorkspace returns the workspace's config, or nil when unset. +func (s *Service) GetConfigForWorkspace(ctx context.Context, workspaceID string) (*Config, error) { + return s.store.GetConfigForWorkspace(ctx, workspaceID) +} + +// SetConfigForWorkspace validates and stores the workspace's config. +func (s *Service) SetConfigForWorkspace(ctx context.Context, workspaceID string, req *SetConfigRequest) (*Config, error) { + if err := req.Normalize(); err != nil { + return nil, err + } + lock := s.workspaceLock(workspaceID) + lock.Lock() + defer lock.Unlock() + return s.store.UpsertConfigForWorkspace(ctx, workspaceID, req) +} + +// DeleteConfigForWorkspace removes the workspace's config. Previously-synced +// workflows are released back to manual ownership first so they become +// editable again — a failed release keeps the config so the user can retry. +func (s *Service) DeleteConfigForWorkspace(ctx context.Context, workspaceID string) error { + lock := s.workspaceLock(workspaceID) + lock.Lock() + defer lock.Unlock() + released, err := s.applier.ReleaseSyncedWorkflows(ctx, workspaceID) + if err != nil { + return fmt.Errorf("failed to release synced workflows: %w", err) + } + if len(released) > 0 { + s.logger.Info("released synced workflows", + zap.String("workspace_id", workspaceID), zap.Int("count", len(released))) + } + return s.store.DeleteConfigForWorkspace(ctx, workspaceID) +} + +// syncableExtensions are the file extensions read from the sync directory. +var syncableExtensions = []string{".yml", ".yaml", ".json"} + +func isSyncableFile(name string) bool { + lower := strings.ToLower(name) + for _, ext := range syncableExtensions { + if strings.HasSuffix(lower, ext) { + return true + } + } + return false +} + +// fetchedFile is one definition file read from the repo. +type fetchedFile struct { + path string + content []byte +} + +// SyncWorkspace fetches the configured repo directory and reconciles the +// workspace's synced workflows with it. Every sync applies the definitions — +// including repairing local edits to synced workflows — but the applier only +// writes (and broadcasts) what actually differs, so a no-drift sync is +// silent. The outcome (including failures) is recorded on the config row so +// the UI can surface it. +func (s *Service) SyncWorkspace(ctx context.Context, workspaceID string) (*SyncResult, error) { + lock := s.workspaceLock(workspaceID) + lock.Lock() + defer lock.Unlock() + cfg, err := s.store.GetConfigForWorkspace(ctx, workspaceID) + if err != nil { + return nil, err + } + if cfg == nil { + return nil, ErrNotConfigured + } + + files, err := s.fetchFiles(ctx, cfg) + if err != nil { + s.recordFailure(ctx, workspaceID, err) + return nil, err + } + + parsed, warnings := parseFiles(files) + applied, err := s.applier.ApplySyncedWorkflows(ctx, workspaceID, parsed) + if err != nil { + s.recordFailure(ctx, workspaceID, err) + return nil, err + } + warnings = append(warnings, applied.Warnings...) + if err := s.store.RecordSyncStatus(ctx, workspaceID, true, "", warnings, contentHash(files), time.Now().UTC()); err != nil { + return nil, err + } + return &SyncResult{ + Created: applied.Created, + Updated: applied.Updated, + Deleted: applied.Deleted, + Warnings: warnings, + Unchanged: len(applied.Created)+len(applied.Updated)+len(applied.Deleted) == 0 && len(warnings) == 0, + }, nil +} + +func (s *Service) recordFailure(ctx context.Context, workspaceID string, syncErr error) { + // Clear the hash so the next successful fetch re-applies from scratch. + if err := s.store.RecordSyncStatus(ctx, workspaceID, false, syncErr.Error(), nil, "", time.Now().UTC()); err != nil { + s.logger.Warn("failed to record sync failure", + zap.String("workspace_id", workspaceID), zap.Error(err)) + } +} + +// fetchFiles lists the configured directory and downloads every workflow +// definition file in it (non-recursive). +func (s *Service) fetchFiles(ctx context.Context, cfg *Config) ([]fetchedFile, error) { + client := s.clients.Client() + if client == nil { + return nil, fmt.Errorf("GitHub is not authenticated; configure a GitHub token to sync workflows") + } + entries, err := client.ListRepoDirectory(ctx, cfg.RepoOwner, cfg.RepoName, cfg.Path, cfg.Branch) + if err != nil { + return nil, fmt.Errorf("failed to list %s/%s@%s:%s: %w", cfg.RepoOwner, cfg.RepoName, cfg.Branch, cfg.Path, err) + } + var files []fetchedFile + for _, entry := range entries { + if entry.Type != "file" || !isSyncableFile(entry.Name) { + continue + } + content, err := client.GetRepoFileContent(ctx, cfg.RepoOwner, cfg.RepoName, entry.Path, cfg.Branch) + if err != nil { + return nil, fmt.Errorf("failed to fetch %s: %w", entry.Path, err) + } + files = append(files, fetchedFile{path: entry.Path, content: content}) + } + sort.Slice(files, func(i, j int) bool { return files[i].path < files[j].path }) + return files, nil +} + +// contentHash is a stable digest of the fetched file set. It is recorded on +// the config row for observability only — every sync reconciles regardless +// (repairing local drift), with the applier writing only actual differences. +func contentHash(files []fetchedFile) string { + h := sha256.New() + for _, f := range files { + _, _ = fmt.Fprintf(h, "%s\x00%d\x00", f.path, len(f.content)) + h.Write(f.content) + } + return hex.EncodeToString(h.Sum(nil)) +} + +// parseFiles decodes and validates each fetched file. Files that fail to +// parse are reported as warnings and passed through with a nil export, which +// tells the applier to leave their previously-synced workflows untouched. +func parseFiles(files []fetchedFile) ([]workflowservice.SyncFileExport, []string) { + parsed := make([]workflowservice.SyncFileExport, 0, len(files)) + var warnings []string + for _, f := range files { + export, err := parseExport(f.path, f.content) + if err != nil { + warnings = append(warnings, fmt.Sprintf("%s: %v", f.path, err)) + parsed = append(parsed, workflowservice.SyncFileExport{Path: f.path}) + continue + } + parsed = append(parsed, workflowservice.SyncFileExport{Path: f.path, Export: export}) + } + return parsed, warnings +} + +func parseExport(path string, data []byte) (*workflowmodels.WorkflowExport, error) { + export := &workflowmodels.WorkflowExport{} + var err error + if strings.HasSuffix(strings.ToLower(path), ".json") { + err = json.Unmarshal(data, export) + } else { + err = yaml.Unmarshal(data, export) + } + if err != nil { + return nil, fmt.Errorf("not a valid workflow export file: %w", err) + } + if err := export.Validate(); err != nil { + return nil, err + } + return export, nil +} + +// SyncDueConfigs runs a periodic sync for every workspace whose interval has +// elapsed. Failures are recorded on the config row and logged, never fatal. +func (s *Service) SyncDueConfigs(ctx context.Context) { + configs, err := s.store.ListConfigs(ctx) + if err != nil { + s.logger.Warn("failed to list workflow sync configs", zap.Error(err)) + return + } + now := time.Now().UTC() + for _, cfg := range configs { + if ctx.Err() != nil { + return + } + if !isSyncDue(cfg, now) { + continue + } + if _, err := s.SyncWorkspace(ctx, cfg.WorkspaceID); err != nil { + s.logger.Warn("periodic workflow sync failed", + zap.String("workspace_id", cfg.WorkspaceID), zap.Error(err)) + } + } +} + +func isSyncDue(cfg *Config, now time.Time) bool { + if !cfg.PollEnabled { + return false + } + if cfg.LastSyncedAt == nil { + return true + } + return now.Sub(*cfg.LastSyncedAt) >= time.Duration(cfg.IntervalSeconds)*time.Second +} diff --git a/apps/backend/internal/workflowsync/service_test.go b/apps/backend/internal/workflowsync/service_test.go new file mode 100644 index 0000000000..96f48ed3d3 --- /dev/null +++ b/apps/backend/internal/workflowsync/service_test.go @@ -0,0 +1,238 @@ +package workflowsync + +import ( + "context" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/github" + workflowservice "github.com/kandev/kandev/internal/workflow/service" +) + +type fakeClients struct { + client github.Client +} + +func (f fakeClients) Client() github.Client { return f.client } + +// fakeApplier is mutex-guarded because the poller invokes it from its own +// goroutine while tests assert on call counts (-race catches unguarded use). +type fakeApplier struct { + mu sync.Mutex + calls [][]workflowservice.SyncFileExport + result *workflowservice.SyncApplyResult + released []string +} + +func (f *fakeApplier) ApplySyncedWorkflows(_ context.Context, _ string, files []workflowservice.SyncFileExport) (*workflowservice.SyncApplyResult, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, files) + if f.result != nil { + return f.result, nil + } + return &workflowservice.SyncApplyResult{}, nil +} + +func (f *fakeApplier) ReleaseSyncedWorkflows(_ context.Context, workspaceID string) ([]string, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.released = append(f.released, workspaceID) + return []string{"Dev Flow"}, nil +} + +func (f *fakeApplier) callCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.calls) +} + +const validExportYAML = `version: 1 +type: kandev_workflow +workflows: + - name: Dev Flow + steps: + - name: Todo + position: 0 + is_start_step: true +` + +func setupTestService(t *testing.T, client github.Client) (*Service, *fakeApplier) { + t.Helper() + store := setupTestStore(t) + applier := &fakeApplier{} + log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "console"}) + require.NoError(t, err) + return NewService(store, fakeClients{client: client}, applier, log), applier +} + +func configureWorkspace(t *testing.T, svc *Service, workspaceID string) { + t.Helper() + _, err := svc.SetConfigForWorkspace(context.Background(), workspaceID, &SetConfigRequest{ + RepoOwner: "acme", + RepoName: "flows", + }) + require.NoError(t, err) +} + +func seededMockClient() *github.MockClient { + mock := github.NewMockClient() + mock.SeedRepoFile("acme", "flows", "main", DefaultPath+"/dev.yml", []byte(validExportYAML)) + return mock +} + +func TestSyncWorkspace_NotConfigured(t *testing.T) { + svc, _ := setupTestService(t, github.NewMockClient()) + _, err := svc.SyncWorkspace(context.Background(), "ws-1") + assert.ErrorIs(t, err, ErrNotConfigured) +} + +func TestSyncWorkspace_AppliesParsedFiles(t *testing.T) { + svc, applier := setupTestService(t, seededMockClient()) + configureWorkspace(t, svc, "ws-1") + applier.result = &workflowservice.SyncApplyResult{Created: []string{"Dev Flow"}} + + result, err := svc.SyncWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + assert.Equal(t, []string{"Dev Flow"}, result.Created) + assert.False(t, result.Unchanged) + + require.Len(t, applier.calls, 1) + require.Len(t, applier.calls[0], 1) + file := applier.calls[0][0] + assert.Equal(t, DefaultPath+"/dev.yml", file.Path) + require.NotNil(t, file.Export) + assert.Equal(t, "Dev Flow", file.Export.Workflows[0].Name) + + cfg, err := svc.GetConfigForWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + assert.True(t, cfg.LastOk) + assert.NotNil(t, cfg.LastSyncedAt) + assert.NotEmpty(t, cfg.LastHash) + assert.Empty(t, cfg.LastError) +} + +func TestSyncWorkspace_AlwaysReconciles(t *testing.T) { + svc, applier := setupTestService(t, seededMockClient()) + configureWorkspace(t, svc, "ws-1") + + _, err := svc.SyncWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + result, err := svc.SyncWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + // Every sync applies (repairing local edits to synced workflows); the + // applier reports nothing changed, which surfaces as Unchanged. + assert.Len(t, applier.calls, 2) + assert.True(t, result.Unchanged) +} + +func TestSyncWorkspace_BrokenFileBecomesWarningAndNilExport(t *testing.T) { + mock := seededMockClient() + mock.SeedRepoFile("acme", "flows", "main", DefaultPath+"/broken.yml", []byte("::not yaml::")) + svc, applier := setupTestService(t, mock) + configureWorkspace(t, svc, "ws-1") + + result, err := svc.SyncWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + require.Len(t, result.Warnings, 1) + assert.Contains(t, result.Warnings[0], "broken.yml") + + require.Len(t, applier.calls, 1) + require.Len(t, applier.calls[0], 2) + byPath := map[string]workflowservice.SyncFileExport{} + for _, f := range applier.calls[0] { + byPath[f.Path] = f + } + assert.Nil(t, byPath[DefaultPath+"/broken.yml"].Export) + assert.NotNil(t, byPath[DefaultPath+"/dev.yml"].Export) + + cfg, err := svc.GetConfigForWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + assert.True(t, cfg.LastOk, "parse warnings do not fail the sync") + require.Len(t, cfg.LastWarnings, 1) +} + +func TestSyncWorkspace_IgnoresNonWorkflowFiles(t *testing.T) { + mock := seededMockClient() + mock.SeedRepoFile("acme", "flows", "main", DefaultPath+"/README.md", []byte("# docs")) + svc, applier := setupTestService(t, mock) + configureWorkspace(t, svc, "ws-1") + + _, err := svc.SyncWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + require.Len(t, applier.calls, 1) + assert.Len(t, applier.calls[0], 1, "only .yml/.yaml/.json files are synced") +} + +func TestSyncWorkspace_MissingDirectoryRecordsFailure(t *testing.T) { + svc, _ := setupTestService(t, github.NewMockClient()) // nothing seeded → 404 + configureWorkspace(t, svc, "ws-1") + + _, err := svc.SyncWorkspace(context.Background(), "ws-1") + require.Error(t, err) + + cfg, cfgErr := svc.GetConfigForWorkspace(context.Background(), "ws-1") + require.NoError(t, cfgErr) + assert.False(t, cfg.LastOk) + assert.NotEmpty(t, cfg.LastError) + assert.NotNil(t, cfg.LastSyncedAt) +} + +func TestSyncWorkspace_NilClientRecordsFailure(t *testing.T) { + svc, _ := setupTestService(t, nil) + configureWorkspace(t, svc, "ws-1") + + _, err := svc.SyncWorkspace(context.Background(), "ws-1") + require.Error(t, err) + assert.Contains(t, err.Error(), "not authenticated") + + cfg, cfgErr := svc.GetConfigForWorkspace(context.Background(), "ws-1") + require.NoError(t, cfgErr) + assert.False(t, cfg.LastOk) +} + +func TestSyncDueConfigs_HonorsInterval(t *testing.T) { + svc, applier := setupTestService(t, seededMockClient()) + configureWorkspace(t, svc, "ws-1") + + svc.SyncDueConfigs(context.Background()) + require.Len(t, applier.calls, 1, "first sync runs immediately (never synced)") + + svc.SyncDueConfigs(context.Background()) + assert.Len(t, applier.calls, 1, "second run within the interval is skipped entirely") +} + +func TestSyncDueConfigs_SkipsPollingDisabled(t *testing.T) { + svc, applier := setupTestService(t, seededMockClient()) + disabled := false + _, err := svc.SetConfigForWorkspace(context.Background(), "ws-1", &SetConfigRequest{ + RepoOwner: "acme", + RepoName: "flows", + PollEnabled: &disabled, + }) + require.NoError(t, err) + + svc.SyncDueConfigs(context.Background()) + assert.Empty(t, applier.calls, "polling-disabled configs never auto-sync") + + // Manual sync still works. + _, err = svc.SyncWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + assert.Len(t, applier.calls, 1) +} + +func TestDeleteConfigForWorkspace_ReleasesSyncedWorkflows(t *testing.T) { + svc, applier := setupTestService(t, seededMockClient()) + configureWorkspace(t, svc, "ws-1") + + require.NoError(t, svc.DeleteConfigForWorkspace(context.Background(), "ws-1")) + assert.Equal(t, []string{"ws-1"}, applier.released, "deleting the config releases its workflows") + + cfg, err := svc.GetConfigForWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + assert.Nil(t, cfg) +} diff --git a/apps/backend/internal/workflowsync/store.go b/apps/backend/internal/workflowsync/store.go new file mode 100644 index 0000000000..e7f1c385bb --- /dev/null +++ b/apps/backend/internal/workflowsync/store.go @@ -0,0 +1,205 @@ +package workflowsync + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + "time" + + "github.com/jmoiron/sqlx" + + "github.com/kandev/kandev/internal/db" +) + +// Store persists workspace-scoped workflow sync configuration. +type Store struct { + db *sqlx.DB + ro *sqlx.DB +} + +// NewStore creates a new Store and initializes the schema if needed. +func NewStore(writer, reader *sqlx.DB) (*Store, error) { + s := &Store{db: writer, ro: reader} + if err := s.initSchema(); err != nil { + return nil, fmt.Errorf("workflowsync schema init: %w", err) + } + return s, nil +} + +const createTablesSQL = ` + CREATE TABLE IF NOT EXISTS workflow_sync_configs ( + workspace_id TEXT PRIMARY KEY, + repo_owner TEXT NOT NULL, + repo_name TEXT NOT NULL, + branch TEXT NOT NULL DEFAULT 'main', + path TEXT NOT NULL DEFAULT '', + interval_seconds INTEGER NOT NULL DEFAULT 300, + poll_enabled INTEGER NOT NULL DEFAULT 1, + last_synced_at DATETIME, + last_ok INTEGER NOT NULL DEFAULT 0, + last_error TEXT NOT NULL DEFAULT '', + last_warnings TEXT NOT NULL DEFAULT '[]', + last_hash TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + ); +` + +func (s *Store) initSchema() error { + if _, err := s.db.Exec(createTablesSQL); err != nil { + return err + } + return s.addPollEnabledColumn() +} + +// addPollEnabledColumn brings databases created before the poll toggle up to +// the current schema. Idempotent and race-safe: the ALTER always runs and a +// "duplicate column name" failure is swallowed via the shared helper. +func (s *Store) addPollEnabledColumn() error { + _, err := s.db.Exec(`ALTER TABLE workflow_sync_configs ADD COLUMN poll_enabled INTEGER NOT NULL DEFAULT 1`) + if err != nil && !db.IsDuplicateColumnError(err) { + return err + } + return nil +} + +const configSelectColumns = `workspace_id, repo_owner, repo_name, branch, path, interval_seconds, + poll_enabled, last_synced_at, last_ok, last_error, last_warnings, last_hash, created_at, updated_at` + +type configScanner interface { + Scan(dest ...interface{}) error +} + +func scanConfig(row configScanner) (*Config, error) { + cfg := &Config{} + var lastOk, pollEnabled int + var lastSyncedAt sql.NullTime + var warningsJSON string + if err := row.Scan( + &cfg.WorkspaceID, + &cfg.RepoOwner, + &cfg.RepoName, + &cfg.Branch, + &cfg.Path, + &cfg.IntervalSeconds, + &pollEnabled, + &lastSyncedAt, + &lastOk, + &cfg.LastError, + &warningsJSON, + &cfg.LastHash, + &cfg.CreatedAt, + &cfg.UpdatedAt, + ); err != nil { + return nil, err + } + cfg.LastOk = lastOk != 0 + cfg.PollEnabled = pollEnabled != 0 + if lastSyncedAt.Valid { + t := lastSyncedAt.Time + cfg.LastSyncedAt = &t + } + if warningsJSON != "" { + // Corrupt JSON degrades to no warnings rather than failing the read. + _ = json.Unmarshal([]byte(warningsJSON), &cfg.LastWarnings) + } + return cfg, nil +} + +// GetConfigForWorkspace returns the config for a workspace, or (nil, nil) +// when none is stored. +func (s *Store) GetConfigForWorkspace(ctx context.Context, workspaceID string) (*Config, error) { + row := s.ro.QueryRowContext(ctx, s.ro.Rebind(` + SELECT `+configSelectColumns+` FROM workflow_sync_configs WHERE workspace_id = ? + `), workspaceID) + cfg, err := scanConfig(row) + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, err + } + return cfg, nil +} + +// ListConfigs returns every stored config, for the background poller. +func (s *Store) ListConfigs(ctx context.Context) ([]*Config, error) { + rows, err := s.ro.QueryContext(ctx, `SELECT `+configSelectColumns+` FROM workflow_sync_configs ORDER BY workspace_id`) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + var configs []*Config + for rows.Next() { + cfg, err := scanConfig(rows) + if err != nil { + return nil, err + } + configs = append(configs, cfg) + } + return configs, rows.Err() +} + +// UpsertConfigForWorkspace creates or replaces a workspace's config. The sync +// status columns are reset so the next sync re-fetches and re-applies. +func (s *Store) UpsertConfigForWorkspace(ctx context.Context, workspaceID string, req *SetConfigRequest) (*Config, error) { + now := time.Now().UTC() + _, err := s.db.ExecContext(ctx, s.db.Rebind(` + INSERT INTO workflow_sync_configs ( + workspace_id, repo_owner, repo_name, branch, path, interval_seconds, poll_enabled, + last_synced_at, last_ok, last_error, last_warnings, last_hash, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, NULL, 0, '', '[]', '', ?, ?) + ON CONFLICT(workspace_id) DO UPDATE SET + repo_owner = excluded.repo_owner, + repo_name = excluded.repo_name, + branch = excluded.branch, + path = excluded.path, + interval_seconds = excluded.interval_seconds, + poll_enabled = excluded.poll_enabled, + last_synced_at = NULL, + last_ok = 0, + last_error = '', + last_warnings = '[]', + last_hash = '', + updated_at = excluded.updated_at + `), workspaceID, req.RepoOwner, req.RepoName, req.Branch, req.Path, req.IntervalSeconds, boolToInt(req.PollEnabled != nil && *req.PollEnabled), now, now) + if err != nil { + return nil, err + } + return s.GetConfigForWorkspace(ctx, workspaceID) +} + +// RecordSyncStatus persists the outcome of a sync attempt. +func (s *Store) RecordSyncStatus(ctx context.Context, workspaceID string, ok bool, errMsg string, warnings []string, hash string, at time.Time) error { + warningsJSON, err := json.Marshal(warnings) + if err != nil { + warningsJSON = []byte("[]") + } + okInt := 0 + if ok { + okInt = 1 + } + _, err = s.db.ExecContext(ctx, s.db.Rebind(` + UPDATE workflow_sync_configs + SET last_synced_at = ?, last_ok = ?, last_error = ?, last_warnings = ?, last_hash = ?, updated_at = ? + WHERE workspace_id = ? + `), at, okInt, errMsg, string(warningsJSON), hash, at, workspaceID) + return err +} + +// DeleteConfigForWorkspace removes a workspace's config. Deleting a missing +// config is a no-op. +func (s *Store) DeleteConfigForWorkspace(ctx context.Context, workspaceID string) error { + _, err := s.db.ExecContext(ctx, s.db.Rebind(`DELETE FROM workflow_sync_configs WHERE workspace_id = ?`), workspaceID) + return err +} + +func boolToInt(b bool) int { + if b { + return 1 + } + return 0 +} diff --git a/apps/backend/internal/workflowsync/store_test.go b/apps/backend/internal/workflowsync/store_test.go new file mode 100644 index 0000000000..b6eda9c47a --- /dev/null +++ b/apps/backend/internal/workflowsync/store_test.go @@ -0,0 +1,139 @@ +package workflowsync + +import ( + "context" + "database/sql" + "testing" + "time" + + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func setupTestStore(t *testing.T) *Store { + t.Helper() + rawDB, err := sql.Open("sqlite3", ":memory:") + require.NoError(t, err) + // Pin to one connection: each new connection to an in-memory SQLite DB + // gets its own isolated database, which makes pooled access flaky. + rawDB.SetMaxOpenConns(1) + db := sqlx.NewDb(rawDB, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + store, err := NewStore(db, db) + require.NoError(t, err) + return store +} + +func testRequest() *SetConfigRequest { + req := &SetConfigRequest{RepoOwner: "acme", RepoName: "flows"} + if err := req.Normalize(); err != nil { + panic(err) + } + return req +} + +func TestStore_GetConfigForWorkspace_MissingReturnsNil(t *testing.T) { + store := setupTestStore(t) + cfg, err := store.GetConfigForWorkspace(context.Background(), "ws-1") + require.NoError(t, err) + assert.Nil(t, cfg) +} + +func TestStore_UpsertAndGetRoundtrip(t *testing.T) { + store := setupTestStore(t) + ctx := context.Background() + + cfg, err := store.UpsertConfigForWorkspace(ctx, "ws-1", testRequest()) + require.NoError(t, err) + assert.Equal(t, "acme", cfg.RepoOwner) + assert.Equal(t, "flows", cfg.RepoName) + assert.Equal(t, DefaultBranch, cfg.Branch) + assert.Equal(t, DefaultPath, cfg.Path) + assert.Equal(t, DefaultIntervalSeconds, cfg.IntervalSeconds) + assert.Nil(t, cfg.LastSyncedAt) + assert.False(t, cfg.LastOk) +} + +func TestStore_UpsertResetsSyncStatus(t *testing.T) { + store := setupTestStore(t) + ctx := context.Background() + _, err := store.UpsertConfigForWorkspace(ctx, "ws-1", testRequest()) + require.NoError(t, err) + require.NoError(t, store.RecordSyncStatus(ctx, "ws-1", true, "", []string{"w1"}, "hash-1", time.Now().UTC())) + + req := testRequest() + req.RepoName = "other" + cfg, err := store.UpsertConfigForWorkspace(ctx, "ws-1", req) + require.NoError(t, err) + assert.Equal(t, "other", cfg.RepoName) + assert.Nil(t, cfg.LastSyncedAt, "changing the config resets sync status") + assert.Empty(t, cfg.LastHash) + assert.Empty(t, cfg.LastWarnings) +} + +func TestStore_RecordSyncStatusRoundtrip(t *testing.T) { + store := setupTestStore(t) + ctx := context.Background() + _, err := store.UpsertConfigForWorkspace(ctx, "ws-1", testRequest()) + require.NoError(t, err) + + at := time.Now().UTC().Truncate(time.Second) + warnings := []string{"workflow \"X\" not updated", "flows/broken.yml: bad yaml"} + require.NoError(t, store.RecordSyncStatus(ctx, "ws-1", false, "boom", warnings, "hash-2", at)) + + cfg, err := store.GetConfigForWorkspace(ctx, "ws-1") + require.NoError(t, err) + require.NotNil(t, cfg.LastSyncedAt) + assert.False(t, cfg.LastOk) + assert.Equal(t, "boom", cfg.LastError) + assert.Equal(t, warnings, cfg.LastWarnings) + assert.Equal(t, "hash-2", cfg.LastHash) +} + +func TestStore_ListConfigs(t *testing.T) { + store := setupTestStore(t) + ctx := context.Background() + _, err := store.UpsertConfigForWorkspace(ctx, "ws-b", testRequest()) + require.NoError(t, err) + _, err = store.UpsertConfigForWorkspace(ctx, "ws-a", testRequest()) + require.NoError(t, err) + + configs, err := store.ListConfigs(ctx) + require.NoError(t, err) + require.Len(t, configs, 2) + assert.Equal(t, "ws-a", configs[0].WorkspaceID) + assert.Equal(t, "ws-b", configs[1].WorkspaceID) +} + +func TestStore_DeleteConfigForWorkspace(t *testing.T) { + store := setupTestStore(t) + ctx := context.Background() + _, err := store.UpsertConfigForWorkspace(ctx, "ws-1", testRequest()) + require.NoError(t, err) + + require.NoError(t, store.DeleteConfigForWorkspace(ctx, "ws-1")) + cfg, err := store.GetConfigForWorkspace(ctx, "ws-1") + require.NoError(t, err) + assert.Nil(t, cfg) + + // Deleting a missing config is a no-op. + require.NoError(t, store.DeleteConfigForWorkspace(ctx, "ws-1")) +} + +func TestStore_PollEnabledRoundtrip(t *testing.T) { + store := setupTestStore(t) + ctx := context.Background() + + cfg, err := store.UpsertConfigForWorkspace(ctx, "ws-1", testRequest()) + require.NoError(t, err) + assert.True(t, cfg.PollEnabled, "polling defaults to enabled") + + req := testRequest() + disabled := false + req.PollEnabled = &disabled + cfg, err = store.UpsertConfigForWorkspace(ctx, "ws-1", req) + require.NoError(t, err) + assert.False(t, cfg.PollEnabled) +} diff --git a/apps/web/app/settings/workspace/workspace-workflows-client.tsx b/apps/web/app/settings/workspace/workspace-workflows-client.tsx index 3e17a4f772..37020eacc6 100644 --- a/apps/web/app/settings/workspace/workspace-workflows-client.tsx +++ b/apps/web/app/settings/workspace/workspace-workflows-client.tsx @@ -3,13 +3,7 @@ import { useMemo, useRef, useState } from "react"; import Link from "@/components/routing/app-link"; import { useRouter } from "@/lib/routing/client-router"; -import { - IconDownload, - IconGripVertical, - IconArrowsShuffle, - IconPlus, - IconUpload, -} from "@tabler/icons-react"; +import { IconGripVertical, IconArrowsShuffle } from "@tabler/icons-react"; import { DndContext, closestCenter, @@ -29,6 +23,8 @@ import { Button } from "@kandev/ui/button"; import { Separator } from "@kandev/ui/separator"; import { SettingsSection } from "@/components/settings/settings-section"; import { WorkflowCard } from "@/components/settings/workflow-card"; +import { WorkflowSectionActions } from "@/components/settings/workflow-section-actions"; +import { WorkflowSyncSection } from "@/components/settings/workflow-sync-section"; import { WorkflowExportDialog } from "@/components/settings/workflow-export-dialog"; import { useToast } from "@/components/toast-provider"; import { useWorkflowSettings } from "@/hooks/domains/settings/use-workflow-settings"; @@ -297,36 +293,6 @@ function useWorkflowActions({ }; } -type WorkflowSectionActionsProps = { - onExport: () => void; - onImport: () => void; - onAdd: () => void; -}; - -function WorkflowSectionActions({ onExport, onImport, onAdd }: WorkflowSectionActionsProps) { - return ( -
Not a recognized GitHub repository link.
+ ) : ( ++ {resolved || "Paste a GitHub link — /tree/… links carry the branch and directory too."} +
+ )} ++ {form.poll_enabled + ? "Checks the repository on this interval (minimum 60s)." + : "Syncs only run when you press Sync now."} +
+{parts.join(" · ")}
; +} + +function WarningsAlert({ warnings }: { warnings: string[] }) { + if (warnings.length === 0) return null; + return ( +{config.last_error || "Sync failed"}
+ )} +