From 2a1f7c2edf6587c4a80f5404c3b342d00f6443ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Thu, 16 Jul 2026 08:31:20 +0100 Subject: [PATCH] feat: sync workflows from a configured github repo (#1721) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: sync workflows from a configured github repo Per-workspace setting pointing at a GitHub repo directory of portable workflow export files. A background poller fetches it on an interval and creates, updates, or deletes the matching workflows; a force-sync endpoint bypasses the content-hash check. Synced workflows carry source/source_path provenance and coexist with manual ones; changes that cannot be applied safely (steps or workflows that still hold tasks, ambiguous step names, unparseable files) surface as warnings in the new settings section. Co-Authored-By: Claude Fable 5 * feat(web): fill workflow sync form from a pasted github link A new Repository link field parses plain repo URLs, SSH remotes, and tree/blob links (which also carry branch and directory) into the structured owner/repo/branch/path fields. Co-Authored-By: Claude Fable 5 * feat(web): simplify workflow sync form to link, branch, and interval The repository link is now the only repo input: owner, repo, and directory resolve from the pasted URL (echoed as a summary under the field), and a stored config renders back into a canonical tree link via buildGitHubRepoUrl. Branch and poll interval stay individually editable. Co-Authored-By: Claude Fable 5 * feat(web): collapse workflow sync into a button, dialog, and status card The Workflows settings page now shows only a GitHub Sync button by default; the link/branch/interval form lives in a dialog that closes on successful save or removal. Once configured, a compact status card (mirroring the GitHub connection-status card) shows the synced repo, branch, directory, last result, warnings, and a Sync now action. Co-Authored-By: Claude Fable 5 * feat(web): move github sync button into the workflows page header The GitHub Sync trigger now stacks under the Workspace settings button on the top right, and the extra separator around the sync area is gone — unconfigured workspaces show no sync chrome at all, configured ones show just the status card above the workflow list. Co-Authored-By: Claude Fable 5 * feat(web): move github sync button next to the workflow actions GitHub Sync now sits in the Workflows section toolbar alongside Export All, Import, and Add Workflow; the status card renders inside the section above the workflow list. The toolbar moved to its own component to keep the page under the file-size limit. Co-Authored-By: Claude Fable 5 * feat(web): separate sync status card from the workflow list Co-Authored-By: Claude Fable 5 * fix(web): align sync status card with the workflow list gutter Co-Authored-By: Claude Fable 5 * fix: repair local edits to synced workflows on periodic sync Periodic syncs previously skipped applying when the repo content hash was unchanged, so local UI edits to a synced workflow were only reverted by a manual Sync now. Every sync now reconciles against the repo; the applier compares each step against its definition and only writes (and broadcasts) actual differences, so a no-drift sync stays silent. Co-Authored-By: Claude Fable 5 * feat: make github-synced workflows read-only Workflows with source=github reject UI mutations: workflow rename, description, agent-profile, and delete return 409 from the task-domain handlers; step create/update/delete/reorder and template application are guarded in the workflow controller. The sync applier writes through the service/repo layer and stays exempt. The frontend disables the card's inputs, save/delete, and the step pipeline editor for synced workflows, with a tooltip pointing at the synced repository. Drift repair on sync remains as the backstop. Co-Authored-By: Claude Fable 5 * feat: optional workflow sync polling and repo-derived branch picker A per-workspace auto-sync toggle (poll_enabled, default on) controls the background polling loop; when off the workspace only syncs via Sync now. The dialog's branch field becomes a select fed by the repository's actual branches (same loader the task-create dialog uses), falling back to free text until branches load. Existing dev databases gain the new column via an idempotent ALTER. Co-Authored-By: Claude Fable 5 * feat: release synced workflows when the sync config is removed Deleting a workspace's workflow sync configuration converts its GitHub-sourced workflows back to manual ownership (source=manual), so they become editable instead of being stranded read-only. Release runs before the config row is deleted so a failure keeps the config for retry. The settings page reloads after removal (and after syncs that changed workflows) so cards unlock without a manual refresh. Co-Authored-By: Claude Fable 5 * feat(web): drop branch selector and compact the auto-sync row The pasted link already carries the branch (main by default), so the dialog no longer needs a branch field — the resolved summary echoes it. The auto-sync toggle is now one compact row with the interval inline and a single short helper line. Co-Authored-By: Claude Fable 5 * fix: address PR review feedback Backend: fail-open read-only guard on workflow lookup errors (fixes MCP step handler CI failures); poller Stop holds its mutex through drain; per-workspace lock serializes syncs against config delete/replace; rollback workflow creation when provenance stamping fails; idiomatic duplicate-column migration via db.IsDuplicateColumnError; branch-name validation and a 30-day interval cap; generic 500s with logged detail; WS read-only rejections use CONFLICT with the real reason; e2e reset aborts when sync-config cleanup fails; test hygiene (t.Cleanup, SetMaxOpenConns). Frontend: malformed percent escapes parse as invalid instead of throwing; save disabled during initial load and for invalid intervals; Remove disabled while saving; stale async config responses discarded; delete-dialog confirms re-check read-only; focusable Synced badge; explicit button types; failed polls labeled 'last attempt'; unique warning list keys; test cleanup via afterEach. Co-Authored-By: Claude Fable 5 * fix: guard workflow sync test fake against concurrent poller access The poller invokes the fake applier from its own goroutine while the synctest test asserts on call counts; -race caught the unguarded reads. Also documents that the per-workspace lock is deliberately held across the GitHub fetch. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Kandev Agent Co-authored-by: Claude Fable 5 --- apps/backend/AGENTS.md | 3 +- apps/backend/internal/backendapp/e2e_reset.go | 9 + apps/backend/internal/backendapp/helpers.go | 6 + apps/backend/internal/backendapp/main.go | 11 + apps/backend/internal/backendapp/services.go | 47 +- apps/backend/internal/backendapp/types.go | 4 + apps/backend/internal/github/client.go | 10 + .../internal/github/controller_test.go | 6 + apps/backend/internal/github/gh_client.go | 70 +++ apps/backend/internal/github/mock_client.go | 136 ++++++ .../internal/github/mock_controller.go | 28 ++ .../internal/github/mock_controller_test.go | 62 +++ apps/backend/internal/github/models.go | 15 + apps/backend/internal/github/noop_client.go | 8 + apps/backend/internal/github/pat_client.go | 40 ++ apps/backend/internal/github/repo_contents.go | 50 ++ .../internal/github/repo_contents_test.go | 442 ++++++++++++++++++ apps/backend/internal/task/dto/dto.go | 12 +- .../task/handlers/workflow_handlers.go | 48 ++ apps/backend/internal/task/models/models.go | 14 + .../task/repository/sqlite/base_migrations.go | 6 + .../task/repository/sqlite/workflow.go | 30 +- .../repository/sqlite/workflow_source_test.go | 139 ++++++ .../internal/task/service/service_events.go | 2 + .../task/service/service_resources.go | 22 + .../workflow/controller/controller.go | 19 + .../internal/workflow/handlers/handlers.go | 12 +- .../internal/workflow/service/readonly.go | 35 ++ .../internal/workflow/service/service.go | 60 +-- .../internal/workflow/service/sync_apply.go | 324 +++++++++++++ .../workflow/service/sync_apply_test.go | 403 ++++++++++++++++ .../internal/workflowsync/goleak_test.go | 15 + .../backend/internal/workflowsync/handlers.go | 128 +++++ apps/backend/internal/workflowsync/models.go | 119 +++++ .../internal/workflowsync/models_test.go | 62 +++ apps/backend/internal/workflowsync/poller.go | 81 ++++ .../internal/workflowsync/poller_test.go | 48 ++ .../backend/internal/workflowsync/provider.go | 20 + apps/backend/internal/workflowsync/service.go | 281 +++++++++++ .../internal/workflowsync/service_test.go | 238 ++++++++++ apps/backend/internal/workflowsync/store.go | 205 ++++++++ .../internal/workflowsync/store_test.go | 139 ++++++ .../workspace/workspace-workflows-client.tsx | 47 +- .../integrations/auth-status-banner.tsx | 5 +- .../settings/workflow-card-actions.test.ts | 116 ++++- .../settings/workflow-card-actions.ts | 24 + .../web/components/settings/workflow-card.tsx | 111 ++++- .../settings/workflow-section-actions.tsx | 57 +++ .../settings/workflow-sync-dialog.tsx | 190 ++++++++ .../settings/workflow-sync-section.tsx | 65 +++ .../settings/workflow-sync-status-banner.tsx | 128 +++++ .../settings/use-workflow-sync.test.ts | 145 ++++++ .../domains/settings/use-workflow-sync.ts | 263 +++++++++++ .../lib/api/domains/workflow-sync-api.test.ts | 112 +++++ apps/web/lib/api/domains/workflow-sync-api.ts | 63 +++ apps/web/lib/api/index.ts | 1 + apps/web/lib/types/http.ts | 8 + apps/web/lib/types/workflow-sync.ts | 59 +++ apps/web/lib/utils/github-repo-url.test.ts | 116 +++++ apps/web/lib/utils/github-repo-url.ts | 64 +++ docs/public/index.md | 1 + docs/public/workflow-import-export.md | 4 + docs/public/workflow-sync.md | 115 +++++ 63 files changed, 4997 insertions(+), 106 deletions(-) create mode 100644 apps/backend/internal/github/repo_contents.go create mode 100644 apps/backend/internal/github/repo_contents_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/workflow_source_test.go create mode 100644 apps/backend/internal/workflow/service/readonly.go create mode 100644 apps/backend/internal/workflow/service/sync_apply.go create mode 100644 apps/backend/internal/workflow/service/sync_apply_test.go create mode 100644 apps/backend/internal/workflowsync/goleak_test.go create mode 100644 apps/backend/internal/workflowsync/handlers.go create mode 100644 apps/backend/internal/workflowsync/models.go create mode 100644 apps/backend/internal/workflowsync/models_test.go create mode 100644 apps/backend/internal/workflowsync/poller.go create mode 100644 apps/backend/internal/workflowsync/poller_test.go create mode 100644 apps/backend/internal/workflowsync/provider.go create mode 100644 apps/backend/internal/workflowsync/service.go create mode 100644 apps/backend/internal/workflowsync/service_test.go create mode 100644 apps/backend/internal/workflowsync/store.go create mode 100644 apps/backend/internal/workflowsync/store_test.go create mode 100644 apps/web/components/settings/workflow-section-actions.tsx create mode 100644 apps/web/components/settings/workflow-sync-dialog.tsx create mode 100644 apps/web/components/settings/workflow-sync-section.tsx create mode 100644 apps/web/components/settings/workflow-sync-status-banner.tsx create mode 100644 apps/web/hooks/domains/settings/use-workflow-sync.test.ts create mode 100644 apps/web/hooks/domains/settings/use-workflow-sync.ts create mode 100644 apps/web/lib/api/domains/workflow-sync-api.test.ts create mode 100644 apps/web/lib/api/domains/workflow-sync-api.ts create mode 100644 apps/web/lib/types/workflow-sync.ts create mode 100644 apps/web/lib/utils/github-repo-url.test.ts create mode 100644 apps/web/lib/utils/github-repo-url.ts create mode 100644 docs/public/workflow-sync.md 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 ( -
- - - -
- ); -} - type WorkflowListProps = { workflowItems: Workflow[]; workspaceId: string; @@ -484,6 +450,7 @@ export function WorkspaceWorkflowsClient({ workflowTemplates, }: WorkspaceWorkflowsClientProps) { const page = useWorkspaceWorkflowsPage(workspace, workflows, workflowTemplates); + const [syncDialogOpen, setSyncDialogOpen] = useState(false); if (!workspace) return page.router.push("/settings/workspace")} />; @@ -509,9 +476,15 @@ export function WorkspaceWorkflowsClient({ onExport={page.handleExportAll} onImport={() => page.setIsImportDialogOpen(true)} onAdd={page.handleOpenAddWorkflowDialog} + onGitHubSync={() => setSyncDialogOpen(true)} /> } > + { const id = setInterval(() => setTick((n) => n + 1), intervalMs); diff --git a/apps/web/components/settings/workflow-card-actions.test.ts b/apps/web/components/settings/workflow-card-actions.test.ts index a664dea032..41508559a3 100644 --- a/apps/web/components/settings/workflow-card-actions.test.ts +++ b/apps/web/components/settings/workflow-card-actions.test.ts @@ -3,11 +3,18 @@ import { describe, expect, it, vi } from "vitest"; import { createWorkflowAction, createWorkflowStepAction, + deleteWorkflowStepAction, + getWorkflowTaskCount, listWorkflowStepsAction, + reorderWorkflowStepsAction, updateWorkflowStepAction, } from "@/app/actions/workspaces"; import type { Workflow, WorkflowStep } from "@/lib/types/http"; -import { useWorkflowSaveActions, useWorkflowStepActions } from "./workflow-card-actions"; +import { + useWorkflowDeleteHandlers, + useWorkflowSaveActions, + useWorkflowStepActions, +} from "./workflow-card-actions"; vi.mock("@/app/actions/workspaces", () => ({ createWorkflowAction: vi.fn(), @@ -68,6 +75,32 @@ function renderNewWorkflowStepActions(initialSteps: WorkflowStep[]) { return { ...view, getSteps: () => steps }; } +function renderReadOnlyWorkflowStepActions(initialSteps: WorkflowStep[]) { + let steps = initialSteps; + const setWorkflowSteps = vi.fn( + (updater: ((prev: WorkflowStep[]) => WorkflowStep[]) | WorkflowStep[]) => { + steps = typeof updater === "function" ? updater(steps) : updater; + }, + ); + const refreshWorkflowSteps = vi.fn(); + const view = renderHook(() => + useWorkflowStepActions({ + workflow, + isNewWorkflow: false, + readOnly: true, + workflowSteps: steps, + setWorkflowSteps, + refreshWorkflowSteps, + setStepToDelete: vi.fn(), + setStepTaskCount: vi.fn(), + setTargetStepForMigration: vi.fn(), + setStepDeleteOpen: vi.fn(), + toast: vi.fn(), + }), + ); + return { ...view, getSteps: () => steps, refreshWorkflowSteps, setWorkflowSteps }; +} + describe("useWorkflowStepActions", () => { it("keeps one start step while editing a new workflow locally", async () => { const { result, getSteps } = renderNewWorkflowStepActions([ @@ -85,6 +118,66 @@ describe("useWorkflowStepActions", () => { .map((s) => s.id), ).toEqual(["step-2"]); }); + + it("refuses to add, update, remove, or reorder steps when readOnly", async () => { + const initialSteps = [step("step-1", "Todo", 0, true), step("step-2", "Plan", 1, false)]; + const { result, getSteps, refreshWorkflowSteps } = + renderReadOnlyWorkflowStepActions(initialSteps); + + await act(async () => { + await result.current.handleAddWorkflowStep(); + }); + await act(async () => { + await result.current.handleUpdateWorkflowStep("step-2", { name: "Renamed" }); + }); + await act(async () => { + await result.current.handleRemoveWorkflowStep("step-1"); + }); + await act(async () => { + await result.current.handleReorderWorkflowSteps([initialSteps[1], initialSteps[0]]); + }); + + expect(getSteps()).toEqual(initialSteps); + expect(createWorkflowStepAction).not.toHaveBeenCalled(); + expect(updateWorkflowStepAction).not.toHaveBeenCalled(); + expect(deleteWorkflowStepAction).not.toHaveBeenCalled(); + expect(reorderWorkflowStepsAction).not.toHaveBeenCalled(); + expect(refreshWorkflowSteps).not.toHaveBeenCalled(); + }); +}); + +describe("useWorkflowDeleteHandlers", () => { + it("refuses to open the delete-workflow dialog when readOnly", async () => { + const wfDel = { + setDeleteOpen: vi.fn(), + setWorkflowTaskCount: vi.fn(), + setWorkflowDeleteLoading: vi.fn(), + setTargetWorkflowId: vi.fn(), + setTargetWorkflowSteps: vi.fn(), + setTargetStepId: vi.fn(), + targetWorkflowId: "", + targetStepId: "", + setMigrateLoading: vi.fn(), + }; + const { result } = renderHook(() => + useWorkflowDeleteHandlers({ + workflow, + isNewWorkflow: false, + readOnly: true, + otherWorkflows: [], + wfDel, + deleteWorkflowRun: vi.fn(), + toast: vi.fn(), + }), + ); + + await act(async () => { + await result.current.handleDeleteWorkflowClick(); + }); + + expect(getWorkflowTaskCount).not.toHaveBeenCalled(); + expect(wfDel.setDeleteOpen).not.toHaveBeenCalled(); + }); }); describe("useWorkflowSaveActions", () => { @@ -154,4 +247,25 @@ describe("useWorkflowSaveActions", () => { pull_from_step_id: backendAddedId, }); }); + + it("refuses to save changes to an existing workflow when readOnly", async () => { + const onSaveWorkflow = vi.fn().mockResolvedValue(undefined); + const { result } = renderHook(() => + useWorkflowSaveActions({ + workflow, + isNewWorkflow: false, + readOnly: true, + workflowSteps: [], + templateStepCount: 0, + onSaveWorkflow, + toast: vi.fn(), + }), + ); + + await act(async () => { + await result.current.handleSaveWorkflow(); + }); + + expect(onSaveWorkflow).not.toHaveBeenCalled(); + }); }); diff --git a/apps/web/components/settings/workflow-card-actions.ts b/apps/web/components/settings/workflow-card-actions.ts index 901788d7de..9dede74e4c 100644 --- a/apps/web/components/settings/workflow-card-actions.ts +++ b/apps/web/components/settings/workflow-card-actions.ts @@ -23,6 +23,13 @@ const FALLBACK_ERROR_MESSAGE = "Request failed"; type WorkflowStepActionsParams = { workflow: Workflow; isNewWorkflow: boolean; + /** + * Workflows synced from GitHub (`workflow.source === "github"`) are + * read-only in the UI; the backend also rejects step mutations with a 409. + * Gating here is defense-in-depth in case a disabled control is somehow + * still triggered. + */ + readOnly?: boolean; workflowSteps: WorkflowStep[]; setWorkflowSteps: (updater: ((prev: WorkflowStep[]) => WorkflowStep[]) | WorkflowStep[]) => void; refreshWorkflowSteps: () => Promise; @@ -133,6 +140,7 @@ function applyWorkflowStepUpdates( export function useWorkflowStepActions({ workflow, isNewWorkflow, + readOnly = false, workflowSteps, setWorkflowSteps, refreshWorkflowSteps, @@ -143,6 +151,7 @@ export function useWorkflowStepActions({ toast, }: WorkflowStepActionsParams) { const handleUpdateWorkflowStep = async (stepId: string, updates: Partial) => { + if (readOnly) return; if (isNewWorkflow) { setWorkflowSteps((prev) => applyWorkflowStepUpdates(prev, stepId, updates)); return; @@ -160,6 +169,7 @@ export function useWorkflowStepActions({ }; const handleAddWorkflowStep = async () => { + if (readOnly) return; if (isNewWorkflow) { addLocalStep(workflow, setWorkflowSteps); return; @@ -168,6 +178,7 @@ export function useWorkflowStepActions({ }; const handleRemoveWorkflowStep = async (stepId: string) => { + if (readOnly) return; if (isNewWorkflow) { setWorkflowSteps((prev) => prev.filter((s) => s.id !== stepId).map((s, i) => ({ ...s, position: i })), @@ -187,6 +198,7 @@ export function useWorkflowStepActions({ }; const handleReorderWorkflowSteps = async (reorderedSteps: WorkflowStep[]) => { + if (readOnly) return; setWorkflowSteps(reorderedSteps); if (isNewWorkflow) return; try { @@ -215,6 +227,8 @@ export function useWorkflowStepActions({ type WorkflowDeleteHandlersParams = { workflow: Workflow; isNewWorkflow: boolean; + /** See `WorkflowStepActionsParams.readOnly` — defense-in-depth for synced workflows. */ + readOnly?: boolean; otherWorkflows: Workflow[]; wfDel: { setDeleteOpen: (v: boolean) => void; @@ -234,6 +248,7 @@ type WorkflowDeleteHandlersParams = { export function useWorkflowDeleteHandlers({ workflow, isNewWorkflow, + readOnly = false, otherWorkflows, wfDel, deleteWorkflowRun, @@ -263,6 +278,7 @@ export function useWorkflowDeleteHandlers({ }, [wfDel.targetWorkflowId]); // eslint-disable-line react-hooks/exhaustive-deps const handleDeleteWorkflowClick = async () => { + if (readOnly) return; if (isNewWorkflow) { wfDel.setWorkflowTaskCount(0); wfDel.setDeleteOpen(true); @@ -287,6 +303,9 @@ export function useWorkflowDeleteHandlers({ }; const handleDeleteWorkflow = async () => { + // A background sync can flip the workflow read-only while the delete + // dialog is already open; re-check at confirm time. + if (readOnly) return; try { await deleteWorkflowRun(); wfDel.setDeleteOpen(false); @@ -300,6 +319,7 @@ export function useWorkflowDeleteHandlers({ }; const handleMigrateAndDeleteWorkflow = async () => { + if (readOnly) return; if (!wfDel.targetWorkflowId || !wfDel.targetStepId) return; wfDel.setMigrateLoading(true); try { @@ -505,6 +525,8 @@ async function reconcileTemplateSteps( type WorkflowSaveActionsParams = { workflow: Workflow; isNewWorkflow: boolean; + /** See `WorkflowStepActionsParams.readOnly` — defense-in-depth for synced workflows. */ + readOnly?: boolean; workflowSteps: WorkflowStep[]; templateStepCount: number; onSaveWorkflow: () => Promise; @@ -515,6 +537,7 @@ type WorkflowSaveActionsParams = { export function useWorkflowSaveActions({ workflow, isNewWorkflow, + readOnly = false, workflowSteps, templateStepCount, onSaveWorkflow, @@ -545,6 +568,7 @@ export function useWorkflowSaveActions({ const activeSaveRequest = isNewWorkflow ? saveNewWorkflowRequest : saveWorkflowRequest; const handleSaveWorkflow = async () => { + if (readOnly) return; try { if (isNewWorkflow) await saveNewWorkflowRequest.run(); else await saveWorkflowRequest.run(); diff --git a/apps/web/components/settings/workflow-card.tsx b/apps/web/components/settings/workflow-card.tsx index 06546f0cf8..77217b2ed4 100644 --- a/apps/web/components/settings/workflow-card.tsx +++ b/apps/web/components/settings/workflow-card.tsx @@ -3,10 +3,12 @@ import { useState, useEffect } from "react"; import { IconDownload, IconTrash } from "@tabler/icons-react"; import { Card, CardContent } from "@kandev/ui/card"; +import { Badge } from "@kandev/ui/badge"; import { Button } from "@kandev/ui/button"; import { Input } from "@kandev/ui/input"; import { Label } from "@kandev/ui/label"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import type { Workflow, WorkflowStep } from "@/lib/types/http"; import { useHealthyAgentProfiles } from "@/hooks/domains/settings/use-healthy-agent-profiles"; import { useRequest } from "@/lib/http/use-request"; @@ -170,8 +172,49 @@ type WorkflowCardActionsProps = { toast: ReturnType["toast"]; onDeleteClick: () => Promise; deleteDisabled: boolean; + readOnly: boolean; }; +const SYNCED_READ_ONLY_REASON = + "Managed by workflow sync — edit or remove it in the synced repository"; + +function DeleteWorkflowButton({ + onDeleteClick, + deleteDisabled, + readOnly, +}: { + onDeleteClick: () => Promise; + deleteDisabled: boolean; + readOnly: boolean; +}) { + const button = ( + + ); + + if (!readOnly) return button; + + return ( + + + + {button} + + + {SYNCED_READ_ONLY_REASON} + + ); +} + function WorkflowCardActions({ isNewWorkflow, workflowId, @@ -180,6 +223,7 @@ function WorkflowCardActions({ toast, onDeleteClick, deleteDisabled, + readOnly, }: WorkflowCardActionsProps) { return (
@@ -194,17 +238,11 @@ function WorkflowCardActions({ Export )} - +
); } @@ -246,8 +284,34 @@ type WorkflowCardBodyProps = { handleRemoveWorkflowStep: (id: string) => Promise; handleReorderWorkflowSteps: (steps: WorkflowStep[]) => Promise; }; + readOnly: boolean; }; +// SyncedBadge marks workflows managed by workflow sync (workflow.source === +// "github"). Hovering / focusing shows which repo-relative file it came from +// so users don't accidentally edit a workflow that the next poll will +// overwrite. +function SyncedBadge({ sourcePath }: { sourcePath?: string }) { + return ( + + + + Synced + + + + Read-only — managed by workflow sync from {sourcePath || "a configured repository"}. Edit or + remove it in the synced repository. + + + ); +} + function WorkflowCardBody({ workflow, isWorkflowDirty, @@ -257,6 +321,7 @@ function WorkflowCardBody({ workflowLoading, workflowSteps, stepActions, + readOnly, }: WorkflowCardBodyProps) { const healthyProfiles = useHealthyAgentProfiles(workflow.agent_profile_id); @@ -267,10 +332,17 @@ function WorkflowCardBody({ onUpdateWorkflow({ name: e.target.value })} + disabled={readOnly} />
@@ -283,6 +355,7 @@ function WorkflowCardBody({ onValueChange={(value) => onUpdateWorkflow({ agent_profile_id: value === "none" ? "" : value }) } + disabled={readOnly} >
@@ -320,6 +394,7 @@ function WorkflowCardBody({ onAddStep={stepActions.handleAddWorkflowStep} onRemoveStep={stepActions.handleRemoveWorkflowStep} onReorderSteps={stepActions.handleReorderWorkflowSteps} + readOnly={readOnly} /> )}
@@ -386,12 +461,17 @@ function useWorkflowCardState(props: WorkflowCardProps) { const wfDel = useWorkflowDeleteState(); const stepDel = useStepDeleteState(); const isNewWorkflow = workflow.id.startsWith("temp-"); + // Workflows synced from a configured GitHub repo are read-only: the + // backend rejects definition mutations with a 409, so the UI disables the + // matching affordances (name/agent-profile/steps/delete) up front. + const readOnly = workflow.source === "github"; const deleteWorkflowRequest = useRequest(onDeleteWorkflow); const { workflowSteps, setWorkflowSteps, workflowLoading, refreshWorkflowSteps } = useWorkflowSteps(workflow.id, initialWorkflowSteps, isNewWorkflow, toast); const stepActions = useWorkflowStepActions({ workflow, isNewWorkflow, + readOnly, workflowSteps, setWorkflowSteps, refreshWorkflowSteps, @@ -404,6 +484,7 @@ function useWorkflowCardState(props: WorkflowCardProps) { const { activeSaveRequest, handleSaveWorkflow } = useWorkflowSaveActions({ workflow, isNewWorkflow, + readOnly, workflowSteps, templateStepCount, onSaveWorkflow, @@ -413,6 +494,7 @@ function useWorkflowCardState(props: WorkflowCardProps) { const wfDeleteHandlers = useWorkflowDeleteHandlers({ workflow, isNewWorkflow, + readOnly, otherWorkflows, wfDel, deleteWorkflowRun: deleteWorkflowRequest.run, @@ -436,6 +518,7 @@ function useWorkflowCardState(props: WorkflowCardProps) { wfDel, stepDel, isNewWorkflow, + readOnly, deleteWorkflowRequest, workflowSteps, workflowLoading, @@ -468,6 +551,7 @@ export function WorkflowCard(props: WorkflowCardProps) { workflowLoading={s.workflowLoading} workflowSteps={s.workflowSteps} stepActions={s.stepActions} + readOnly={s.readOnly} /> diff --git a/apps/web/components/settings/workflow-section-actions.tsx b/apps/web/components/settings/workflow-section-actions.tsx new file mode 100644 index 0000000000..cbc749a4df --- /dev/null +++ b/apps/web/components/settings/workflow-section-actions.tsx @@ -0,0 +1,57 @@ +"use client"; + +import { IconDownload, IconPlus, IconUpload } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { WorkflowSyncButton } from "@/components/settings/workflow-sync-section"; + +type WorkflowSectionActionsProps = { + onExport: () => void; + onImport: () => void; + onAdd: () => void; + onGitHubSync: () => void; +}; + +// WorkflowSectionActions is the toolbar of the Workflows settings section: +// GitHub Sync, Export All, Import, and Add Workflow. +export function WorkflowSectionActions({ + onExport, + onImport, + onAdd, + onGitHubSync, +}: WorkflowSectionActionsProps) { + return ( +
+ + + + +
+ ); +} diff --git a/apps/web/components/settings/workflow-sync-dialog.tsx b/apps/web/components/settings/workflow-sync-dialog.tsx new file mode 100644 index 0000000000..17cff5957d --- /dev/null +++ b/apps/web/components/settings/workflow-sync-dialog.tsx @@ -0,0 +1,190 @@ +"use client"; + +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@kandev/ui/dialog"; +import { Button } from "@kandev/ui/button"; +import { Input } from "@kandev/ui/input"; +import { Label } from "@kandev/ui/label"; +import { Switch } from "@kandev/ui/switch"; +import type { + WorkflowSyncController, + WorkflowSyncFormState, +} from "@/hooks/domains/settings/use-workflow-sync"; + +const HELP_TEXT = + "The directory should contain workflow export files (.yml/.yaml/.json) in the kandev_workflow format — the same format produced by workflow export."; + +type RepoUrlFieldProps = { + url: string; + invalid: boolean; + resolved: string; + onChange: (value: string) => void; +}; + +// RepoUrlField is the primary input: a full GitHub link (optionally with +// /tree//) that resolves into the stored owner, repo, +// branch, and directory. The resolved target is echoed underneath so the +// structured fields stay visible to the user. +function RepoUrlField({ url, invalid, resolved, onChange }: RepoUrlFieldProps) { + return ( +
+ + onChange(e.target.value)} + aria-invalid={invalid} + /> + {invalid ? ( +

Not a recognized GitHub repository link.

+ ) : ( +

+ {resolved || "Paste a GitHub link — /tree/… links carry the branch and directory too."} +

+ )} +
+ ); +} + +type FieldsProps = { + form: WorkflowSyncFormState; + update: (key: K, value: WorkflowSyncFormState[K]) => void; +}; + +// PollFields is a single compact row: the auto-sync switch and, when on, the +// interval right beside it. The branch needs no field of its own — it comes +// from the pasted link (or defaults to main) and is echoed in the resolved +// summary under the link input. +function PollFields({ form, update }: FieldsProps) { + return ( +
+
+ update("poll_enabled", checked)} + className="cursor-pointer" + /> + + {form.poll_enabled && ( +
+ + update("interval_seconds", Number(e.target.value) || 0)} + /> + seconds +
+ )} +
+

+ {form.poll_enabled + ? "Checks the repository on this interval (minimum 60s)." + : "Syncs only run when you press Sync now."} +

+
+ ); +} + +type WorkflowSyncDialogProps = { + open: boolean; + onOpenChange: (open: boolean) => void; + sync: WorkflowSyncController; +}; + +// WorkflowSyncDialog holds the GitHub Sync configuration form. It closes +// itself after a successful save or removal; failures keep it open with the +// error surfaced via toast. +export function WorkflowSyncDialog({ open, onOpenChange, sync }: WorkflowSyncDialogProps) { + const hasConfig = !!sync.config; + const intervalInvalid = + sync.form.poll_enabled && + (!Number.isInteger(sync.form.interval_seconds) || sync.form.interval_seconds < 60); + const disableSave = + sync.saving || + sync.loading || + sync.urlInvalid || + intervalInvalid || + !sync.form.repo_owner.trim() || + !sync.form.repo_name.trim(); + const resolved = sync.form.repo_owner + ? `Syncing ${sync.form.repo_owner}/${sync.form.repo_name} @ ${sync.form.branch || "main"} — directory ${sync.form.path || "(repository root)"}.` + : ""; + + const handleSave = async () => { + if (await sync.handleSave()) onOpenChange(false); + }; + const handleRemove = async () => { + if (await sync.handleDelete()) onOpenChange(false); + }; + + return ( + + + + GitHub Sync + + Automatically sync workflow definitions from a GitHub repository into this workspace. + + +
+ + +

{HELP_TEXT}

+
+ + {hasConfig && ( + + )} + + + + + ); +} + +function saveLabel(saving: boolean, hasConfig: boolean): string { + if (saving) return "Saving..."; + return hasConfig ? "Update" : "Save"; +} diff --git a/apps/web/components/settings/workflow-sync-section.tsx b/apps/web/components/settings/workflow-sync-section.tsx new file mode 100644 index 0000000000..2b26d84969 --- /dev/null +++ b/apps/web/components/settings/workflow-sync-section.tsx @@ -0,0 +1,65 @@ +"use client"; + +import { IconBrandGithub } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { Separator } from "@kandev/ui/separator"; +import { WorkflowSyncDialog } from "@/components/settings/workflow-sync-dialog"; +import { WorkflowSyncStatusCard } from "@/components/settings/workflow-sync-status-banner"; +import { useWorkflowSync } from "@/hooks/domains/settings/use-workflow-sync"; + +// WorkflowSyncButton is the GitHub Sync entry point, rendered alongside the +// other workflow actions (Export / Import / Add). The dialog open state lives +// with the caller so button and section can sit in different parts of the +// layout. +export function WorkflowSyncButton({ onClick }: { onClick: () => void }) { + return ( + + ); +} + +type WorkflowSyncSectionProps = { + workspaceId: string; + dialogOpen: boolean; + onDialogOpenChange: (open: boolean) => void; +}; + +// WorkflowSyncSection renders the GitHub-sync state inside the Workflows +// settings section: the configuration dialog (opened via WorkflowSyncButton +// in the section's action row) and — once a sync is configured — a compact +// status card above the workflow list showing what is syncing and how the +// last attempt went. Unconfigured workspaces render nothing visible. +export function WorkflowSyncSection({ + workspaceId, + dialogOpen, + onDialogOpenChange, +}: WorkflowSyncSectionProps) { + const sync = useWorkflowSync(workspaceId); + + return ( + <> + {sync.config && ( + // pl-8 matches the workflow list's drag-handle gutter so the card's + // left edge lines up with the workflow cards below. +
+ + +
+ )} + + + ); +} diff --git a/apps/web/components/settings/workflow-sync-status-banner.tsx b/apps/web/components/settings/workflow-sync-status-banner.tsx new file mode 100644 index 0000000000..e4fc255579 --- /dev/null +++ b/apps/web/components/settings/workflow-sync-status-banner.tsx @@ -0,0 +1,128 @@ +"use client"; + +import { formatDistanceToNow } from "date-fns"; +import { + IconAlertTriangle, + IconCheck, + IconClock, + IconLoader2, + IconRefresh, +} from "@tabler/icons-react"; +import { Alert, AlertDescription } from "@kandev/ui/alert"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { useTick } from "@/components/integrations/auth-status-banner"; +import type { WorkflowSyncConfig } from "@/lib/types/workflow-sync"; + +type SyncState = "waiting" | "ok" | "failed"; + +function syncState(config: WorkflowSyncConfig): SyncState { + if (!config.last_synced_at) return "waiting"; + return config.last_ok ? "ok" : "failed"; +} + +function StateIcon({ state }: { state: SyncState }) { + if (state === "ok") return ; + if (state === "failed") return ; + return ; +} + +function lastSyncedLabel(config: WorkflowSyncConfig): string { + if (config.last_synced_at) { + const when = formatDistanceToNow(new Date(config.last_synced_at), { addSuffix: true }); + return config.last_ok ? `last synced ${when}` : `last attempt ${when}`; + } + return config.poll_enabled ? "waiting for first sync…" : "not synced yet — use Sync now"; +} + +function MetadataLine({ config }: { config: WorkflowSyncConfig }) { + useTick(30_000); + const parts = [ + `Directory ${config.path || "(repository root)"}`, + config.poll_enabled ? `every ${config.interval_seconds}s` : "auto-sync off", + lastSyncedLabel(config), + ]; + return

{parts.join(" · ")}

; +} + +function WarningsAlert({ warnings }: { warnings: string[] }) { + if (warnings.length === 0) return null; + return ( + + + +
    + {warnings.map((warning, index) => ( + // Warnings are free-form backend sentences with no stable id; + // include the index so repeated sentences keep unique keys. +
  • {warning}
    • + ))} +
+ + + ); +} + +type WorkflowSyncStatusCardProps = { + config: WorkflowSyncConfig; + syncing: boolean; + onSyncNow: () => void; +}; + +// WorkflowSyncStatusCard is the compact always-visible summary of an active +// GitHub sync (mirrors the GitHub integration's connection-status card): +// headline with state icon, repo and branch, a muted metadata line, the last +// error when failing, and any warnings from the most recent attempt — +// warnings can be present even when last_ok is true (e.g. one file failed to +// parse but the rest synced). +export function WorkflowSyncStatusCard({ + config, + syncing, + onSyncNow, +}: WorkflowSyncStatusCardProps) { + const state = syncState(config); + return ( +
+
+
+ + + Syncing from{" "} + + {config.repo_owner}/{config.repo_name} + + + {config.branch} +
+ +
+ + {state === "failed" && ( +

{config.last_error || "Sync failed"}

+ )} + +
+ ); +} diff --git a/apps/web/hooks/domains/settings/use-workflow-sync.test.ts b/apps/web/hooks/domains/settings/use-workflow-sync.test.ts new file mode 100644 index 0000000000..63404a106f --- /dev/null +++ b/apps/web/hooks/domains/settings/use-workflow-sync.test.ts @@ -0,0 +1,145 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, waitFor, act } from "@testing-library/react"; +import type { WorkflowSyncConfig } from "@/lib/types/workflow-sync"; + +const mockToast = vi.fn(); +vi.mock("@/components/toast-provider", () => ({ + useToast: () => ({ toast: mockToast }), +})); + +const getWorkflowSyncConfigMock = vi.fn<(opts?: unknown) => Promise>(); +const setWorkflowSyncConfigMock = vi.fn<(payload: unknown, opts?: unknown) => Promise>(); +const deleteWorkflowSyncConfigMock = vi.fn<(opts?: unknown) => Promise>(); +const forceWorkflowSyncMock = vi.fn<(opts?: unknown) => Promise>(); + +vi.mock("@/lib/api/domains/workflow-sync-api", () => ({ + getWorkflowSyncConfig: (opts?: unknown) => getWorkflowSyncConfigMock(opts), + setWorkflowSyncConfig: (payload: unknown, opts?: unknown) => + setWorkflowSyncConfigMock(payload, opts), + deleteWorkflowSyncConfig: (opts?: unknown) => deleteWorkflowSyncConfigMock(opts), + forceWorkflowSync: (opts?: unknown) => forceWorkflowSyncMock(opts), +})); + +import { useWorkflowSync } from "./use-workflow-sync"; + +function makeConfig(overrides: Partial = {}): WorkflowSyncConfig { + return { + workspace_id: "ws-1", + repo_owner: "kdlbs", + repo_name: "kandev", + branch: "main", + path: ".kandev/workflows", + interval_seconds: 300, + poll_enabled: true, + last_ok: false, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + ...overrides, + }; +} + +describe("useWorkflowSync", () => { + beforeEach(() => { + vi.clearAllMocks(); + getWorkflowSyncConfigMock.mockResolvedValue(null); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("loads config and pre-fills the form on mount", async () => { + getWorkflowSyncConfigMock.mockResolvedValue(makeConfig({ repo_owner: "acme", branch: "dev" })); + const { result } = renderHook(() => useWorkflowSync("ws-1")); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.config?.repo_owner).toBe("acme"); + expect(result.current.form.repo_owner).toBe("acme"); + expect(result.current.form.branch).toBe("dev"); + }); + + it("falls back to defaults when unconfigured (204/null)", async () => { + getWorkflowSyncConfigMock.mockResolvedValue(null); + const { result } = renderHook(() => useWorkflowSync("ws-1")); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.config).toBeNull(); + expect(result.current.form.branch).toBe("main"); + expect(result.current.form.path).toBe(".kandev/workflows"); + }); + + it("handleSave posts the form and updates config from the response", async () => { + getWorkflowSyncConfigMock.mockResolvedValue(null); + const saved = makeConfig({ repo_owner: "kdlbs", repo_name: "kandev" }); + setWorkflowSyncConfigMock.mockResolvedValue(saved); + const { result } = renderHook(() => useWorkflowSync("ws-1")); + await waitFor(() => expect(result.current.loading).toBe(false)); + + act(() => result.current.update("repo_owner", "kdlbs")); + act(() => result.current.update("repo_name", "kandev")); + await act(async () => { + await result.current.handleSave(); + }); + + expect(setWorkflowSyncConfigMock).toHaveBeenCalledWith( + expect.objectContaining({ repo_owner: "kdlbs", repo_name: "kandev" }), + { workspaceId: "ws-1" }, + ); + expect(result.current.config?.repo_owner).toBe("kdlbs"); + }); + + it("handleSyncNow updates config from the force-sync response, including failures", async () => { + getWorkflowSyncConfigMock.mockResolvedValue(makeConfig({ last_ok: true })); + forceWorkflowSyncMock.mockResolvedValue({ + config: makeConfig({ last_ok: false, last_error: "clone failed" }), + error: "clone failed", + }); + const { result } = renderHook(() => useWorkflowSync("ws-1")); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + await result.current.handleSyncNow(); + }); + + expect(forceWorkflowSyncMock).toHaveBeenCalledWith({ workspaceId: "ws-1" }); + expect(result.current.config?.last_ok).toBe(false); + expect(result.current.config?.last_error).toBe("clone failed"); + expect(result.current.syncing).toBe(false); + }); + + it("handleDelete clears config and resets the form when confirmed", async () => { + getWorkflowSyncConfigMock.mockResolvedValue(makeConfig()); + deleteWorkflowSyncConfigMock.mockResolvedValue({ deleted: true }); + vi.stubGlobal( + "confirm", + vi.fn(() => true), + ); + const { result } = renderHook(() => useWorkflowSync("ws-1")); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + await result.current.handleDelete(); + }); + + expect(deleteWorkflowSyncConfigMock).toHaveBeenCalledWith({ workspaceId: "ws-1" }); + expect(result.current.config).toBeNull(); + expect(result.current.form.repo_owner).toBe(""); + }); + + it("polls getWorkflowSyncConfig on the background refresh interval", async () => { + vi.useFakeTimers(); + getWorkflowSyncConfigMock.mockResolvedValue(makeConfig({ last_ok: true })); + const { result } = renderHook(() => useWorkflowSync("ws-1")); + await vi.waitFor(() => expect(result.current.loading).toBe(false)); + expect(getWorkflowSyncConfigMock).toHaveBeenCalledTimes(1); + + getWorkflowSyncConfigMock.mockResolvedValue(makeConfig({ last_ok: false, last_error: "boom" })); + await act(async () => { + await vi.advanceTimersByTimeAsync(90_000); + }); + expect(getWorkflowSyncConfigMock.mock.calls.length).toBeGreaterThanOrEqual(2); + expect(result.current.config?.last_error).toBe("boom"); + }); +}); diff --git a/apps/web/hooks/domains/settings/use-workflow-sync.ts b/apps/web/hooks/domains/settings/use-workflow-sync.ts new file mode 100644 index 0000000000..5e5f50c7bd --- /dev/null +++ b/apps/web/hooks/domains/settings/use-workflow-sync.ts @@ -0,0 +1,263 @@ +"use client"; + +import { useCallback, useEffect, useState } from "react"; +import { useToast } from "@/components/toast-provider"; +import { useRouter } from "@/lib/routing/client-router"; +import { INTEGRATION_STATUS_REFRESH_MS } from "@/hooks/domains/integrations/use-integration-availability"; +import { + getWorkflowSyncConfig, + setWorkflowSyncConfig, + deleteWorkflowSyncConfig, + forceWorkflowSync, +} from "@/lib/api/domains/workflow-sync-api"; +import type { WorkflowSyncConfig, WorkflowSyncSetConfigRequest } from "@/lib/types/workflow-sync"; +import { buildGitHubRepoUrl, parseGitHubRepoUrl } from "@/lib/utils/github-repo-url"; + +export type WorkflowSyncFormState = { + repo_owner: string; + repo_name: string; + branch: string; + path: string; + interval_seconds: number; + poll_enabled: boolean; +}; + +const DEFAULT_FORM: WorkflowSyncFormState = { + repo_owner: "", + repo_name: "", + branch: "main", + path: ".kandev/workflows", + interval_seconds: 300, + poll_enabled: true, +}; + +function configToForm(cfg: WorkflowSyncConfig | null): WorkflowSyncFormState { + if (!cfg) return DEFAULT_FORM; + return { + repo_owner: cfg.repo_owner, + repo_name: cfg.repo_name, + branch: cfg.branch, + path: cfg.path, + interval_seconds: cfg.interval_seconds, + poll_enabled: cfg.poll_enabled, + }; +} + +// Background refresh so the status banner picks up new poller results +// (last_ok / last_error / last_warnings) without requiring a page reload. We +// re-fetch the config rather than the loud full `load()` to avoid flashing +// the form while the user is editing it. +function useWorkflowSyncConfigRefresh( + workspaceId: string, + setConfig: (cfg: WorkflowSyncConfig | null) => void, +) { + useEffect(() => { + let cancelled = false; + const id = setInterval(() => { + getWorkflowSyncConfig({ workspaceId }) + .then((cfg) => { + if (!cancelled) setConfig(cfg); + }) + .catch(() => { + /* transient failures are fine — next tick retries */ + }); + }, INTEGRATION_STATUS_REFRESH_MS); + return () => { + cancelled = true; + clearInterval(id); + }; + }, [workspaceId, setConfig]); +} + +// useWorkflowSyncForm owns the editable form state. The repository link is +// the primary input: owner, repo, and directory only change through it (or a +// loaded config), while branch and interval remain individually editable via +// `update`. Branch and directory are only overwritten when the link actually +// carried them (/tree/... or /blob/... forms), so a bare repo URL keeps the +// current values. +function useWorkflowSyncForm() { + const [form, setForm] = useState(DEFAULT_FORM); + const [url, setUrl] = useState(""); + + const update = useCallback( + (key: K, value: WorkflowSyncFormState[K]) => + setForm((prev) => ({ ...prev, [key]: value })), + [], + ); + + const setUrlInput = useCallback((value: string) => { + setUrl(value); + const parsed = parseGitHubRepoUrl(value); + if (!parsed) return; + setForm((prev) => ({ + ...prev, + repo_owner: parsed.owner, + repo_name: parsed.repo, + branch: parsed.branch ?? prev.branch, + path: parsed.path ?? prev.path, + })); + }, []); + + // reset re-derives both the structured form and the displayed link from a + // loaded/saved config (or clears them when the config was removed). + const reset = useCallback((cfg: WorkflowSyncConfig | null) => { + setForm(configToForm(cfg)); + setUrl( + cfg + ? buildGitHubRepoUrl({ + owner: cfg.repo_owner, + repo: cfg.repo_name, + branch: cfg.branch, + path: cfg.path, + }) + : "", + ); + }, []); + + const urlInvalid = !!url.trim() && !parseGitHubRepoUrl(url); + return { form, url, urlInvalid, update, setUrlInput, reset }; +} + +type SyncToast = { description: string; variant?: "success" | "error" | "default" }; + +function syncOutcomeToast(error: string | undefined, warnings: string[] | undefined): SyncToast { + if (error) return { description: `Sync failed: ${error}`, variant: "error" }; + if (warnings?.length) return { description: "Sync completed with warnings", variant: "default" }; + return { description: "Workflow sync completed", variant: "success" }; +} + +type InitialLoadDeps = { + setConfig: (cfg: WorkflowSyncConfig | null) => void; + reset: (cfg: WorkflowSyncConfig | null) => void; + setLoading: (loading: boolean) => void; + toast: ReturnType["toast"]; +}; + +// useWorkflowSyncInitialLoad fetches the stored config on mount / workspace +// change. It guards against stale responses: if the hook re-renders for a +// different workspace while a fetch is in flight, the old workspace's config +// must not populate the new one's form. +function useWorkflowSyncInitialLoad( + workspaceId: string, + { setConfig, reset, setLoading, toast }: InitialLoadDeps, +) { + useEffect(() => { + let cancelled = false; + setLoading(true); + getWorkflowSyncConfig({ workspaceId }) + .then((cfg) => { + if (cancelled) return; + setConfig(cfg); + reset(cfg); + }) + .catch((err) => { + if (cancelled) return; + toast({ + description: `Failed to load workflow sync config: ${String(err)}`, + variant: "error", + }); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [workspaceId]); +} + +export function useWorkflowSync(workspaceId: string) { + const { toast } = useToast(); + const router = useRouter(); + const [config, setConfig] = useState(null); + const { form, url, urlInvalid, update, setUrlInput, reset } = useWorkflowSyncForm(); + const [loading, setLoading] = useState(true); + const [saving, setSaving] = useState(false); + const [syncing, setSyncing] = useState(false); + + useWorkflowSyncInitialLoad(workspaceId, { setConfig, reset, setLoading, toast }); + + useWorkflowSyncConfigRefresh(workspaceId, setConfig); + + const handleSave = useCallback(async () => { + setSaving(true); + try { + const payload: WorkflowSyncSetConfigRequest = { + repo_owner: form.repo_owner.trim(), + repo_name: form.repo_name.trim(), + branch: form.branch.trim(), + path: form.path.trim(), + interval_seconds: form.interval_seconds, + poll_enabled: form.poll_enabled, + }; + const saved = await setWorkflowSyncConfig(payload, { workspaceId }); + setConfig(saved); + reset(saved); + toast({ description: "Workflow sync configuration saved", variant: "success" }); + return true; + } catch (err) { + toast({ description: `Save failed: ${String(err)}`, variant: "error" }); + return false; + } finally { + setSaving(false); + } + }, [workspaceId, form, toast, reset]); + + const handleDelete = useCallback(async () => { + if ( + !confirm("Remove workflow sync configuration? This will not delete already-synced workflows.") + ) { + return false; + } + try { + await deleteWorkflowSyncConfig({ workspaceId }); + setConfig(null); + reset(null); + toast({ description: "Workflow sync removed — synced workflows are editable again" }); + // Released workflows lose their read-only state server-side; reload the + // page data so the cards unlock without a manual refresh. + router.refresh(); + return true; + } catch (err) { + toast({ description: `Delete failed: ${String(err)}`, variant: "error" }); + return false; + } + }, [workspaceId, toast, reset, router]); + + const handleSyncNow = useCallback(async () => { + setSyncing(true); + try { + const res = await forceWorkflowSync({ workspaceId }); + setConfig(res.config); + reset(res.config); + toast(syncOutcomeToast(res.error, res.result?.warnings)); + // Reload the workflow list when the sync actually changed something so + // created/updated/deleted workflows show up without a manual refresh. + if (res.result && !res.result.unchanged) { + router.refresh(); + } + } catch (err) { + toast({ description: `Sync failed: ${String(err)}`, variant: "error" }); + } finally { + setSyncing(false); + } + }, [workspaceId, toast, reset, router]); + + return { + config, + form, + url, + urlInvalid, + loading, + saving, + syncing, + update, + setUrlInput, + handleSave, + handleDelete, + handleSyncNow, + }; +} + +export type WorkflowSyncController = ReturnType; diff --git a/apps/web/lib/api/domains/workflow-sync-api.test.ts b/apps/web/lib/api/domains/workflow-sync-api.test.ts new file mode 100644 index 0000000000..9e4a700a60 --- /dev/null +++ b/apps/web/lib/api/domains/workflow-sync-api.test.ts @@ -0,0 +1,112 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + getWorkflowSyncConfig, + setWorkflowSyncConfig, + deleteWorkflowSyncConfig, + forceWorkflowSync, +} from "./workflow-sync-api"; + +const CONFIG_PATH = "/api/v1/workflow-sync/config"; +const WS_PARAM = "workspace_id=ws-1"; + +const originalFetch = global.fetch; + +function mockResponse(data: unknown, status = 200) { + return new Response(data === undefined ? null : JSON.stringify(data), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +function makeConfigBody(overrides: Record = {}) { + return { + workspace_id: "ws-1", + repo_owner: "kdlbs", + repo_name: "kandev", + branch: "main", + path: ".kandev/workflows", + interval_seconds: 300, + last_ok: true, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + ...overrides, + }; +} + +describe("workflow-sync-api", () => { + let fetchSpy: ReturnType; + + beforeEach(() => { + fetchSpy = vi.fn(); + global.fetch = fetchSpy as unknown as typeof fetch; + }); + + afterEach(() => { + global.fetch = originalFetch; + vi.restoreAllMocks(); + }); + + it("getWorkflowSyncConfig calls /api/v1/workflow-sync/config with workspace_id", async () => { + fetchSpy.mockResolvedValueOnce(mockResponse(makeConfigBody())); + const config = await getWorkflowSyncConfig({ workspaceId: "ws-1" }); + expect(fetchSpy).toHaveBeenCalledTimes(1); + const url = fetchSpy.mock.calls[0]![0] as string; + expect(url).toContain(CONFIG_PATH); + expect(url).toContain(WS_PARAM); + expect(config?.repo_owner).toBe("kdlbs"); + }); + + it("getWorkflowSyncConfig returns null on 204 (not configured)", async () => { + fetchSpy.mockResolvedValueOnce(mockResponse(undefined, 204)); + const config = await getWorkflowSyncConfig({ workspaceId: "ws-1" }); + expect(config).toBeNull(); + }); + + it("setWorkflowSyncConfig POSTs the payload", async () => { + fetchSpy.mockResolvedValueOnce(mockResponse(makeConfigBody({ last_ok: false }))); + const saved = await setWorkflowSyncConfig( + { repo_owner: "kdlbs", repo_name: "kandev" }, + { workspaceId: "ws-1" }, + ); + const url = fetchSpy.mock.calls[0]![0] as string; + const init = fetchSpy.mock.calls[0]![1] as RequestInit; + expect(url).toContain(CONFIG_PATH); + expect(url).toContain(WS_PARAM); + expect(init.method).toBe("POST"); + expect(JSON.parse(init.body as string)).toEqual({ repo_owner: "kdlbs", repo_name: "kandev" }); + expect(saved.repo_name).toBe("kandev"); + }); + + it("deleteWorkflowSyncConfig issues DELETE", async () => { + fetchSpy.mockResolvedValueOnce(mockResponse({ deleted: true })); + const result = await deleteWorkflowSyncConfig({ workspaceId: "ws-1" }); + const url = String(fetchSpy.mock.calls[0]![0]); + expect(url).toContain(CONFIG_PATH); + expect(url).toContain(WS_PARAM); + const init = fetchSpy.mock.calls[0]![1] as RequestInit; + expect(init.method).toBe("DELETE"); + expect(result.deleted).toBe(true); + }); + + it("forceWorkflowSync POSTs with no body and returns config + result", async () => { + fetchSpy.mockResolvedValueOnce( + mockResponse({ + config: makeConfigBody(), + result: { created: ["a"], updated: [], deleted: [], warnings: [], unchanged: false }, + }), + ); + const res = await forceWorkflowSync({ workspaceId: "ws-1" }); + const url = fetchSpy.mock.calls[0]![0] as string; + const init = fetchSpy.mock.calls[0]![1] as RequestInit; + expect(url).toContain("/api/v1/workflow-sync/sync"); + expect(url).toContain(WS_PARAM); + expect(init.method).toBe("POST"); + expect(res.config.repo_owner).toBe("kdlbs"); + expect(res.result?.created).toEqual(["a"]); + }); + + it("forceWorkflowSync rejects with the parsed error body on 404 (not configured)", async () => { + fetchSpy.mockResolvedValueOnce(mockResponse({ error: "not configured" }, 404)); + await expect(forceWorkflowSync({ workspaceId: "ws-1" })).rejects.toThrow("not configured"); + }); +}); diff --git a/apps/web/lib/api/domains/workflow-sync-api.ts b/apps/web/lib/api/domains/workflow-sync-api.ts new file mode 100644 index 0000000000..e85543b291 --- /dev/null +++ b/apps/web/lib/api/domains/workflow-sync-api.ts @@ -0,0 +1,63 @@ +import { fetchJson, type ApiRequestOptions } from "../client"; +import type { + WorkflowSyncConfig, + WorkflowSyncForceSyncResponse, + WorkflowSyncSetConfigRequest, +} from "@/lib/types/workflow-sync"; + +type WorkspaceApiOptions = ApiRequestOptions & { workspaceId?: string }; + +function withWorkspace(path: string, options?: WorkspaceApiOptions): string { + if (!options?.workspaceId) return path; + const separator = path.includes("?") ? "&" : "?"; + return `${path}${separator}workspace_id=${encodeURIComponent(options.workspaceId)}`; +} + +function requestOptions(options?: WorkspaceApiOptions): ApiRequestOptions | undefined { + if (!options) return undefined; + const { workspaceId: _workspaceId, ...rest } = options; + return rest; +} + +// getWorkflowSyncConfig returns null when the backend responds 204 (no +// config yet for this workspace). fetchJson already maps 204 → undefined; we +// narrow it to null for callers. +export async function getWorkflowSyncConfig( + options?: WorkspaceApiOptions, +): Promise { + const res = await fetchJson( + withWorkspace(`/api/v1/workflow-sync/config`, options), + requestOptions(options), + ); + return res ?? null; +} + +export async function setWorkflowSyncConfig( + payload: WorkflowSyncSetConfigRequest, + options?: WorkspaceApiOptions, +) { + return fetchJson(withWorkspace(`/api/v1/workflow-sync/config`, options), { + ...requestOptions(options), + init: { ...(options?.init ?? {}), method: "POST", body: JSON.stringify(payload) }, + }); +} + +export async function deleteWorkflowSyncConfig(options?: WorkspaceApiOptions) { + return fetchJson<{ deleted: boolean }>(withWorkspace(`/api/v1/workflow-sync/config`, options), { + ...requestOptions(options), + init: { ...(options?.init ?? {}), method: "DELETE" }, + }); +} + +// forceWorkflowSync triggers an immediate sync. Rejects with an ApiError +// (404) when the workspace has no config; a failed sync attempt still +// resolves 200 with `error` set and `config.last_ok === false`. +export async function forceWorkflowSync(options?: WorkspaceApiOptions) { + return fetchJson( + withWorkspace(`/api/v1/workflow-sync/sync`, options), + { + ...requestOptions(options), + init: { ...(options?.init ?? {}), method: "POST" }, + }, + ); +} diff --git a/apps/web/lib/api/index.ts b/apps/web/lib/api/index.ts index 13f5048200..022e41a39b 100644 --- a/apps/web/lib/api/index.ts +++ b/apps/web/lib/api/index.ts @@ -8,5 +8,6 @@ export * from "./domains/workspace-api"; export * from "./domains/settings-api"; export * from "./domains/process-api"; export * from "./domains/workflow-api"; +export * from "./domains/workflow-sync-api"; export * from "./domains/github-api"; export * from "./domains/runtime-flags-api"; diff --git a/apps/web/lib/types/http.ts b/apps/web/lib/types/http.ts index 93f30d2771..78db803e4c 100644 --- a/apps/web/lib/types/http.ts +++ b/apps/web/lib/types/http.ts @@ -167,6 +167,14 @@ export type Workflow = { * shell (kanban board, office task pane, etc). Backend does NOT branch on it. */ style?: "kanban" | "office" | "custom"; + /** + * Workflow provenance. `"github"` marks workflows synced from a configured + * GitHub repo (see workflow sync); omitted/`"manual"` for user-created + * workflows. `source_path` is the repo-relative file the workflow was + * synced from and is omitted for manual workflows. + */ + source?: string; + source_path?: string; created_at: string; updated_at: string; }; diff --git a/apps/web/lib/types/workflow-sync.ts b/apps/web/lib/types/workflow-sync.ts new file mode 100644 index 0000000000..d41af7812e --- /dev/null +++ b/apps/web/lib/types/workflow-sync.ts @@ -0,0 +1,59 @@ +/** + * Per-workspace workflow sync configuration: a GitHub repo + branch + + * directory containing workflow export files (`.yml`/`.yaml`/`.json` in the + * `kandev_workflow` format). The backend polls the directory on + * `interval_seconds` cadence and syncs workflows; `last_*` fields report the + * outcome of the most recent sync attempt (poller or forced). + */ +export interface WorkflowSyncConfig { + workspace_id: string; + repo_owner: string; + repo_name: string; + branch: string; + path: string; + interval_seconds: number; + /** When false, the workspace only syncs via "Sync now". */ + poll_enabled: boolean; + /** RFC3339 timestamp; absent until the first sync attempt. */ + last_synced_at?: string; + last_ok: boolean; + last_error?: string; + last_warnings?: string[]; + created_at: string; + updated_at: string; +} + +/** + * Payload for creating or updating a workspace's workflow sync config. + * `branch`, `path`, and `interval_seconds` fall back to server-side defaults + * (`main`, `.kandev/workflows`, 300s / min 60s) when omitted. + */ +export interface WorkflowSyncSetConfigRequest { + repo_owner: string; + repo_name: string; + branch?: string; + path?: string; + interval_seconds?: number; + /** Defaults to true server-side when omitted. */ + poll_enabled?: boolean; +} + +/** Outcome of a single sync run (poller or forced). */ +export interface WorkflowSyncResult { + created: string[]; + updated: string[]; + deleted: string[]; + warnings: string[]; + unchanged: boolean; +} + +/** + * Response from a forced sync. A failed sync still responds 200 with `error` + * set and `config.last_ok === false` — the request itself only rejects (404) + * when no config exists for the workspace. + */ +export interface WorkflowSyncForceSyncResponse { + config: WorkflowSyncConfig; + result?: WorkflowSyncResult; + error?: string; +} diff --git a/apps/web/lib/utils/github-repo-url.test.ts b/apps/web/lib/utils/github-repo-url.test.ts new file mode 100644 index 0000000000..d3e308fb81 --- /dev/null +++ b/apps/web/lib/utils/github-repo-url.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, it } from "vitest"; +import { buildGitHubRepoUrl, parseGitHubRepoUrl } from "./github-repo-url"; + +describe("parseGitHubRepoUrl", () => { + it("parses a plain repository URL", () => { + expect(parseGitHubRepoUrl("https://github.com/jcfs/kandev-workflows-test")).toEqual({ + owner: "jcfs", + repo: "kandev-workflows-test", + }); + }); + + it("tolerates trailing slash, .git suffix, www, and missing scheme", () => { + expect(parseGitHubRepoUrl("https://github.com/jcfs/repo/")).toEqual({ + owner: "jcfs", + repo: "repo", + }); + expect(parseGitHubRepoUrl("https://www.github.com/jcfs/repo.git")).toEqual({ + owner: "jcfs", + repo: "repo", + }); + expect(parseGitHubRepoUrl("github.com/jcfs/repo")).toEqual({ owner: "jcfs", repo: "repo" }); + }); + + it("parses an SSH remote", () => { + expect(parseGitHubRepoUrl("git@github.com:jcfs/repo.git")).toEqual({ + owner: "jcfs", + repo: "repo", + }); + }); + + it("extracts branch and directory from a /tree/ link", () => { + expect( + parseGitHubRepoUrl( + "https://github.com/jcfs/kandev-workflows-test/tree/main/.kandev/workflows", + ), + ).toEqual({ + owner: "jcfs", + repo: "kandev-workflows-test", + branch: "main", + path: ".kandev/workflows", + }); + }); + + it("extracts branch without path from a branch-root /tree/ link", () => { + expect(parseGitHubRepoUrl("https://github.com/jcfs/repo/tree/develop")).toEqual({ + owner: "jcfs", + repo: "repo", + branch: "develop", + }); + }); + + it("resolves a /blob/ file link to the file's directory", () => { + expect( + parseGitHubRepoUrl("https://github.com/jcfs/repo/blob/main/.kandev/workflows/dev.yml"), + ).toEqual({ + owner: "jcfs", + repo: "repo", + branch: "main", + path: ".kandev/workflows", + }); + }); + + it("ignores unknown path markers beyond owner/repo", () => { + expect(parseGitHubRepoUrl("https://github.com/jcfs/repo/pulls")).toEqual({ + owner: "jcfs", + repo: "repo", + }); + }); + + it("rejects non-GitHub and malformed input", () => { + expect(parseGitHubRepoUrl("https://gitlab.com/jcfs/repo")).toBeNull(); + expect(parseGitHubRepoUrl("https://github.com/only-owner")).toBeNull(); + expect(parseGitHubRepoUrl("not a url at all :::")).toBeNull(); + expect(parseGitHubRepoUrl("")).toBeNull(); + }); + + it("returns null instead of throwing on malformed percent escapes", () => { + expect(parseGitHubRepoUrl("https://github.com/org/repo/tree/main/%")).toBeNull(); + expect(parseGitHubRepoUrl("https://github.com/org/repo/tree/main/%zz")).toBeNull(); + }); + + it("decodes percent-encoded path segments", () => { + expect(parseGitHubRepoUrl("https://github.com/jcfs/repo/tree/main/my%20flows")).toEqual({ + owner: "jcfs", + repo: "repo", + branch: "main", + path: "my flows", + }); + }); +}); + +describe("buildGitHubRepoUrl", () => { + it("renders owner/repo/branch/path back into a tree link", () => { + const url = buildGitHubRepoUrl({ + owner: "jcfs", + repo: "kandev-workflows-test", + branch: "main", + path: ".kandev/workflows", + }); + expect(url).toBe("https://github.com/jcfs/kandev-workflows-test/tree/main/.kandev/workflows"); + }); + + it("round-trips through parseGitHubRepoUrl", () => { + const parts = { owner: "jcfs", repo: "repo", branch: "dev", path: "my flows/sub" }; + expect(parseGitHubRepoUrl(buildGitHubRepoUrl(parts))).toEqual(parts); + }); + + it("omits the tree suffix without a branch and the path when empty", () => { + expect(buildGitHubRepoUrl({ owner: "jcfs", repo: "repo" })).toBe( + "https://github.com/jcfs/repo", + ); + expect(buildGitHubRepoUrl({ owner: "jcfs", repo: "repo", branch: "main" })).toBe( + "https://github.com/jcfs/repo/tree/main", + ); + }); +}); diff --git a/apps/web/lib/utils/github-repo-url.ts b/apps/web/lib/utils/github-repo-url.ts new file mode 100644 index 0000000000..b6022753e9 --- /dev/null +++ b/apps/web/lib/utils/github-repo-url.ts @@ -0,0 +1,64 @@ +export type ParsedGitHubRepoUrl = { + owner: string; + repo: string; + branch?: string; + path?: string; +}; + +const GITHUB_HOST = "github.com"; +const SSH_URL_RE = /^git@github\.com:([^/\s]+)\/([^/\s]+?)(?:\.git)?\/?$/; + +// parseGitHubRepoUrl extracts owner/repo (and branch + directory for +// /tree/... and /blob/... links) from a pasted GitHub URL. Returns null when +// the input isn't a recognizable GitHub repository link. +// +// Branch names containing "/" cannot be told apart from the leading path +// segments without asking the GitHub API, so single-segment branch names are +// assumed. A /blob/ link to a file resolves to the file's directory. +export function parseGitHubRepoUrl(input: string): ParsedGitHubRepoUrl | null { + const raw = input.trim(); + if (!raw) return null; + + const ssh = raw.match(SSH_URL_RE); + if (ssh) return { owner: ssh[1], repo: ssh[2] }; + + let url: URL; + try { + url = new URL(raw.includes("://") ? raw : `https://${raw}`); + } catch { + return null; + } + if (url.hostname !== GITHUB_HOST && url.hostname !== `www.${GITHUB_HOST}`) return null; + + let segments: string[]; + try { + segments = url.pathname.split("/").filter(Boolean).map(decodeURIComponent); + } catch { + // Malformed percent escapes (e.g. a trailing "%") must read as "not a + // recognized link", not crash the dialog on every keystroke. + return null; + } + if (segments.length < 2) return null; + const [owner, rawRepo, ...rest] = segments; + const repo = rawRepo.replace(/\.git$/, ""); + if (!owner || !repo) return null; + return { owner, repo, ...parseBranchAndPath(rest) }; +} + +// buildGitHubRepoUrl renders a stored config back into a canonical GitHub +// link (the inverse of parseGitHubRepoUrl) so the settings form can show one +// URL instead of separate owner/repo/path fields. +export function buildGitHubRepoUrl(parts: ParsedGitHubRepoUrl): string { + const base = `https://github.com/${parts.owner}/${parts.repo}`; + if (!parts.branch) return base; + const path = parts.path ? `/${parts.path.split("/").map(encodeURIComponent).join("/")}` : ""; + return `${base}/tree/${encodeURIComponent(parts.branch)}${path}`; +} + +function parseBranchAndPath(segments: string[]): Pick { + const [marker, branch, ...rest] = segments; + if ((marker !== "tree" && marker !== "blob") || !branch) return {}; + const pathSegments = marker === "blob" ? rest.slice(0, -1) : rest; + if (pathSegments.length === 0) return { branch }; + return { branch, path: pathSegments.join("/") }; +} diff --git a/docs/public/index.md b/docs/public/index.md index 7953f4a8fb..03d2212ba3 100644 --- a/docs/public/index.md +++ b/docs/public/index.md @@ -7,6 +7,7 @@ This page expands the short feature list in the README without turning the READM - **Parallel task execution:** run and review multiple agent tasks at once. - **Kanban and pipeline workflows:** define workflow steps, prompts, automations, and agent handoffs per step. - **Workflow import/export:** export one workflow or every workflow in a workspace as portable YAML, then import it into another workspace or Kandev install. See [workflow-import-export.md](workflow-import-export.md). +- **Workflow sync from GitHub:** keep a workspace's workflows in sync with definition files in a GitHub repo — Kandev polls the repo on an interval, with a force-sync option and warnings when a change can't be applied. See [workflow-sync.md](workflow-sync.md). - **Subtasks:** split work into child tasks that inherit the parent task's workspace, workflow, agent profile, executor, and repositories by default. - **Multi-repository tasks:** attach several repositories to one task. Each repository gets its own worktree and is grouped separately in changes, review, and PR surfaces. - **Multi-branch tasks:** attach multiple branches from the same repository to one task when the work needs to produce several PRs. diff --git a/docs/public/workflow-import-export.md b/docs/public/workflow-import-export.md index 9ae0ac6017..76d3c1e0c2 100644 --- a/docs/public/workflow-import-export.md +++ b/docs/public/workflow-import-export.md @@ -14,6 +14,10 @@ Everything below is derived from the source of truth: > **Looking for the built-in workflows instead?** See [Workflows](workflow-tips.md) > for the default templates and when to use each. +> **Want workflows managed from a git repo?** The same file format powers +> [workflow sync from GitHub](workflow-sync.md), which polls a configured repo +> and keeps workspace workflows up to date automatically. + --- ## How import/export is exposed diff --git a/docs/public/workflow-sync.md b/docs/public/workflow-sync.md new file mode 100644 index 0000000000..f61b72e17b --- /dev/null +++ b/docs/public/workflow-sync.md @@ -0,0 +1,115 @@ +# Workflow Sync — Manage Workflows from a GitHub Repo + +Kandev can keep a workspace's workflows in sync with definition files stored +in a GitHub repository. Point a workspace at a repo directory, commit workflow +export files there, and Kandev polls the repo and creates, updates, or removes +the matching workflows automatically. Synced workflows coexist with workflows +you create by hand — manual workflows are never touched by a sync. + +This is useful for: + +- **Sharing workflows across a team:** everyone's Kandev pulls the same + definitions from one reviewed repo. +- **Versioning workflows:** changes go through pull requests and history + instead of ad-hoc UI edits. +- **Provisioning new workspaces:** configure the repo once and the standard + workflows appear on the next sync. + +--- + +## Setup + +1. Open **Settings → Workspaces → \ → Workflows** and click the + **GitHub Sync** button at the top. +2. In the dialog, paste a GitHub link into **Repository link** — a plain repo + URL, an SSH remote, or a `…/tree//` link that also + carries the branch and directory. The resolved target (owner/repo and + directory) is shown under the field; the directory defaults to + `.kandev/workflows` when the link doesn't include one. +3. The branch comes from the pasted link (`main` when the link doesn't carry + one). Toggle **Auto-sync**: when on, Kandev polls the repo on the given + interval (default 300 seconds, minimum 60); when off, nothing syncs until + you press **Sync now**. +4. Save. A status card appears on the page showing what is syncing, the last + sync result, and any warnings; use its **Sync now** button to run a sync + immediately instead of waiting for the poller. Reopen the dialog via + **GitHub Sync** to change or remove the configuration. + +Authentication reuses Kandev's existing GitHub access (the `gh` CLI login, a +`GITHUB_TOKEN`/`GH_TOKEN` environment variable, or a PAT stored in the secret +manager). Private repos work as long as that identity can read them. + +## File format + +The directory should contain `.yml`, `.yaml`, or `.json` files in the portable +`kandev_workflow` export format — exactly what **Export** produces on the +Workflows settings page. See +[workflow-import-export.md](workflow-import-export.md) for the full field +reference. Files with other extensions are ignored; subdirectories are not +scanned. + +A minimal file: + +```yaml +version: 1 +type: kandev_workflow +workflows: + - name: Dev Flow + steps: + - name: Todo + position: 0 + is_start_step: true + - name: In Progress + position: 1 + - name: Done + position: 2 +``` + +The easiest authoring path: build the workflow in the Kandev UI, export it, +and commit the exported YAML to the repo. + +## Sync semantics + +- **Matching:** a synced workflow is identified by its source file path plus + its `name` inside that file. Renaming a workflow in the repo counts as + removing one workflow and adding another. +- **Create:** definitions with no matching workflow are created and marked as + synced (they show a **Synced** badge in the workflow list). +- **Update:** matched workflows are updated in place. Steps are matched **by + name**, so tasks sitting in a step keep their position when the step's + color, prompt, events, or order change. +- **Read-only:** the repo is the source of truth. Synced workflows cannot be + renamed, edited, or deleted from the UI (the API rejects such changes) — + edit their definitions in the repo instead. Any drift that slips in anyway + is repaired on the next sync. Reordering workflows on the board and + exporting them remain available. +- **Delete:** a previously-synced workflow whose definition disappeared from + the repo is deleted — but only if it holds no tasks. +- **Manual workflows** (created in the UI) are never modified or deleted by a + sync, even if they share a name with a synced definition. + +### When a workflow can't be updated + +Some changes can't be applied safely, and Kandev records a **warning** instead +of forcing them. The warning appears in the status banner of the Sync from +GitHub section until you resolve it and sync again. Cases: + +- A step was removed from the definition but still has tasks in it. +- A removed workflow still has tasks. +- Step names inside a definition (or the existing workflow) are not unique, + so steps can't be matched reliably. +- A file is not valid workflow-export YAML/JSON. The file is skipped and its + previously-synced workflows are left untouched. + +A failed sync (repo unreachable, directory missing, GitHub not authenticated) +is also surfaced in the same banner with the error message. + +## Notes + +- Sync configuration is **per workspace**; different workspaces can track + different repos, branches, or directories. +- Every sync — periodic or manual — reconciles the workspace against the repo, + but only writes what actually differs, so a no-drift sync changes (and + broadcasts) nothing. +- Removing the sync configuration releases the previously-synced workflows: + they stay in place but become regular, editable workflows again.