From 37b56ba11c01001f586ddc077c5d2581e00cca65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Mon, 27 Jul 2026 10:24:57 +0100 Subject: [PATCH] feat(automations): reliable cron scheduling and webhook-driven push/CI triggers (#1972) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(automations): reliable cron scheduling and webhook-driven push/CI triggers Automations were "wounded": cron schedules mishandled timezones and some never fired, and the GitHub push/CI trigger types were coming-soon stubs. Cron (fix): - The scheduler reduced every cron expression to a fixed interval derived only from a */N step in the minute/hour field, so any pinned schedule (0 9 * * *, 30 14 * * *, weekday ranges, day-of-month) never fired, and the configured timezone was parsed but never used. - Replace the interval approximation with robfig/cron/v3: shouldFire now computes the true next fire time in the configured timezone (DST-aware, defaulting to UTC for host-independence), anchored to last-evaluated or creation time. Dead interval-parsing helpers and shorthand constants removed. GitHub push + CI triggers (feat): - Implement push and CI-failure triggers end to end via the existing per-workspace GitHub App webhook plumbing (#1810): the HMAC-verified webhook service now parses push and completed check_run events, resolves the installation to its workspaces, and publishes normalized bus events. - A new automation subscriber matches enabled github_push / github_ci triggers by workspace + repo + branch/conclusion/check-name filters and fires them (deduped by commit SHA / check-run id). The old poll-based stub evaluators are removed; the PR poll path is unchanged. - Subscribe the App manifest to push/check_run and enable both trigger types in the registry (CI defaults to the failure conclusion). Docs: - Update docs/public/feature-status.md and automation-and-mcp.md so the cron and push/CI rows reflect shipped reality. Tests: regression tests pin the previously-never-firing cron shapes, timezone/DST behavior, and host-independence; backend tests cover the webhook parse->publish and subscriber match/fire/dedup flows for both push and CI-failure. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(automations): populate push.message and ci.url placeholders in fired trigger data Address PR review blockers: the default push and CI automation prompts reference {{push.message}} and {{ci.url}}, which the interpolator maps to data["message"] / data["html_url"] — but the webhook subscriber never populated those fields, so both always rendered empty. - Parse head_commit.message (push) and check_run.html_url (CI) in the GitHub webhook service and carry them on the published bus payloads. - Include "message" / "html_url" in the fired trigger data so the placeholders resolve; extend the webhook and subscriber tests to assert both flow through end to end. Non-blocking review cleanups: - Replace the misleading reuse of matchesAuthors for CI conclusions/check names with a clearly-named matchesFilterValue helper. - Drop dead context.Background() line and its unused import from the cron regression test. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(automations): scope webhook triggers to active installs and per-branch dedup Address CodeRabbit review on the push/CI webhook triggers: - Security: resolveWorkspaceIDs now only returns connections whose status is active, so a suspended or revoked GitHub App installation can no longer fire push/CI automations. - Correctness: include the branch in the push dedup key (push:owner/name@branch@sha) so the same commit pushed to two matching branches fires once per branch instead of being suppressed. - Correctness: add an optional head-branch filter to github_ci triggers and gate CI matching on it, matching the push trigger's branch-scoping. - Stability: make the webhook subscriber's Start retryable — a subscription failure rolls back partial subscriptions and leaves the subscriber un-started so a later Start re-attempts, instead of permanently disabling push/CI processing. - Tests: cover multi-connection workspace resolution (dedup + inactive filtering), per-branch push dedup, and CI branch filtering. - Docs: note the CI branch filter and per-branch push dedup. Co-Authored-By: Claude Opus 4.8 (1M context) * test(automations): cover webhook subscriber Start retry-after-failure path Add TestGitHubWebhookSubscriberStartRetryableAfterFailure with a minimal flaky bus whose second Subscribe fails, pinning the retry contract: a subscription failure rolls back the partial subscription and leaves the subscriber un-started, and a later Start re-attempts and succeeds. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 4.8 (1M context) --- apps/backend/go.mod | 1 + apps/backend/go.sum | 2 + apps/backend/internal/automation/evaluator.go | 69 +-- .../automation/github_webhook_subscriber.go | 206 +++++++++ .../github_webhook_subscriber_test.go | 428 ++++++++++++++++++ apps/backend/internal/automation/models.go | 19 +- apps/backend/internal/automation/provider.go | 22 +- apps/backend/internal/automation/scheduler.go | 145 +++--- .../automation/scheduler_cron_test.go | 202 +++++++++ .../internal/automation/trigger_registry.go | 22 +- apps/backend/internal/events/types.go | 2 + .../app_registration_test_helpers_test.go | 4 +- .../github/controller_webhook_test.go | 2 +- .../github/deployment_app_manifest.go | 7 + .../github/deployment_app_manifest_test.go | 4 +- .../internal/github/service_app_auth.go | 2 +- .../github/webhook_automation_events.go | 27 ++ .../internal/github/webhook_push_ci_test.go | 303 +++++++++++++ .../internal/github/webhook_service.go | 170 ++++++- .../internal/github/webhook_service_test.go | 24 +- docs/public/automation-and-mcp.md | 52 ++- docs/public/feature-status.md | 8 +- 22 files changed, 1515 insertions(+), 206 deletions(-) create mode 100644 apps/backend/internal/automation/github_webhook_subscriber.go create mode 100644 apps/backend/internal/automation/github_webhook_subscriber_test.go create mode 100644 apps/backend/internal/automation/scheduler_cron_test.go create mode 100644 apps/backend/internal/github/webhook_automation_events.go create mode 100644 apps/backend/internal/github/webhook_push_ci_test.go diff --git a/apps/backend/go.mod b/apps/backend/go.mod index 9ecf327016..0ccd2573ea 100644 --- a/apps/backend/go.mod +++ b/apps/backend/go.mod @@ -96,6 +96,7 @@ require ( github.com/opencontainers/image-spec v1.1.1 // indirect github.com/pelletier/go-toml/v2 v2.0.8 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/robfig/cron/v3 v3.0.1 // indirect github.com/spf13/afero v1.9.5 // indirect github.com/spf13/cast v1.7.1 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect diff --git a/apps/backend/go.sum b/apps/backend/go.sum index aec989bf53..aedfc81ca3 100644 --- a/apps/backend/go.sum +++ b/apps/backend/go.sum @@ -296,6 +296,8 @@ github.com/pkg/sftp v1.13.10/go.mod h1:bJ1a7uDhrX/4OII+agvy28lzRvQrmIQuaHrcI1Hbe github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= +github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= +github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= diff --git a/apps/backend/internal/automation/evaluator.go b/apps/backend/internal/automation/evaluator.go index c933263a02..e1b1db1cbf 100644 --- a/apps/backend/internal/automation/evaluator.go +++ b/apps/backend/internal/automation/evaluator.go @@ -83,8 +83,6 @@ func (e *GitHubEvaluator) loop(ctx context.Context) { func (e *GitHubEvaluator) evaluate(ctx context.Context) { e.evaluatePRTriggers(ctx) - e.evaluatePushTriggers(ctx) - e.evaluateCITriggers(ctx) } func (e *GitHubEvaluator) evaluatePRTriggers(ctx context.Context) { @@ -170,52 +168,6 @@ func (e *GitHubEvaluator) firePRTrigger(ctx context.Context, t *AutomationTrigge } } -func (e *GitHubEvaluator) evaluatePushTriggers(ctx context.Context) { - triggers, err := e.svc.Store().ListEnabledTriggersByType(ctx, TriggerTypeGitHubPush) - if err != nil { - e.logger.Error("failed to list github_push triggers", zap.Error(err)) - return - } - for i := range triggers { - t := &triggers[i] - e.checkPushTrigger(ctx, t) - } -} - -func (e *GitHubEvaluator) checkPushTrigger(ctx context.Context, t *AutomationTrigger) { - var cfg GitHubPushTriggerConfig - if err := json.Unmarshal(t.Config, &cfg); err != nil { - return - } - // Push trigger evaluation requires comparing commit SHAs. - // For now, mark as evaluated; full implementation needs commit tracking. - now := time.Now().UTC() - _ = e.svc.Store().UpdateTriggerEvaluatedAt(ctx, t.ID, now) -} - -func (e *GitHubEvaluator) evaluateCITriggers(ctx context.Context) { - triggers, err := e.svc.Store().ListEnabledTriggersByType(ctx, TriggerTypeGitHubCI) - if err != nil { - e.logger.Error("failed to list github_ci triggers", zap.Error(err)) - return - } - for i := range triggers { - t := &triggers[i] - e.checkCITrigger(ctx, t) - } -} - -func (e *GitHubEvaluator) checkCITrigger(ctx context.Context, t *AutomationTrigger) { - var cfg GitHubCITriggerConfig - if err := json.Unmarshal(t.Config, &cfg); err != nil { - return - } - // CI trigger evaluation requires tracking check run completion. - // For now, mark as evaluated; full implementation needs check run tracking. - now := time.Now().UTC() - _ = e.svc.Store().UpdateTriggerEvaluatedAt(ctx, t.ID, now) -} - // matchesBranches checks if a branch matches any of the filter patterns. // Empty filter means match all. func matchesBranches(branch string, filters []string) bool { @@ -230,14 +182,33 @@ func matchesBranches(branch string, filters []string) bool { return false } +// matchesRepo checks if owner/name matches any entry in the repo filter list. +// An empty list never matches — webhook-driven push/CI triggers cannot act +// without knowing which repo(s) to scope to. +func matchesRepo(owner, name string, repos []github.RepoFilter) bool { + for _, r := range repos { + if r.Owner == owner && r.Name == name { + return true + } + } + return false +} + // matchesAuthors checks if an author matches the filter list. // Empty filter means match all. func matchesAuthors(author string, filters []string) bool { + return matchesFilterValue(author, filters) +} + +// matchesFilterValue reports whether value equals one of the filters, treating +// an empty filter list as "match all". Used for exact-string filters such as +// PR authors, CI conclusions, and CI check names. +func matchesFilterValue(value string, filters []string) bool { if len(filters) == 0 { return true } for _, f := range filters { - if f == author { + if f == value { return true } } diff --git a/apps/backend/internal/automation/github_webhook_subscriber.go b/apps/backend/internal/automation/github_webhook_subscriber.go new file mode 100644 index 0000000000..81f303ba87 --- /dev/null +++ b/apps/backend/internal/automation/github_webhook_subscriber.go @@ -0,0 +1,206 @@ +package automation + +import ( + "context" + "encoding/json" + "fmt" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/github" +) + +// GitHubWebhookSubscriber turns webhook-driven GitHub bus events (push, +// completed check runs) into fired github_push / github_ci automation +// triggers. It owns its subscriptions: Start registers handlers on the bus, +// Stop unsubscribes. The bus delivers to handlers synchronously on the +// publisher's goroutine, so this type spawns no goroutines of its own. +type GitHubWebhookSubscriber struct { + svc *Service + eventBus bus.EventBus + logger *logger.Logger + + subs []bus.Subscription + started bool +} + +// NewGitHubWebhookSubscriber creates a new subscriber. +func NewGitHubWebhookSubscriber(svc *Service, eventBus bus.EventBus, log *logger.Logger) *GitHubWebhookSubscriber { + return &GitHubWebhookSubscriber{svc: svc, eventBus: eventBus, logger: log} +} + +// Start subscribes to the GitHub webhook bus events. Idempotent, and +// retryable: a subscription failure leaves the subscriber un-started (partial +// subscriptions are rolled back) so a later Start re-attempts both, rather than +// permanently disabling push/CI processing. +func (s *GitHubWebhookSubscriber) Start(_ context.Context) { + if s.started || s.eventBus == nil { + return + } + + subs, err := s.subscribeAll() + if err != nil { + for _, sub := range subs { + _ = sub.Unsubscribe() + } + s.logger.Error("failed to subscribe automation GitHub webhook events; will retry on next Start", + zap.Error(err)) + return + } + + s.subs = subs + s.started = true + s.logger.Info("automation GitHub webhook subscriber started") +} + +// subscribeAll subscribes to both webhook events, returning any subscriptions +// created so far alongside the first error (for rollback by the caller). +func (s *GitHubWebhookSubscriber) subscribeAll() ([]bus.Subscription, error) { + var subs []bus.Subscription + pushSub, err := s.eventBus.Subscribe(events.GitHubPushReceived, s.handlePush) + if err != nil { + return subs, fmt.Errorf("subscribe %s: %w", events.GitHubPushReceived, err) + } + subs = append(subs, pushSub) + ciSub, err := s.eventBus.Subscribe(events.GitHubCheckRunCompleted, s.handleCheckRun) + if err != nil { + return subs, fmt.Errorf("subscribe %s: %w", events.GitHubCheckRunCompleted, err) + } + return append(subs, ciSub), nil +} + +// Stop unsubscribes from the bus. Idempotent. +func (s *GitHubWebhookSubscriber) Stop() { + if !s.started { + return + } + for _, sub := range s.subs { + if sub != nil { + _ = sub.Unsubscribe() + } + } + s.subs = nil + s.started = false + s.logger.Info("automation GitHub webhook subscriber stopped") +} + +func (s *GitHubWebhookSubscriber) handlePush(ctx context.Context, event *bus.Event) error { + payload, ok := event.Data.(*github.GitHubPushEventPayload) + if !ok || payload == nil { + return nil + } + triggers, err := s.svc.Store().ListEnabledTriggersByType(ctx, TriggerTypeGitHubPush) + if err != nil { + s.logger.Error("failed to list github_push triggers", zap.Error(err)) + return nil + } + for i := range triggers { + s.checkPushTrigger(ctx, &triggers[i], payload) + } + return nil +} + +func (s *GitHubWebhookSubscriber) checkPushTrigger( + ctx context.Context, t *AutomationTrigger, payload *github.GitHubPushEventPayload, +) { + var cfg GitHubPushTriggerConfig + if err := json.Unmarshal(t.Config, &cfg); err != nil { + s.logger.Debug("invalid github_push trigger config", zap.String("trigger_id", t.ID), zap.Error(err)) + return + } + if !s.triggerMatchesWorkspaceAndRepo(ctx, t, payload.WorkspaceIDs, payload.Owner, payload.Name, cfg.Repos) { + return + } + if !matchesBranches(payload.Branch, cfg.Branches) { + return + } + + // The same commit SHA can be pushed to multiple matching branches; key the + // dedup on the branch too so a later branch's push isn't suppressed. + dedupKey := fmt.Sprintf("push:%s/%s@%s@%s", payload.Owner, payload.Name, payload.Branch, payload.SHA) + data, _ := json.Marshal(map[string]interface{}{ + "repo": fmt.Sprintf("%s/%s", payload.Owner, payload.Name), + "branch": payload.Branch, + "sha": payload.SHA, + "pusher_login": payload.PusherLogin, + "message": payload.HeadCommitMsg, + }) + if err := s.svc.FireTrigger(ctx, t.AutomationID, t.ID, TriggerTypeGitHubPush, data, dedupKey); err != nil { + s.logger.Error("failed to fire push trigger", zap.String("trigger_id", t.ID), zap.Error(err)) + } +} + +func (s *GitHubWebhookSubscriber) handleCheckRun(ctx context.Context, event *bus.Event) error { + payload, ok := event.Data.(*github.GitHubCheckRunEventPayload) + if !ok || payload == nil { + return nil + } + triggers, err := s.svc.Store().ListEnabledTriggersByType(ctx, TriggerTypeGitHubCI) + if err != nil { + s.logger.Error("failed to list github_ci triggers", zap.Error(err)) + return nil + } + for i := range triggers { + s.checkCITrigger(ctx, &triggers[i], payload) + } + return nil +} + +func (s *GitHubWebhookSubscriber) checkCITrigger( + ctx context.Context, t *AutomationTrigger, payload *github.GitHubCheckRunEventPayload, +) { + var cfg GitHubCITriggerConfig + if err := json.Unmarshal(t.Config, &cfg); err != nil { + s.logger.Debug("invalid github_ci trigger config", zap.String("trigger_id", t.ID), zap.Error(err)) + return + } + if !s.triggerMatchesWorkspaceAndRepo(ctx, t, payload.WorkspaceIDs, payload.Owner, payload.Name, cfg.Repos) { + return + } + if !matchesFilterValue(payload.Conclusion, cfg.Conclusions) || + !matchesFilterValue(payload.CheckName, cfg.CheckNames) || + !matchesBranches(payload.Branch, cfg.Branches) { + return + } + + dedupKey := fmt.Sprintf("ci:%s/%s#%d", payload.Owner, payload.Name, payload.CheckRunID) + data, _ := json.Marshal(map[string]interface{}{ + "repo": fmt.Sprintf("%s/%s", payload.Owner, payload.Name), + "branch": payload.Branch, + "sha": payload.SHA, + "check_name": payload.CheckName, + "conclusion": payload.Conclusion, + "check_run_id": payload.CheckRunID, + "html_url": payload.HTMLURL, + }) + if err := s.svc.FireTrigger(ctx, t.AutomationID, t.ID, TriggerTypeGitHubCI, data, dedupKey); err != nil { + s.logger.Error("failed to fire CI trigger", zap.String("trigger_id", t.ID), zap.Error(err)) + } +} + +// triggerMatchesWorkspaceAndRepo loads the trigger's owning automation and +// checks its workspace is among the event's resolved workspaces, and that +// the event's repo matches the trigger's repo filter (empty repo filter +// never matches — these webhook triggers cannot act without a repo). +func (s *GitHubWebhookSubscriber) triggerMatchesWorkspaceAndRepo( + ctx context.Context, t *AutomationTrigger, workspaceIDs []string, owner, name string, repos []github.RepoFilter, +) bool { + if !matchesRepo(owner, name, repos) { + return false + } + a, err := s.svc.GetAutomation(ctx, t.AutomationID) + if err != nil || a == nil { + s.logger.Debug("automation trigger is missing workspace ownership", + zap.String("trigger_id", t.ID), zap.Error(err)) + return false + } + for _, id := range workspaceIDs { + if id == a.WorkspaceID { + return true + } + } + return false +} diff --git a/apps/backend/internal/automation/github_webhook_subscriber_test.go b/apps/backend/internal/automation/github_webhook_subscriber_test.go new file mode 100644 index 0000000000..274a3abdac --- /dev/null +++ b/apps/backend/internal/automation/github_webhook_subscriber_test.go @@ -0,0 +1,428 @@ +package automation + +import ( + "context" + "encoding/json" + "errors" + "testing" + "time" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/github" +) + +var errFlakySubscribe = errors.New("flaky subscribe failure") + +func newTestSubscriber(t *testing.T, svc *Service) *GitHubWebhookSubscriber { + t.Helper() + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + t.Fatal(err) + } + sub := NewGitHubWebhookSubscriber(svc, svc.eventBus, log) + sub.Start(context.Background()) + t.Cleanup(sub.Stop) + return sub +} + +func createTestAutomation(t *testing.T, svc *Service, workspaceID string) *Automation { + t.Helper() + a, err := svc.CreateAutomation(context.Background(), &CreateAutomationRequest{ + WorkspaceID: workspaceID, + Name: "test automation", + AgentProfileID: "agent-1", + ExecutorProfileID: "exec-1", + ExecutionMode: ExecutionModeRun, + }) + if err != nil { + t.Fatalf("CreateAutomation() error = %v", err) + } + return a +} + +func addTestTrigger(t *testing.T, svc *Service, automationID string, triggerType TriggerType, cfg interface{}) *AutomationTrigger { + t.Helper() + raw, err := json.Marshal(cfg) + if err != nil { + t.Fatal(err) + } + trig, err := svc.AddTrigger(context.Background(), &AddTriggerRequest{ + AutomationID: automationID, + Type: triggerType, + Config: raw, + Enabled: true, + }) + if err != nil { + t.Fatalf("AddTrigger() error = %v", err) + } + return trig +} + +func subscribeAutomationTriggered(t *testing.T, eventBus bus.EventBus) chan *AutomationTriggeredEvent { + t.Helper() + ch := make(chan *AutomationTriggeredEvent, 4) + if _, err := eventBus.Subscribe(events.AutomationTriggered, func(_ context.Context, e *bus.Event) error { + if evt, ok := e.Data.(*AutomationTriggeredEvent); ok { + ch <- evt + } + return nil + }); err != nil { + t.Fatal(err) + } + return ch +} + +func TestGitHubWebhookSubscriberFiresCheckRunTrigger(t *testing.T) { + svc := newTestService(t) + newTestSubscriber(t, svc) + fired := subscribeAutomationTriggered(t, svc.eventBus) + + a := createTestAutomation(t, svc, "ws-1") + trig := addTestTrigger(t, svc, a.ID, TriggerTypeGitHubCI, GitHubCITriggerConfig{ + Repos: []github.RepoFilter{{Owner: "acme", Name: "repo"}}, + Conclusions: []string{"failure"}, + }) + + if err := svc.eventBus.Publish(context.Background(), events.GitHubCheckRunCompleted, bus.NewEvent( + events.GitHubCheckRunCompleted, "test", &github.GitHubCheckRunEventPayload{ + WorkspaceIDs: []string{"ws-1"}, + Owner: "acme", + Name: "repo", + Branch: "main", + SHA: "abc1234", + CheckName: "build", + Conclusion: "failure", + CheckRunID: 555, + HTMLURL: "https://github.com/acme/repo/runs/555", + }, + )); err != nil { + t.Fatal(err) + } + + select { + case evt := <-fired: + if evt.AutomationID != a.ID || evt.TriggerID != trig.ID || evt.TriggerType != TriggerTypeGitHubCI { + t.Fatalf("evt = %+v", evt) + } + if evt.DedupKey != "ci:acme/repo#555" { + t.Fatalf("dedup key = %q", evt.DedupKey) + } + // The {{ci.url}} placeholder resolves from data["html_url"]; assert the + // subscriber populated it so the default CI prompt doesn't render empty. + var data map[string]interface{} + if err := json.Unmarshal(evt.TriggerData, &data); err != nil { + t.Fatal(err) + } + if data["html_url"] != "https://github.com/acme/repo/runs/555" { + t.Fatalf("expected html_url in trigger data, got %+v", data) + } + default: + t.Fatal("expected AutomationTriggered to fire") + } +} + +func TestGitHubWebhookSubscriberCheckRunNegativeCases(t *testing.T) { + base := github.GitHubCheckRunEventPayload{ + WorkspaceIDs: []string{"ws-1"}, + Owner: "acme", + Name: "repo", + Branch: "main", + SHA: "abc1234", + CheckName: "build", + Conclusion: "failure", + CheckRunID: 555, + } + + tests := []struct { + name string + mutate func(p *github.GitHubCheckRunEventPayload) + }{ + {"wrong workspace", func(p *github.GitHubCheckRunEventPayload) { p.WorkspaceIDs = []string{"ws-other"} }}, + {"wrong repo", func(p *github.GitHubCheckRunEventPayload) { p.Name = "other-repo" }}, + {"wrong conclusion", func(p *github.GitHubCheckRunEventPayload) { p.Conclusion = "success" }}, + {"wrong check name", func(p *github.GitHubCheckRunEventPayload) {}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc := newTestService(t) + newTestSubscriber(t, svc) + fired := subscribeAutomationTriggered(t, svc.eventBus) + + a := createTestAutomation(t, svc, "ws-1") + cfg := GitHubCITriggerConfig{ + Repos: []github.RepoFilter{{Owner: "acme", Name: "repo"}}, + Conclusions: []string{"failure"}, + } + if tt.name == "wrong check name" { + cfg.CheckNames = []string{"lint"} + } + addTestTrigger(t, svc, a.ID, TriggerTypeGitHubCI, cfg) + + payload := base + tt.mutate(&payload) + if err := svc.eventBus.Publish(context.Background(), events.GitHubCheckRunCompleted, bus.NewEvent( + events.GitHubCheckRunCompleted, "test", &payload, + )); err != nil { + t.Fatal(err) + } + + select { + case evt := <-fired: + t.Fatalf("expected no fire, got %+v", evt) + default: + } + }) + } +} + +func TestGitHubWebhookSubscriberPushTriggerBranchMatch(t *testing.T) { + svc := newTestService(t) + newTestSubscriber(t, svc) + fired := subscribeAutomationTriggered(t, svc.eventBus) + + a := createTestAutomation(t, svc, "ws-1") + trig := addTestTrigger(t, svc, a.ID, TriggerTypeGitHubPush, GitHubPushTriggerConfig{ + Repos: []github.RepoFilter{{Owner: "acme", Name: "repo"}}, + Branches: []string{"main"}, + }) + + publishPush := func(branch, sha string) { + if err := svc.eventBus.Publish(context.Background(), events.GitHubPushReceived, bus.NewEvent( + events.GitHubPushReceived, "test", &github.GitHubPushEventPayload{ + WorkspaceIDs: []string{"ws-1"}, + Owner: "acme", + Name: "repo", + Branch: branch, + SHA: sha, + PusherLogin: "alice", + HeadCommitMsg: "fix: bug", + }, + )); err != nil { + t.Fatal(err) + } + } + + // Non-matching branch: no fire. + publishPush("develop", "sha-1") + select { + case evt := <-fired: + t.Fatalf("expected no fire for non-matching branch, got %+v", evt) + default: + } + + // Matching branch: fires. + publishPush("main", "sha-2") + var dedupKey string + select { + case evt := <-fired: + if evt.AutomationID != a.ID || evt.TriggerID != trig.ID { + t.Fatalf("evt = %+v", evt) + } + if evt.DedupKey != "push:acme/repo@main@sha-2" { + t.Fatalf("dedup key = %q", evt.DedupKey) + } + // {{push.message}} resolves from data["message"]; assert it's populated. + var data map[string]interface{} + if err := json.Unmarshal(evt.TriggerData, &data); err != nil { + t.Fatal(err) + } + if data["message"] != "fix: bug" { + t.Fatalf("expected message in trigger data, got %+v", data) + } + dedupKey = evt.DedupKey + default: + t.Fatal("expected AutomationTriggered to fire for matching branch") + } + + // A real deployment records a run row for the fired trigger (the + // orchestrator does this on AutomationTriggered); do so here so the + // dedup check in FireTrigger has something to find. + if err := svc.RecordRun(context.Background(), &AutomationRun{ + AutomationID: a.ID, TriggerID: trig.ID, TriggerType: TriggerTypeGitHubPush, + Status: RunStatusTaskCreated, DedupKey: dedupKey, + }); err != nil { + t.Fatal(err) + } + + // Same SHA again: dedup key prevents double-fire. + publishPush("main", "sha-2") + select { + case evt := <-fired: + t.Fatalf("expected dedup to suppress second fire, got %+v", evt) + default: + } +} + +// TestGitHubWebhookSubscriberPushDedupIsPerBranch pins that the same commit SHA +// pushed to two different (matching) branches fires once per branch, while a +// duplicate delivery on one branch stays suppressed. +func TestGitHubWebhookSubscriberPushDedupIsPerBranch(t *testing.T) { + svc := newTestService(t) + newTestSubscriber(t, svc) + fired := subscribeAutomationTriggered(t, svc.eventBus) + + a := createTestAutomation(t, svc, "ws-1") + trig := addTestTrigger(t, svc, a.ID, TriggerTypeGitHubPush, GitHubPushTriggerConfig{ + Repos: []github.RepoFilter{{Owner: "acme", Name: "repo"}}, + Branches: []string{"main", "develop"}, + }) + + publishPush := func(branch, sha string) { + if err := svc.eventBus.Publish(context.Background(), events.GitHubPushReceived, bus.NewEvent( + events.GitHubPushReceived, "test", &github.GitHubPushEventPayload{ + WorkspaceIDs: []string{"ws-1"}, Owner: "acme", Name: "repo", + Branch: branch, SHA: sha, + }, + )); err != nil { + t.Fatal(err) + } + } + recordFired := func() { + select { + case evt := <-fired: + // Record as a terminal (succeeded) run: it still carries the dedup + // key for same-key suppression, but doesn't count toward the + // concurrency cap, so a distinct-branch push can still fire. + if err := svc.RecordRun(context.Background(), &AutomationRun{ + AutomationID: a.ID, TriggerID: trig.ID, TriggerType: TriggerTypeGitHubPush, + Status: RunStatusSucceeded, DedupKey: evt.DedupKey, + }); err != nil { + t.Fatal(err) + } + default: + t.Fatal("expected AutomationTriggered to fire") + } + } + + // Same SHA on two branches: each fires (distinct dedup keys). + publishPush("main", "shaX") + recordFired() + publishPush("develop", "shaX") + recordFired() + + // Duplicate delivery on the same branch is suppressed. + publishPush("main", "shaX") + select { + case evt := <-fired: + t.Fatalf("expected same-branch duplicate to be suppressed, got %+v", evt) + default: + } +} + +// TestGitHubWebhookSubscriberCITriggerBranchFilter pins that a CI trigger scoped +// to a branch only fires for check runs on that branch. +func TestGitHubWebhookSubscriberCITriggerBranchFilter(t *testing.T) { + svc := newTestService(t) + newTestSubscriber(t, svc) + fired := subscribeAutomationTriggered(t, svc.eventBus) + + a := createTestAutomation(t, svc, "ws-1") + addTestTrigger(t, svc, a.ID, TriggerTypeGitHubCI, GitHubCITriggerConfig{ + Repos: []github.RepoFilter{{Owner: "acme", Name: "repo"}}, + Conclusions: []string{"failure"}, + Branches: []string{"main"}, + }) + + publishCI := func(branch string, runID int64) { + if err := svc.eventBus.Publish(context.Background(), events.GitHubCheckRunCompleted, bus.NewEvent( + events.GitHubCheckRunCompleted, "test", &github.GitHubCheckRunEventPayload{ + WorkspaceIDs: []string{"ws-1"}, Owner: "acme", Name: "repo", + Branch: branch, SHA: "abc", CheckName: "build", Conclusion: "failure", CheckRunID: runID, + }, + )); err != nil { + t.Fatal(err) + } + } + + // Wrong branch: no fire. + publishCI("develop", 1) + select { + case evt := <-fired: + t.Fatalf("expected no fire for non-matching CI branch, got %+v", evt) + default: + } + + // Matching branch: fires. + publishCI("main", 2) + select { + case <-fired: + default: + t.Fatal("expected CI trigger to fire for matching branch") + } +} + +func TestGitHubWebhookSubscriberStopDrainsCleanly(t *testing.T) { + svc := newTestService(t) + sub := newTestSubscriber(t, svc) + sub.Stop() + // Calling Stop twice (once here, once via t.Cleanup) must be safe. +} + +// flakySubscribeBus is a minimal bus.EventBus whose Subscribe fails on the +// call whose 1-based index equals failCall (0 = never), used to exercise the +// subscriber's retryable-Start contract. It tracks the count of live +// subscriptions so a test can assert partial subscriptions are rolled back. +type flakySubscribeBus struct { + subscribeCalls int + failCall int + activeCount int +} + +type flakySub struct{ bus *flakySubscribeBus } + +func (s *flakySub) Unsubscribe() error { s.bus.activeCount--; return nil } +func (s *flakySub) IsValid() bool { return true } + +func (b *flakySubscribeBus) Subscribe(string, bus.EventHandler) (bus.Subscription, error) { + b.subscribeCalls++ + if b.subscribeCalls == b.failCall { + return nil, errFlakySubscribe + } + b.activeCount++ + return &flakySub{bus: b}, nil +} + +func (b *flakySubscribeBus) Publish(context.Context, string, *bus.Event) error { return nil } +func (b *flakySubscribeBus) QueueSubscribe(string, string, bus.EventHandler) (bus.Subscription, error) { + return nil, nil +} + +func (b *flakySubscribeBus) Request(context.Context, string, *bus.Event, time.Duration) (*bus.Event, error) { + return nil, nil +} +func (b *flakySubscribeBus) Close() {} +func (b *flakySubscribeBus) IsConnected() bool { return true } + +// TestGitHubWebhookSubscriberStartRetryableAfterFailure pins the retry +// contract: a failed subscription leaves the subscriber un-started with any +// partial subscription rolled back, and a later Start re-attempts and succeeds. +func TestGitHubWebhookSubscriberStartRetryableAfterFailure(t *testing.T) { + svc := newTestService(t) + fb := &flakySubscribeBus{failCall: 2} // first Subscribe ok, second fails + log, _ := logger.NewFromZap(zap.NewNop()) + sub := NewGitHubWebhookSubscriber(svc, fb, log) + t.Cleanup(sub.Stop) + + sub.Start(context.Background()) + if sub.started { + t.Fatal("expected subscriber to remain un-started after a subscription failure") + } + if fb.activeCount != 0 { + t.Fatalf("expected the partial subscription to be rolled back, active=%d", fb.activeCount) + } + + // Bus recovers; a later Start must re-attempt and succeed. + fb.failCall = 0 + sub.Start(context.Background()) + if !sub.started { + t.Fatal("expected subscriber to be started after a successful retry") + } + if fb.activeCount != 2 { + t.Fatalf("expected 2 live subscriptions after retry, active=%d", fb.activeCount) + } +} diff --git a/apps/backend/internal/automation/models.go b/apps/backend/internal/automation/models.go index 01dd8b74f9..c36ecd526e 100644 --- a/apps/backend/internal/automation/models.go +++ b/apps/backend/internal/automation/models.go @@ -22,18 +22,12 @@ const ( ) const ( - defaultBranchMain = "main" - exampleRepositoryOwner = "org/repo" - placeholderRepositoryOwner = "Repository (owner/name)" - triggerCategoryGitHub = "github" - triggerDataSourceKey = "source" - triggerDataSourceManual = "manual" - triggerCronHourlyShorthand = "@hourly" - triggerCronHourlyExpression = "0 * * * *" - triggerCronDailyShorthand = "@daily" - triggerCronDailyExpression = "0 0 * * *" - triggerCronWeeklyShorthand = "@weekly" - triggerCronWeeklyExpression = "0 0 * * 0" + defaultBranchMain = "main" + exampleRepositoryOwner = "org/repo" + placeholderRepositoryOwner = "Repository (owner/name)" + triggerCategoryGitHub = "github" + triggerDataSourceKey = "source" + triggerDataSourceManual = "manual" ) // RunStatus tracks the outcome of a trigger firing. @@ -174,6 +168,7 @@ type GitHubCITriggerConfig struct { Repos []github.RepoFilter `json:"repos"` Conclusions []string `json:"conclusions"` // success, failure, etc. CheckNames []string `json:"check_names,omitempty"` + Branches []string `json:"branches,omitempty"` // head-branch glob filter; empty = all } // WebhookTriggerConfig holds configuration for webhook triggers. diff --git a/apps/backend/internal/automation/provider.go b/apps/backend/internal/automation/provider.go index 44e41e772a..2c65ca1b2f 100644 --- a/apps/backend/internal/automation/provider.go +++ b/apps/backend/internal/automation/provider.go @@ -12,24 +12,28 @@ import ( // Components holds the automation subsystem components for lifecycle management. type Components struct { - Service *Service - Scheduler *CronScheduler - Evaluator *GitHubEvaluator + Service *Service + Scheduler *CronScheduler + Evaluator *GitHubEvaluator + WebhookSubscriber *GitHubWebhookSubscriber } -// Start begins background processing (scheduler + GitHub polling). +// Start begins background processing (scheduler + GitHub polling + webhook subscriber). func (c *Components) Start(ctx context.Context) { c.Scheduler.Start(ctx) c.Evaluator.Start(ctx) + c.WebhookSubscriber.Start(ctx) } // Stop gracefully shuts down background processing. func (c *Components) Stop() { c.Scheduler.Stop() c.Evaluator.Stop() + c.WebhookSubscriber.Stop() } -// Provide creates the full automation stack: store, service, scheduler, evaluator. +// Provide creates the full automation stack: store, service, scheduler, evaluator, +// webhook subscriber. func Provide( writer, reader *sqlx.DB, eventBus bus.EventBus, @@ -45,10 +49,12 @@ func Provide( scheduler := NewCronScheduler(svc, log) evaluator := NewGitHubEvaluator(svc, ghSvc, log) + webhookSubscriber := NewGitHubWebhookSubscriber(svc, eventBus, log) return &Components{ - Service: svc, - Scheduler: scheduler, - Evaluator: evaluator, + Service: svc, + Scheduler: scheduler, + Evaluator: evaluator, + WebhookSubscriber: webhookSubscriber, }, nil } diff --git a/apps/backend/internal/automation/scheduler.go b/apps/backend/internal/automation/scheduler.go index 0ba991588d..0fcbad2657 100644 --- a/apps/backend/internal/automation/scheduler.go +++ b/apps/backend/internal/automation/scheduler.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "fmt" + "strings" "sync" "time" + "github.com/robfig/cron/v3" "go.uber.org/zap" "github.com/kandev/kandev/internal/common/logger" @@ -100,9 +102,17 @@ func (cs *CronScheduler) evaluate(ctx context.Context) { } } -// shouldFire determines if a scheduled trigger is due based on its cron expression. -// Uses a simplified interval-based approach: parses the cron expression to derive -// an interval and checks if enough time has passed since last evaluation. +// shouldFire determines if a scheduled trigger is due based on its cron +// expression. It computes the next fire time after the trigger's anchor (the +// last time it was evaluated, or its creation time on first evaluation) in the +// trigger's configured timezone, and reports whether that time has arrived. +// +// This is a true wall-clock schedule, not an interval approximation: pinned +// expressions ("0 9 * * *"), weekday ranges, and day-of-month schedules all +// resolve to concrete instants, and the configured timezone (including its DST +// transitions) is honored. A schedule missed while the scheduler was down +// fires once on the next tick rather than once per missed occurrence, matching +// the per-minute dedup key in fire(). func (cs *CronScheduler) shouldFire(t *AutomationTrigger, now time.Time) bool { var cfg ScheduledTriggerConfig if err := json.Unmarshal(t.Config, &cfg); err != nil { @@ -114,7 +124,16 @@ func (cs *CronScheduler) shouldFire(t *AutomationTrigger, now time.Time) bool { return false } - interval, err := parseCronInterval(cfg.CronExpression) + // Anchor the next-fire computation to the last evaluation, falling back to + // the trigger's creation time so a pinned schedule created mid-day doesn't + // fire immediately — its first fire is the next scheduled occurrence after + // creation. + anchor := t.CreatedAt + if t.LastEvaluatedAt != nil { + anchor = *t.LastEvaluatedAt + } + + next, err := nextCronFire(cfg.CronExpression, cfg.Timezone, anchor) if err != nil { cs.logger.Debug("unparseable cron expression", zap.String("trigger_id", t.ID), @@ -122,11 +141,7 @@ func (cs *CronScheduler) shouldFire(t *AutomationTrigger, now time.Time) bool { zap.Error(err)) return false } - - if t.LastEvaluatedAt == nil { - return true - } - return now.Sub(*t.LastEvaluatedAt) >= interval + return !next.After(now) } func (cs *CronScheduler) fire(ctx context.Context, t *AutomationTrigger, now time.Time) { @@ -144,86 +159,48 @@ func (cs *CronScheduler) fire(ctx context.Context, t *AutomationTrigger, now tim } } -// parseCronInterval converts common cron expressions to a duration. -// Supports standard 5-field cron and shorthand like "@every 5m". -func parseCronInterval(expr string) (time.Duration, error) { - if d, ok := parseCronShorthand(expr); ok { - return d, nil - } - return parseCronFields(expr) -} +// cronParser accepts the standard 5-field cron syntax plus the named +// descriptors (@hourly, @daily, @weekly, @monthly, @yearly) and @every +// interval shorthand that the automation UI exposes as presets. +var cronParser = cron.NewParser( + cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor, +) -func parseCronShorthand(expr string) (time.Duration, bool) { - // Handle @every shorthand. - if len(expr) > 7 && expr[:6] == "@every" { - d, err := time.ParseDuration(expr[7:]) - if err == nil { - return d, true +// nextCronFire computes the next time the given cron expression fires strictly +// after `after`, interpreted in the given timezone. An empty timezone means +// UTC, so scheduling is deterministic regardless of the host's local clock. +// +// Wall-clock schedules honor the timezone's DST transitions (a "0 9 * * *" +// daily schedule always fires at 09:00 local, whether that is 13:00 or 14:00 +// UTC). The @every interval shorthand is timezone-independent by nature. +func nextCronFire(expr, timezone string, after time.Time) (time.Time, error) { + expr = strings.TrimSpace(expr) + if expr == "" { + return time.Time{}, fmt.Errorf("empty cron expression") + } + + // Bind the schedule to its timezone. @every is a pure interval and the + // parser rejects a CRON_TZ prefix on it, so only wall-clock schedules get + // the prefix. An expression that already carries its own TZ/CRON_TZ prefix + // is left untouched. + if !strings.HasPrefix(expr, "@every") && !hasCronTZPrefix(expr) { + tz := timezone + if tz == "" { + tz = "UTC" } - } - // Common presets. - switch expr { - case triggerCronHourlyShorthand, triggerCronHourlyExpression: - return time.Hour, true - case triggerCronDailyShorthand, triggerCronDailyExpression: - return 24 * time.Hour, true - case triggerCronWeeklyShorthand, triggerCronWeeklyExpression: - return 7 * 24 * time.Hour, true - } - return 0, false -} - -func parseCronFields(expr string) (time.Duration, error) { - fields := splitFields(expr) - if len(fields) < 2 { - return 0, fmt.Errorf("invalid cron expression: %s", expr) - } - - minuteInterval := parseFieldInterval(fields[0], 60) - hourInterval := parseFieldInterval(fields[1], 24) - - if minuteInterval > 0 && minuteInterval < 60 { - return time.Duration(minuteInterval) * time.Minute, nil - } - if hourInterval > 0 && hourInterval < 24 { - return time.Duration(hourInterval) * time.Hour, nil - } - return 0, fmt.Errorf("unsupported cron expression (no step interval found): %s", expr) -} - -func splitFields(expr string) []string { - var fields []string - field := "" - for _, c := range expr { - if c == ' ' || c == '\t' { - if field != "" { - fields = append(fields, field) - field = "" - } - } else { - field += string(c) + if _, err := time.LoadLocation(tz); err != nil { + return time.Time{}, fmt.Errorf("invalid timezone %q: %w", tz, err) } + expr = "CRON_TZ=" + tz + " " + expr } - if field != "" { - fields = append(fields, field) + + schedule, err := cronParser.Parse(expr) + if err != nil { + return time.Time{}, fmt.Errorf("parse cron expression: %w", err) } - return fields + return schedule.Next(after), nil } -// parseFieldInterval extracts the step value from a cron field (e.g., "*/5" → 5). -func parseFieldInterval(field string, max int) int { - for i, c := range field { - if c == '/' { - val := 0 - for _, d := range field[i+1:] { - if d >= '0' && d <= '9' { - val = val*10 + int(d-'0') - } - } - if val > 0 && val <= max { - return val - } - } - } - return 0 +func hasCronTZPrefix(expr string) bool { + return strings.HasPrefix(expr, "TZ=") || strings.HasPrefix(expr, "CRON_TZ=") } diff --git a/apps/backend/internal/automation/scheduler_cron_test.go b/apps/backend/internal/automation/scheduler_cron_test.go new file mode 100644 index 0000000000..77f108a905 --- /dev/null +++ b/apps/backend/internal/automation/scheduler_cron_test.go @@ -0,0 +1,202 @@ +package automation + +import ( + "encoding/json" + "testing" + "time" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" +) + +// TestNextCronFire_PinnedExpressionsFire pins the "never fires" bug: the old +// interval-reducing scheduler only understood a `*/N` step in the minute or +// hour field, so any pinned expression (a fixed minute/hour, a weekday range, +// a day-of-month) produced "no step interval found" and never fired. Each of +// these must now yield a concrete next-fire time. +func TestNextCronFire_PinnedExpressionsFire(t *testing.T) { + // A Wednesday at 08:00 UTC — a deterministic anchor. + after := time.Date(2026, time.July, 22, 8, 0, 0, 0, time.UTC) + + cases := []struct { + name string + expr string + want time.Time + }{ + { + name: "daily at 09:00", + expr: "0 9 * * *", + want: time.Date(2026, time.July, 22, 9, 0, 0, 0, time.UTC), + }, + { + name: "daily at 14:30", + expr: "30 14 * * *", + want: time.Date(2026, time.July, 22, 14, 30, 0, 0, time.UTC), + }, + { + name: "weekdays at 09:15 skips to next weekday when after is already past", + expr: "15 9 * * 1-5", + // 08:00 Wed -> 09:15 same Wed. + want: time.Date(2026, time.July, 22, 9, 15, 0, 0, time.UTC), + }, + { + name: "first of month at 00:00", + expr: "0 0 1 * *", + want: time.Date(2026, time.August, 1, 0, 0, 0, 0, time.UTC), + }, + { + name: "sunday weekly (named alias via standard field)", + expr: "0 0 * * 0", + // Next Sunday after Wed 22nd is the 26th. + want: time.Date(2026, time.July, 26, 0, 0, 0, 0, time.UTC), + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := nextCronFire(tc.expr, "", after) + if err != nil { + t.Fatalf("nextCronFire(%q) error: %v", tc.expr, err) + } + if !got.Equal(tc.want) { + t.Fatalf("nextCronFire(%q) = %s, want %s", tc.expr, got.UTC(), tc.want) + } + }) + } +} + +// TestNextCronFire_ShorthandStillWorks guards the shipped presets that DID work +// under the old scheduler (@every / @hourly / @daily / @weekly). They must keep +// firing after the rewrite. +func TestNextCronFire_ShorthandStillWorks(t *testing.T) { + after := time.Date(2026, time.July, 22, 8, 30, 0, 0, time.UTC) + + cases := []struct { + expr string + want time.Time + }{ + {"@every 5m", time.Date(2026, time.July, 22, 8, 35, 0, 0, time.UTC)}, + {"@hourly", time.Date(2026, time.July, 22, 9, 0, 0, 0, time.UTC)}, + {"@daily", time.Date(2026, time.July, 23, 0, 0, 0, 0, time.UTC)}, + {"0 */6 * * *", time.Date(2026, time.July, 22, 12, 0, 0, 0, time.UTC)}, + } + for _, tc := range cases { + t.Run(tc.expr, func(t *testing.T) { + got, err := nextCronFire(tc.expr, "", after) + if err != nil { + t.Fatalf("nextCronFire(%q) error: %v", tc.expr, err) + } + if !got.Equal(tc.want) { + t.Fatalf("nextCronFire(%q) = %s, want %s", tc.expr, got.UTC(), tc.want) + } + }) + } +} + +// TestNextCronFire_Timezone pins the timezone bug: "daily at 09:00" in a +// non-UTC zone must resolve to that zone's 09:00 wall-clock, not UTC's. The +// old scheduler ignored ScheduledTriggerConfig.Timezone entirely. +func TestNextCronFire_Timezone(t *testing.T) { + // 08:00 UTC on 2026-07-22. In America/New_York (EDT, UTC-4 in July) that is + // 04:00 local, so the next 09:00 EDT is 13:00 UTC the same day. + after := time.Date(2026, time.July, 22, 8, 0, 0, 0, time.UTC) + got, err := nextCronFire("0 9 * * *", "America/New_York", after) + if err != nil { + t.Fatalf("nextCronFire error: %v", err) + } + want := time.Date(2026, time.July, 22, 13, 0, 0, 0, time.UTC) + if !got.Equal(want) { + t.Fatalf("NY 09:00 in July: got %s, want %s (13:00 UTC)", got.UTC(), want) + } + + // In winter (EST, UTC-5) the same 09:00 local is 14:00 UTC. + afterWinter := time.Date(2026, time.January, 15, 6, 0, 0, 0, time.UTC) + gotWinter, err := nextCronFire("0 9 * * *", "America/New_York", afterWinter) + if err != nil { + t.Fatalf("nextCronFire (winter) error: %v", err) + } + wantWinter := time.Date(2026, time.January, 15, 14, 0, 0, 0, time.UTC) + if !gotWinter.Equal(wantWinter) { + t.Fatalf("NY 09:00 in January: got %s, want %s (14:00 UTC)", gotWinter.UTC(), wantWinter) + } +} + +// TestNextCronFire_DSTSpringForward covers the DST edge: at the US spring +// forward (2026-03-08, 02:00 -> 03:00 local), a 09:00 daily schedule must +// still land on a real 09:00 local instant. The day the clocks jump, 09:00 EDT +// is 13:00 UTC. +func TestNextCronFire_DSTSpringForward(t *testing.T) { + after := time.Date(2026, time.March, 8, 5, 0, 0, 0, time.UTC) // 00:00 EST that morning + got, err := nextCronFire("0 9 * * *", "America/New_York", after) + if err != nil { + t.Fatalf("nextCronFire error: %v", err) + } + want := time.Date(2026, time.March, 8, 13, 0, 0, 0, time.UTC) // 09:00 EDT + if !got.Equal(want) { + t.Fatalf("DST spring-forward 09:00 EDT: got %s, want %s", got.UTC(), want) + } +} + +// TestNextCronFire_DefaultsToUTCRegardlessOfHost pins the non-UTC-host +// requirement: with no timezone configured, the schedule is interpreted in UTC, +// independent of the process's local clock. We assert the computed instant is +// exactly UTC 09:00, which would differ if the host TZ leaked in. +func TestNextCronFire_DefaultsToUTCRegardlessOfHost(t *testing.T) { + after := time.Date(2026, time.July, 22, 8, 0, 0, 0, time.UTC) + got, err := nextCronFire("0 9 * * *", "", after) + if err != nil { + t.Fatalf("nextCronFire error: %v", err) + } + want := time.Date(2026, time.July, 22, 9, 0, 0, 0, time.UTC) + if !got.Equal(want) { + t.Fatalf("empty timezone should mean UTC: got %s, want %s", got.UTC(), want) + } +} + +// TestNextCronFire_InvalidExpression surfaces a clear error instead of silently +// never firing. +func TestNextCronFire_InvalidExpression(t *testing.T) { + if _, err := nextCronFire("not a cron", "", time.Now()); err == nil { + t.Fatal("expected error for invalid cron expression") + } + if _, err := nextCronFire("0 9 * * *", "Mars/Phobos", time.Now()); err == nil { + t.Fatal("expected error for invalid timezone") + } +} + +// TestCronScheduler_PinnedExpression_FiresWhenDueThenWaits is the end-to-end +// regression for the pinned-expression bug through shouldFire: a "daily at +// 09:00 UTC" trigger created at 08:00 is not due at 08:59, is due at 09:00, and +// after firing (LastEvaluatedAt advanced) is not due again until the next day. +func TestCronScheduler_PinnedExpression_FiresWhenDueThenWaits(t *testing.T) { + svc := newTestService(t) + log, _ := logger.NewFromZap(zap.NewNop()) + cs := NewCronScheduler(svc, log) + + created := time.Date(2026, time.July, 22, 8, 0, 0, 0, time.UTC) + cfg, _ := json.Marshal(ScheduledTriggerConfig{CronExpression: "0 9 * * *", Timezone: "UTC"}) + trig := &AutomationTrigger{ + Type: TriggerTypeScheduled, + Config: cfg, + Enabled: true, + CreatedAt: created, + } + + if cs.shouldFire(trig, time.Date(2026, time.July, 22, 8, 59, 0, 0, time.UTC)) { + t.Fatal("pinned 09:00 trigger should not be due at 08:59") + } + if !cs.shouldFire(trig, time.Date(2026, time.July, 22, 9, 0, 30, 0, time.UTC)) { + t.Fatal("pinned 09:00 trigger should be due at 09:00:30") + } + + // Simulate the fire advancing LastEvaluatedAt. + fired := time.Date(2026, time.July, 22, 9, 0, 30, 0, time.UTC) + trig.LastEvaluatedAt = &fired + if cs.shouldFire(trig, time.Date(2026, time.July, 22, 9, 5, 0, 0, time.UTC)) { + t.Fatal("pinned 09:00 trigger must not refire minutes after it fired") + } + if !cs.shouldFire(trig, time.Date(2026, time.July, 23, 9, 0, 5, 0, time.UTC)) { + t.Fatal("pinned 09:00 trigger should be due again the next day at 09:00") + } +} diff --git a/apps/backend/internal/automation/trigger_registry.go b/apps/backend/internal/automation/trigger_registry.go index 221be760ed..3c26497ba8 100644 --- a/apps/backend/internal/automation/trigger_registry.go +++ b/apps/backend/internal/automation/trigger_registry.go @@ -63,11 +63,12 @@ var triggerTypeRegistry = []TriggerTypeInfo{ DefaultConfig: json.RawMessage(`{"events":["opened"],"repos":[],"exclude_draft":false}`), }, { - Type: TriggerTypeGitHubPush, - Label: "Push to branch", - Description: "Triggers when commits are pushed to matching branches", - Category: triggerCategoryGitHub, - Enabled: false, + Type: TriggerTypeGitHubPush, + Label: "Push to branch", + Description: "Webhook-driven: triggers when commits are pushed to matching branches. Requires " + + "a workspace GitHub App connection subscribed to push events.", + Category: triggerCategoryGitHub, + Enabled: true, Placeholders: append([]PlaceholderInfo{ {Key: "push.branch", Description: "Branch that was pushed to", Example: defaultBranchMain}, {Key: "push.repo", Description: placeholderRepositoryOwner, Example: exampleRepositoryOwner}, @@ -79,11 +80,12 @@ var triggerTypeRegistry = []TriggerTypeInfo{ DefaultConfig: json.RawMessage(`{"repos":[],"branches":["main"]}`), }, { - Type: TriggerTypeGitHubCI, - Label: "CI check result", - Description: "Triggers when a CI check completes with matching conclusion", - Category: triggerCategoryGitHub, - Enabled: false, + Type: TriggerTypeGitHubCI, + Label: "CI check result", + Description: "Webhook-driven: triggers when a CI check completes with matching conclusion. Requires " + + "a workspace GitHub App connection subscribed to check_run events.", + Category: triggerCategoryGitHub, + Enabled: true, Placeholders: append([]PlaceholderInfo{ {Key: "ci.check_name", Description: "CI check/workflow name", Example: "build"}, {Key: "ci.conclusion", Description: "CI conclusion", Example: "failure"}, diff --git a/apps/backend/internal/events/types.go b/apps/backend/internal/events/types.go index 24fa2b0139..566a8cd52b 100644 --- a/apps/backend/internal/events/types.go +++ b/apps/backend/internal/events/types.go @@ -260,6 +260,8 @@ const ( GitHubTaskCIOptionsUpdated = "github.task_ci_options.updated" // Task CI automation options updated GitHubWatchEvent = "github.watch.event" // Watch created/deleted GitHubRateLimitUpdated = "github.rate_limit.updated" // GitHub API rate-limit snapshot changed + GitHubPushReceived = "github.push_received" // Push webhook verified + installation resolved to workspaces + GitHubCheckRunCompleted = "github.check_run_completed" // Completed check_run webhook resolved to workspaces ) // Event types for GitLab integration diff --git a/apps/backend/internal/github/app_registration_test_helpers_test.go b/apps/backend/internal/github/app_registration_test_helpers_test.go index 7c1cd0a930..1f82f3ff79 100644 --- a/apps/backend/internal/github/app_registration_test_helpers_test.go +++ b/apps/backend/internal/github/app_registration_test_helpers_test.go @@ -25,6 +25,8 @@ func testManifestConversion(t *testing.T, ownerLogin, ownerType string) Manifest "metadata": "read", "pull_requests": "write", "statuses": "read", "workflows": "write", }, - Events: []string{"installation", "installation_repositories", "github_app_authorization"}, + Events: []string{ + "installation", "installation_repositories", "github_app_authorization", "push", "check_run", + }, } } diff --git a/apps/backend/internal/github/controller_webhook_test.go b/apps/backend/internal/github/controller_webhook_test.go index f9f213077c..998fd399a5 100644 --- a/apps/backend/internal/github/controller_webhook_test.go +++ b/apps/backend/internal/github/controller_webhook_test.go @@ -13,7 +13,7 @@ func TestHTTPGitHubWebhookRoutesByAppRegistration(t *testing.T) { gin.SetMode(gin.TestMode) store := &githubWebhookMemoryStore{} webhooks := NewAppRegistrationWebhookService( - "registration-work", "work-secret", store, nil, nil, + "registration-work", "work-secret", store, nil, nil, nil, ) runtime := &githubAppRuntime{ registrationID: "registration-work", source: DeploymentAppSourceManaged, diff --git a/apps/backend/internal/github/deployment_app_manifest.go b/apps/backend/internal/github/deployment_app_manifest.go index 39fb85940a..940f21788f 100644 --- a/apps/backend/internal/github/deployment_app_manifest.go +++ b/apps/backend/internal/github/deployment_app_manifest.go @@ -228,8 +228,15 @@ func BuildDeploymentAppManifest( "metadata": "read", "pull_requests": "write", "statuses": "read", "workflows": "write", }, + // push and check_run enable webhook-driven github_push/github_ci automation + // triggers. Deployment caveat: GitHub Apps only deliver events a user has + // explicitly subscribed to at installation time, so existing installations + // created before this manifest change must be re-accepted (reinstalled, or + // updated via the App's GitHub settings page) before push/check_run + // webhooks start arriving for them. DefaultEvents: []string{ "installation", "installation_repositories", "github_app_authorization", + "push", "check_run", }, RequestOAuthOnInstall: true, SetupOnUpdate: false, diff --git a/apps/backend/internal/github/deployment_app_manifest_test.go b/apps/backend/internal/github/deployment_app_manifest_test.go index da3f596814..b8882d47d0 100644 --- a/apps/backend/internal/github/deployment_app_manifest_test.go +++ b/apps/backend/internal/github/deployment_app_manifest_test.go @@ -77,7 +77,9 @@ func TestDeploymentAppManifestExactPolicyAndURLs(t *testing.T) { if !reflect.DeepEqual(manifest.DefaultPermissions, wantPermissions) { t.Fatalf("DefaultPermissions = %#v, want %#v", manifest.DefaultPermissions, wantPermissions) } - wantEvents := []string{"installation", "installation_repositories", "github_app_authorization"} + wantEvents := []string{ + "installation", "installation_repositories", "github_app_authorization", "push", "check_run", + } if !reflect.DeepEqual(manifest.DefaultEvents, wantEvents) { t.Fatalf("DefaultEvents = %#v, want %#v", manifest.DefaultEvents, wantEvents) } diff --git a/apps/backend/internal/github/service_app_auth.go b/apps/backend/internal/github/service_app_auth.go index 963bd096f4..5f2149998e 100644 --- a/apps/backend/internal/github/service_app_auth.go +++ b/apps/backend/internal/github/service_app_auth.go @@ -192,7 +192,7 @@ func (s *Service) buildDeploymentAppRuntimeWithClient( personalAuth: personal, webhookAuth: NewAppRegistrationWebhookService( registrationID, config.WebhookSecret, connectionStore, - &installationRepositorySettingsUpdater{service: s}, personalRepo, + &installationRepositorySettingsUpdater{service: s}, personalRepo, s.eventBus, GitHubWebhookReconciliation{Installations: appClient, Personal: personal}, ), }, nil diff --git a/apps/backend/internal/github/webhook_automation_events.go b/apps/backend/internal/github/webhook_automation_events.go new file mode 100644 index 0000000000..f978932cd0 --- /dev/null +++ b/apps/backend/internal/github/webhook_automation_events.go @@ -0,0 +1,27 @@ +package github + +// GitHubPushEventPayload is published on events.GitHubPushReceived after a +// push webhook is verified and its installation resolved to workspaces. +type GitHubPushEventPayload struct { + WorkspaceIDs []string + Owner string + Name string + Branch string // ref with refs/heads/ stripped + SHA string // head commit (payload "after") + PusherLogin string + HeadCommitMsg string // head_commit.message (may be empty for some pushes) +} + +// GitHubCheckRunEventPayload is published on events.GitHubCheckRunCompleted +// for a completed check_run. +type GitHubCheckRunEventPayload struct { + WorkspaceIDs []string + Owner string + Name string // repository name + Branch string // check_run.check_suite.head_branch + SHA string // check_run.head_sha + CheckName string // check_run.name + Conclusion string // check_run.conclusion (success, failure, timed_out, ...) + CheckRunID int64 // check_run.id — for dedup uniqueness + HTMLURL string // check_run.html_url — the check's page on GitHub +} diff --git a/apps/backend/internal/github/webhook_push_ci_test.go b/apps/backend/internal/github/webhook_push_ci_test.go new file mode 100644 index 0000000000..63c662689d --- /dev/null +++ b/apps/backend/internal/github/webhook_push_ci_test.go @@ -0,0 +1,303 @@ +package github + +import ( + "context" + "testing" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" +) + +func newTestMemoryBus(t *testing.T) *bus.MemoryEventBus { + t.Helper() + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + t.Fatal(err) + } + return bus.NewMemoryEventBus(log) +} + +func TestGitHubWebhookPushPublishesPushReceived(t *testing.T) { + installationID := int64(42) + store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{ + installationID: {{ + WorkspaceID: "workspace-1", AppRegistrationID: "registration-work", + Source: ConnectionSourceGitHubAppInstallation, InstallationID: &installationID, + Status: ConnectionStatusActive, + }}, + }} + eventBus := newTestMemoryBus(t) + var received *GitHubPushEventPayload + if _, err := eventBus.Subscribe(events.GitHubPushReceived, func(_ context.Context, e *bus.Event) error { + received = e.Data.(*GitHubPushEventPayload) + return nil + }); err != nil { + t.Fatal(err) + } + service := NewAppRegistrationWebhookService("registration-work", "work-secret", store, nil, nil, eventBus) + payload := []byte(`{ + "ref":"refs/heads/main","after":"abc1234", + "repository":{"name":"repo","owner":{"login":"acme"}}, + "installation":{"id":42},"pusher":{"name":"alice"}, + "head_commit":{"message":"feat: add widget"} + }`) + request := signedWebhookRequest("work-secret", "delivery-push-1", "push", payload) + + result, err := service.Handle(context.Background(), request) + if err != nil { + t.Fatalf("Handle() error = %v", err) + } + if result.Affected != 1 || result.Status != WebhookDeliveryStatusProcessed { + t.Fatalf("result = %+v", result) + } + if received == nil { + t.Fatal("expected push event to be published") + } + if received.WorkspaceIDs[0] != "workspace-1" || received.Owner != "acme" || received.Name != "repo" || + received.Branch != "main" || received.SHA != "abc1234" || received.PusherLogin != "alice" || + received.HeadCommitMsg != "feat: add widget" { + t.Fatalf("received = %+v", received) + } +} + +func TestGitHubWebhookResolvesDistinctActiveWorkspaces(t *testing.T) { + installationID := int64(42) + conn := func(ws string, status ConnectionStatus) *WorkspaceConnection { + return &WorkspaceConnection{ + WorkspaceID: ws, AppRegistrationID: "registration-work", + Source: ConnectionSourceGitHubAppInstallation, InstallationID: &installationID, + Status: status, + } + } + store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{ + installationID: { + conn("workspace-1", ConnectionStatusActive), + conn("workspace-1", ConnectionStatusActive), // duplicate → collapsed + conn("workspace-2", ConnectionStatusActive), // distinct → kept + conn("workspace-3", ConnectionStatusSuspended), // suspended → excluded + conn("workspace-4", ConnectionStatusRevoked), // revoked → excluded + }, + }} + eventBus := newTestMemoryBus(t) + var received *GitHubPushEventPayload + if _, err := eventBus.Subscribe(events.GitHubPushReceived, func(_ context.Context, e *bus.Event) error { + received = e.Data.(*GitHubPushEventPayload) + return nil + }); err != nil { + t.Fatal(err) + } + service := NewAppRegistrationWebhookService("registration-work", "work-secret", store, nil, nil, eventBus) + payload := []byte(`{ + "ref":"refs/heads/main","after":"abc1234", + "repository":{"name":"repo","owner":{"login":"acme"}}, + "installation":{"id":42},"pusher":{"name":"alice"} + }`) + request := signedWebhookRequest("work-secret", "delivery-multi-ws", "push", payload) + + result, err := service.Handle(context.Background(), request) + if err != nil { + t.Fatalf("Handle() error = %v", err) + } + if result.Affected != 2 { + t.Fatalf("expected 2 distinct active workspaces, got affected=%d", result.Affected) + } + if received == nil { + t.Fatal("expected push event to be published") + } + got := map[string]bool{} + for _, ws := range received.WorkspaceIDs { + got[ws] = true + } + if len(received.WorkspaceIDs) != 2 || !got["workspace-1"] || !got["workspace-2"] || + got["workspace-3"] || got["workspace-4"] { + t.Fatalf("expected [workspace-1 workspace-2], got %v", received.WorkspaceIDs) + } +} + +func TestGitHubWebhookPushIgnoresBranchDelete(t *testing.T) { + installationID := int64(42) + store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{ + installationID: {{ + WorkspaceID: "workspace-1", AppRegistrationID: "registration-work", + Source: ConnectionSourceGitHubAppInstallation, InstallationID: &installationID, + Status: ConnectionStatusActive, + }}, + }} + eventBus := newTestMemoryBus(t) + published := false + if _, err := eventBus.Subscribe(events.GitHubPushReceived, func(_ context.Context, _ *bus.Event) error { + published = true + return nil + }); err != nil { + t.Fatal(err) + } + service := NewAppRegistrationWebhookService("registration-work", "work-secret", store, nil, nil, eventBus) + payload := []byte(`{ + "ref":"refs/heads/main","after":"0000000000000000000000000000000000000000","deleted":true, + "repository":{"name":"repo","owner":{"login":"acme"}}, + "installation":{"id":42},"pusher":{"name":"alice"} + }`) + request := signedWebhookRequest("work-secret", "delivery-push-2", "push", payload) + + result, err := service.Handle(context.Background(), request) + if err != nil { + t.Fatalf("Handle() error = %v", err) + } + if result.Affected != 0 || result.Status != WebhookDeliveryStatusIgnored { + t.Fatalf("result = %+v", result) + } + if published { + t.Fatal("expected branch-delete push not to publish an event") + } +} + +func TestGitHubWebhookPushIgnoresNonBranchRef(t *testing.T) { + installationID := int64(42) + store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{ + installationID: {{ + WorkspaceID: "workspace-1", AppRegistrationID: "registration-work", + Source: ConnectionSourceGitHubAppInstallation, InstallationID: &installationID, + Status: ConnectionStatusActive, + }}, + }} + eventBus := newTestMemoryBus(t) + published := false + if _, err := eventBus.Subscribe(events.GitHubPushReceived, func(_ context.Context, _ *bus.Event) error { + published = true + return nil + }); err != nil { + t.Fatal(err) + } + service := NewAppRegistrationWebhookService("registration-work", "work-secret", store, nil, nil, eventBus) + payload := []byte(`{ + "ref":"refs/tags/v1.0.0","after":"abc1234", + "repository":{"name":"repo","owner":{"login":"acme"}}, + "installation":{"id":42},"pusher":{"name":"alice"} + }`) + request := signedWebhookRequest("work-secret", "delivery-push-3", "push", payload) + + if _, err := service.Handle(context.Background(), request); err != nil { + t.Fatalf("Handle() error = %v", err) + } + if published { + t.Fatal("expected tag push not to publish an event") + } +} + +func TestGitHubWebhookCheckRunPublishesCompletedEvent(t *testing.T) { + installationID := int64(42) + store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{ + installationID: {{ + WorkspaceID: "workspace-1", AppRegistrationID: "registration-work", + Source: ConnectionSourceGitHubAppInstallation, InstallationID: &installationID, + Status: ConnectionStatusActive, + }}, + }} + eventBus := newTestMemoryBus(t) + var received *GitHubCheckRunEventPayload + if _, err := eventBus.Subscribe(events.GitHubCheckRunCompleted, func(_ context.Context, e *bus.Event) error { + received = e.Data.(*GitHubCheckRunEventPayload) + return nil + }); err != nil { + t.Fatal(err) + } + service := NewAppRegistrationWebhookService("registration-work", "work-secret", store, nil, nil, eventBus) + payload := []byte(`{ + "action":"completed", + "check_run":{ + "id":555,"name":"build","head_sha":"abc1234","conclusion":"failure", + "html_url":"https://github.com/acme/repo/runs/555", + "check_suite":{"head_branch":"main"} + }, + "repository":{"name":"repo","owner":{"login":"acme"}}, + "installation":{"id":42} + }`) + request := signedWebhookRequest("work-secret", "delivery-ci-1", "check_run", payload) + + result, err := service.Handle(context.Background(), request) + if err != nil { + t.Fatalf("Handle() error = %v", err) + } + if result.Affected != 1 || result.Status != WebhookDeliveryStatusProcessed { + t.Fatalf("result = %+v", result) + } + if received == nil { + t.Fatal("expected check_run event to be published") + } + if received.WorkspaceIDs[0] != "workspace-1" || received.Owner != "acme" || received.Name != "repo" || + received.Branch != "main" || received.SHA != "abc1234" || received.CheckName != "build" || + received.Conclusion != "failure" || received.CheckRunID != 555 || + received.HTMLURL != "https://github.com/acme/repo/runs/555" { + t.Fatalf("received = %+v", received) + } +} + +func TestGitHubWebhookCheckRunIgnoresNonCompletedAction(t *testing.T) { + installationID := int64(42) + store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{ + installationID: {{ + WorkspaceID: "workspace-1", AppRegistrationID: "registration-work", + Source: ConnectionSourceGitHubAppInstallation, InstallationID: &installationID, + Status: ConnectionStatusActive, + }}, + }} + eventBus := newTestMemoryBus(t) + published := false + if _, err := eventBus.Subscribe(events.GitHubCheckRunCompleted, func(_ context.Context, _ *bus.Event) error { + published = true + return nil + }); err != nil { + t.Fatal(err) + } + service := NewAppRegistrationWebhookService("registration-work", "work-secret", store, nil, nil, eventBus) + payload := []byte(`{ + "action":"in_progress", + "check_run":{ + "id":555,"name":"build","head_sha":"abc1234","conclusion":"", + "check_suite":{"head_branch":"main"} + }, + "repository":{"name":"repo","owner":{"login":"acme"}}, + "installation":{"id":42} + }`) + request := signedWebhookRequest("work-secret", "delivery-ci-2", "check_run", payload) + + result, err := service.Handle(context.Background(), request) + if err != nil { + t.Fatalf("Handle() error = %v", err) + } + if result.Affected != 0 || result.Status != WebhookDeliveryStatusIgnored { + t.Fatalf("result = %+v", result) + } + if published { + t.Fatal("expected non-completed check_run not to publish an event") + } +} + +func TestGitHubWebhookPublishNilPublisherIsNoOp(t *testing.T) { + installationID := int64(42) + store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{ + installationID: {{ + WorkspaceID: "workspace-1", AppRegistrationID: "registration-work", + Source: ConnectionSourceGitHubAppInstallation, InstallationID: &installationID, + Status: ConnectionStatusActive, + }}, + }} + service := NewAppRegistrationWebhookService("registration-work", "work-secret", store, nil, nil, nil) + payload := []byte(`{ + "ref":"refs/heads/main","after":"abc1234", + "repository":{"name":"repo","owner":{"login":"acme"}}, + "installation":{"id":42},"pusher":{"name":"alice"} + }`) + request := signedWebhookRequest("work-secret", "delivery-push-nil", "push", payload) + + result, err := service.Handle(context.Background(), request) + if err != nil { + t.Fatalf("Handle() error = %v", err) + } + if result.Affected != 1 || result.Status != WebhookDeliveryStatusProcessed { + t.Fatalf("result = %+v", result) + } +} diff --git a/apps/backend/internal/github/webhook_service.go b/apps/backend/internal/github/webhook_service.go index 9f3a34cc9a..3992f8e4ab 100644 --- a/apps/backend/internal/github/webhook_service.go +++ b/apps/backend/internal/github/webhook_service.go @@ -12,6 +12,9 @@ import ( "time" "go.uber.org/zap" + + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" ) const ( @@ -142,6 +145,7 @@ type GitHubWebhookService struct { personal personalConnectionRevoker installations appInstallationVerifier personalReconciler personalAuthorizationReconciler + publisher bus.EventBus now func() time.Time } @@ -150,10 +154,12 @@ func NewGitHubWebhookService( store githubWebhookStore, repositories installationRepositoryUpdater, personal personalConnectionRevoker, + publisher bus.EventBus, reconciliation ...GitHubWebhookReconciliation, ) *GitHubWebhookService { service := &GitHubWebhookService{ - secret: []byte(secret), store: store, repos: repositories, personal: personal, now: time.Now, + secret: []byte(secret), store: store, repos: repositories, personal: personal, + publisher: publisher, now: time.Now, } if len(reconciliation) > 0 { service.installations = reconciliation[0].Installations @@ -167,9 +173,10 @@ func NewAppRegistrationWebhookService( store githubWebhookStore, repositories installationRepositoryUpdater, personal personalConnectionRevoker, + publisher bus.EventBus, reconciliation ...GitHubWebhookReconciliation, ) *GitHubWebhookService { - service := NewGitHubWebhookService(secret, store, repositories, personal, reconciliation...) + service := NewGitHubWebhookService(secret, store, repositories, personal, publisher, reconciliation...) service.registrationID = strings.TrimSpace(registrationID) return service } @@ -244,6 +251,10 @@ func (s *GitHubWebhookService) process(ctx context.Context, event string, payloa return s.processInstallationRepositories(ctx, payload) case "github_app_authorization": return s.processAuthorization(ctx, payload) + case "push": + return s.processPush(ctx, payload) + case "check_run": + return s.processCheckRun(ctx, payload) default: return 0, "event ignored", nil } @@ -450,6 +461,161 @@ func (s *GitHubWebhookService) processAuthorization( return affected, fmt.Sprintf("authorization revoked: %d binding(s)", affected), nil } +type pushWebhookEvent struct { + Ref string `json:"ref"` + After string `json:"after"` + Deleted bool `json:"deleted"` + Repository struct { + Name string `json:"name"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + } `json:"repository"` + Installation struct { + ID int64 `json:"id"` + } `json:"installation"` + Pusher struct { + Name string `json:"name"` + } `json:"pusher"` + HeadCommit struct { + Message string `json:"message"` + } `json:"head_commit"` +} + +const gitBranchRefPrefix = "refs/heads/" + +func isZeroSHA(sha string) bool { + if sha == "" { + return true + } + for _, c := range sha { + if c != '0' { + return false + } + } + return true +} + +// processPush handles the "push" webhook, publishing events.GitHubPushReceived +// with the resolved workspace IDs when the push targets a branch ref and +// isn't a branch delete. +func (s *GitHubWebhookService) processPush(ctx context.Context, payload []byte) (int, string, error) { + var event pushWebhookEvent + if err := json.Unmarshal(payload, &event); err != nil { + return 0, "", fmt.Errorf("decode push webhook: %w", err) + } + if event.Deleted || isZeroSHA(event.After) || !strings.HasPrefix(event.Ref, gitBranchRefPrefix) { + return 0, "push ignored", nil + } + workspaceIDs, err := s.resolveWorkspaceIDs(ctx, event.Installation.ID) + if err != nil { + return 0, "", err + } + if len(workspaceIDs) == 0 { + return 0, "push ignored", nil + } + s.publishEvent(ctx, events.GitHubPushReceived, &GitHubPushEventPayload{ + WorkspaceIDs: workspaceIDs, + Owner: event.Repository.Owner.Login, + Name: event.Repository.Name, + Branch: strings.TrimPrefix(event.Ref, gitBranchRefPrefix), + SHA: event.After, + PusherLogin: event.Pusher.Name, + HeadCommitMsg: event.HeadCommit.Message, + }) + return len(workspaceIDs), fmt.Sprintf("push published: %d workspace(s)", len(workspaceIDs)), nil +} + +type checkRunWebhookEvent struct { + Action string `json:"action"` + CheckRun struct { + ID int64 `json:"id"` + Name string `json:"name"` + HeadSHA string `json:"head_sha"` + Conclusion string `json:"conclusion"` + HTMLURL string `json:"html_url"` + CheckSuite struct { + HeadBranch string `json:"head_branch"` + } `json:"check_suite"` + } `json:"check_run"` + Repository struct { + Name string `json:"name"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + } `json:"repository"` + Installation struct { + ID int64 `json:"id"` + } `json:"installation"` +} + +// processCheckRun handles the "check_run" webhook, publishing +// events.GitHubCheckRunCompleted with the resolved workspace IDs for +// completed check runs only. +func (s *GitHubWebhookService) processCheckRun(ctx context.Context, payload []byte) (int, string, error) { + var event checkRunWebhookEvent + if err := json.Unmarshal(payload, &event); err != nil { + return 0, "", fmt.Errorf("decode check_run webhook: %w", err) + } + if event.Action != "completed" { + return 0, "check_run ignored", nil + } + workspaceIDs, err := s.resolveWorkspaceIDs(ctx, event.Installation.ID) + if err != nil { + return 0, "", err + } + if len(workspaceIDs) == 0 { + return 0, "check_run ignored", nil + } + s.publishEvent(ctx, events.GitHubCheckRunCompleted, &GitHubCheckRunEventPayload{ + WorkspaceIDs: workspaceIDs, + Owner: event.Repository.Owner.Login, + Name: event.Repository.Name, + Branch: event.CheckRun.CheckSuite.HeadBranch, + SHA: event.CheckRun.HeadSHA, + CheckName: event.CheckRun.Name, + Conclusion: event.CheckRun.Conclusion, + CheckRunID: event.CheckRun.ID, + HTMLURL: event.CheckRun.HTMLURL, + }) + return len(workspaceIDs), fmt.Sprintf("check_run published: %d workspace(s)", len(workspaceIDs)), nil +} + +// resolveWorkspaceIDs resolves an installation ID to the distinct set of +// workspace IDs connected to it under this registration. +func (s *GitHubWebhookService) resolveWorkspaceIDs(ctx context.Context, installationID int64) ([]string, error) { + if installationID <= 0 { + return nil, nil + } + connections, err := s.listWorkspaceConnections(ctx, installationID) + if err != nil { + return nil, fmt.Errorf("load installation bindings: %w", err) + } + seen := make(map[string]bool, len(connections)) + workspaceIDs := make([]string, 0, len(connections)) + for _, connection := range connections { + // Only active installations may drive automation events: a suspended + // or revoked App binding must not still fire push/CI automations. + if connection == nil || connection.WorkspaceID == "" || + connection.Status != ConnectionStatusActive || seen[connection.WorkspaceID] { + continue + } + seen[connection.WorkspaceID] = true + workspaceIDs = append(workspaceIDs, connection.WorkspaceID) + } + return workspaceIDs, nil +} + +// publishEvent is nil-safe and never fails webhook delivery: publish errors +// are swallowed so a bus outage doesn't turn a real webhook into a failed +// delivery. +func (s *GitHubWebhookService) publishEvent(ctx context.Context, eventType string, data interface{}) { + if s.publisher == nil { + return + } + _ = s.publisher.Publish(ctx, eventType, bus.NewEvent(eventType, "github_webhook_service", data)) +} + func (s *GitHubWebhookService) reconcileInstallation( ctx context.Context, installationID int64, diff --git a/apps/backend/internal/github/webhook_service_test.go b/apps/backend/internal/github/webhook_service_test.go index 1a2b13e7a4..0c4db56199 100644 --- a/apps/backend/internal/github/webhook_service_test.go +++ b/apps/backend/internal/github/webhook_service_test.go @@ -188,7 +188,7 @@ func newRegistrationWebhookService( reconciliation ...GitHubWebhookReconciliation, ) *GitHubWebhookService { return NewAppRegistrationWebhookService( - registrationID, secret, store, repositories, personal, reconciliation..., + registrationID, secret, store, repositories, personal, nil, reconciliation..., ) } @@ -342,7 +342,7 @@ func TestGitHubWebhookConcurrentDeliveryIsProcessedOncePerRegistration(t *testin t.Fatal(err) } service := NewAppRegistrationWebhookService( - "registration-work", "work-secret", store, nil, nil, + "registration-work", "work-secret", store, nil, nil, nil, ) request := signedWebhookRequest( "work-secret", "delivery-concurrent", "installation", @@ -391,7 +391,7 @@ func TestGitHubWebhookDeduplicatesInstallationTransition(t *testing.T) { InstallationID: &installationID, Status: ConnectionStatusActive, CredentialGeneration: 2, } store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{42: {connection}}} - service := NewGitHubWebhookService("webhook-secret", store, nil, nil) + service := NewGitHubWebhookService("webhook-secret", store, nil, nil, nil) payload := []byte(`{"action":"suspend","installation":{"id":42}}`) request := signedWebhookRequest("webhook-secret", "delivery-1", "installation", payload) @@ -416,7 +416,7 @@ func TestGitHubWebhookUnsuspendsAndDeletesKnownInstallation(t *testing.T) { InstallationID: &installationID, Status: ConnectionStatusSuspended, CredentialGeneration: 3, } store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{42: {connection}}} - service := NewGitHubWebhookService("webhook-secret", store, nil, nil) + service := NewGitHubWebhookService("webhook-secret", store, nil, nil, nil) unsuspend := []byte(`{"action":"unsuspend","installation":{"id":42}}`) if _, err := service.Handle( @@ -446,7 +446,7 @@ func TestGitHubWebhookLateUnsuspendCannotReactivateDeletedInstallation(t *testin InstallationID: &installationID, Status: ConnectionStatusRevoked, CredentialGeneration: 5, } store := &githubWebhookMemoryStore{workspaces: map[int64][]*WorkspaceConnection{42: {connection}}} - service := NewGitHubWebhookService("webhook-secret", store, nil, nil) + service := NewGitHubWebhookService("webhook-secret", store, nil, nil, nil) payload := []byte(`{"action":"unsuspend","installation":{"id":42}}`) result, err := service.Handle( context.Background(), signedWebhookRequest("webhook-secret", "delivery-late", "installation", payload), @@ -472,7 +472,7 @@ func TestGitHubWebhookReconcilesOutOfOrderInstallationEventsWithProvider(t *test ID: 42, AccountLogin: "acme", AccountType: "Organization", }} service := NewGitHubWebhookService( - "webhook-secret", store, nil, nil, + "webhook-secret", store, nil, nil, nil, GitHubWebhookReconciliation{Installations: verifier}, ) @@ -496,7 +496,7 @@ func TestGitHubWebhookReconcilesOutOfOrderInstallationEventsWithProvider(t *test func TestGitHubWebhookUnknownInstallationDoesNotCreateBinding(t *testing.T) { store := &githubWebhookMemoryStore{} - service := NewGitHubWebhookService("webhook-secret", store, nil, nil) + service := NewGitHubWebhookService("webhook-secret", store, nil, nil, nil) payload := []byte(`{"action":"created","installation":{"id":999}}`) result, err := service.Handle( context.Background(), signedWebhookRequest("webhook-secret", "delivery-2", "installation", payload), @@ -522,7 +522,7 @@ func TestGitHubWebhookStaleInstallationRowCannotReplacePAT(t *testing.T) { connection.Login = "octocat" connection.CredentialGeneration = 3 } - service := NewGitHubWebhookService("webhook-secret", store, nil, nil) + service := NewGitHubWebhookService("webhook-secret", store, nil, nil, nil) payload := []byte(`{"action":"suspend","installation":{"id":42}}`) result, err := service.Handle( @@ -609,7 +609,7 @@ func TestGitHubWebhookAuthorizationRevocationRemovesExactUsers(t *testing.T) { {WorkspaceID: "workspace-2", UserID: "user-9", GitHubUserID: 11}, }}} revoker := &webhookPersonalRevoker{} - service := NewGitHubWebhookService("webhook-secret", store, nil, revoker) + service := NewGitHubWebhookService("webhook-secret", store, nil, revoker, nil) payload := []byte(`{"action":"revoked","sender":{"id":11,"login":"octocat"}}`) result, err := service.Handle( context.Background(), signedWebhookRequest("webhook-secret", "delivery-4", "github_app_authorization", payload), @@ -629,7 +629,7 @@ func TestGitHubWebhookLateAuthorizationRevokePreservesVerifiedReconnect(t *testi }}}} reconciler := &webhookPersonalReconciler{} service := NewGitHubWebhookService( - "webhook-secret", store, nil, nil, + "webhook-secret", store, nil, nil, nil, GitHubWebhookReconciliation{Personal: reconciler}, ) payload := []byte(`{"action":"revoked","sender":{"id":11}}`) @@ -651,7 +651,7 @@ func TestGitHubWebhookRetriesFailedDelivery(t *testing.T) { workspaces: map[int64][]*WorkspaceConnection{42: {connection}}, } repos := &webhookRepoUpdater{err: errors.New("temporary database failure")} - service := NewGitHubWebhookService("webhook-secret", store, repos, nil) + service := NewGitHubWebhookService("webhook-secret", store, repos, nil, nil) payload := []byte(`{ "action":"added","installation":{"id":42}, "repositories_added":[{"id":7,"full_name":"acme/repo"}],"repositories_removed":[] @@ -685,7 +685,7 @@ func TestGitHubWebhookReclaimsStaleReceivedDelivery(t *testing.T) { ReceivedAt: now.Add(-webhookDeliveryStaleAfter - time.Second), }, }} - service := NewGitHubWebhookService("webhook-secret", store, nil, nil) + service := NewGitHubWebhookService("webhook-secret", store, nil, nil, nil) service.now = func() time.Time { return now } payload := []byte(`{"action":"created","installation":{"id":999}}`) result, err := service.Handle( diff --git a/docs/public/automation-and-mcp.md b/docs/public/automation-and-mcp.md index 8df33ce17b..c4a31bc856 100644 --- a/docs/public/automation-and-mcp.md +++ b/docs/public/automation-and-mcp.md @@ -7,14 +7,14 @@ description: "Create scheduled, GitHub, or webhook automations and connect agent Kandev has several mechanisms that can act without repeated manual setup. Their scopes and trust boundaries differ: -| Mechanism | Purpose | -|---|---| -| Workflow events and actions | React to an existing task entering a step, receiving a message, or completing an agent turn. | -| Workspace automations | Create task-backed work from a schedule, GitHub pull request, webhook, or manual trigger. | -| Task MCP | Give an active Kandev session task, plan, conversation, and coordination tools. | -| Office MCP and runtime CLI | Give an Office run a restricted coordination surface and permission-checked commands for Office state changes. | -| Profile MCP | Add third-party MCP servers to one agent profile, subject to executor policy. | -| External MCP | Let a client outside a task configure Kandev and create or manage work through the backend. | +| Mechanism | Purpose | +| --------------------------- | -------------------------------------------------------------------------------------------------------------- | +| Workflow events and actions | React to an existing task entering a step, receiving a message, or completing an agent turn. | +| Workspace automations | Create task-backed work from a schedule, GitHub pull request, webhook, or manual trigger. | +| Task MCP | Give an active Kandev session task, plan, conversation, and coordination tools. | +| Office MCP and runtime CLI | Give an Office run a restricted coordination surface and permission-checked commands for Office state changes. | +| Profile MCP | Add third-party MCP servers to one agent profile, subject to executor policy. | +| External MCP | Let a client outside a task configure Kandev and create or manage work through the backend. | Use workflow events for predictable transitions on existing work. Use a workspace automation when an external signal must create new work. MCP is a tool interface, not a scheduler. @@ -63,7 +63,7 @@ In the current backend, the schedule and GitHub PR condition are independent tri ### Schedule -The scheduler checks every 30 seconds and treats expressions as elapsed intervals, not calendar cron times. If a new enabled schedule has never been evaluated, its first check fires immediately; later runs are spaced from the last evaluation. +The scheduler checks every 30 seconds and computes each expression's next calendar fire time in its configured timezone. A schedule created part-way through the day first fires at its next scheduled occurrence after creation (not immediately). A schedule missed while the backend was stopped fires once on the next check rather than once per missed occurrence. Workspaces > _workspace_ > GitHub) whose installation is subscribed to the `push` and `check_run` events. When the App delivers a matching, HMAC-verified webhook, the installation is resolved to its workspace and the matching automation fires. + +- **Push**: fires when commits are pushed to a matching branch. Configure an explicit repository list and optional branch glob filters (`main`, `release/*`). Branch deletions are ignored. Deduplicated per repository by branch and pushed commit SHA. +- **CI check**: fires when a check run completes. Configure an explicit repository list, the conclusions to match (defaults to `failure`, which drives auto-fix-CI flows), and optional check-name and head-branch filters. Deduplicated per repository by check-run ID. + +Because GitHub Apps only deliver events a user subscribed to at installation time, an App connection created before push/CI support was added must be reinstalled (or updated from its GitHub settings page) before these webhooks begin arriving. This generic CI trigger is distinct from the task-specific PR check remediation described under review features. ### Webhook @@ -138,15 +147,15 @@ Task tools use normal client discovery. When `step_complete_kandev` is required Task mode currently registers these tool groups: -| Group | Available operations | -|---|---| -| Board lookups and task lifecycle | List workspaces, workflows, workflow steps, tasks, agents, and executor profiles; create, update, move, archive, or delete tasks; halt all live work on a direct child. This mode does not mutate workflows, profiles, or executors. | -| Coordination | Message a task or targeted session, spawn a named session on the current or another same-workspace task, and read task conversation. See [Agent Communication](agent-communication.md) for delivery semantics, bidirectional reply patterns, and a worked example. | -| User interaction | Ask a structured question when the current agent/session supports it. | -| Plans | Create, get, update, and delete the current task plan. | -| Walkthroughs | Show, get, and delete the task's code walkthrough. | -| Relationships and workspace sources | List related tasks, add a mixed repository/folder source batch to an idle task, use the legacy one-branch tool, and change a repository's diff base. | -| Workflow signal | Signal step completion when an auto-advance step explicitly requires that signal. | +| Group | Available operations | +| ----------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Board lookups and task lifecycle | List workspaces, workflows, workflow steps, tasks, agents, and executor profiles; create, update, move, archive, or delete tasks; halt all live work on a direct child. This mode does not mutate workflows, profiles, or executors. | +| Coordination | Message a task or targeted session, spawn a named session on the current or another same-workspace task, and read task conversation. See [Agent Communication](agent-communication.md) for delivery semantics, bidirectional reply patterns, and a worked example. | +| User interaction | Ask a structured question when the current agent/session supports it. | +| Plans | Create, get, update, and delete the current task plan. | +| Walkthroughs | Show, get, and delete the task's code walkthrough. | +| Relationships and workspace sources | List related tasks, add a mixed repository/folder source batch to an idle task, use the legacy one-branch tool, and change a repository's diff base. | +| Workflow signal | Signal step completion when an auto-advance step explicitly requires that signal. | Task identity is injected for operations that require it. Workspace, parent/subtask, executor, and task-state rules still apply. @@ -251,7 +260,8 @@ The backend's `/mcp`, `/mcp/sse`, and `/mcp/message` routes currently have no Ka ## Troubleshooting -- **No scheduled run:** use a preset or `@every`; fixed calendar cron and timezone scheduling are not implemented. +- **No scheduled run:** confirm the cron expression is valid five-field or `@`-shorthand text and the automation/trigger is enabled; a schedule fires at its next occurrence after creation, not immediately. +- **No GitHub push or CI runs:** connect a workspace GitHub App whose installation is subscribed to `push`/`check_run` events, reinstall a pre-existing App so it receives them, and select explicit repositories (an empty repository list is not evaluated). - **Scheduled run happened as well as a PR run:** clear the schedule expression. The two stored triggers fire independently. - **No GitHub PR runs:** connect GitHub and select explicit repositories; **All repos** currently evaluates none. - **Run fails before a task starts:** select valid non-passthrough agent and non-local executor profiles, and add/select a repository. diff --git a/docs/public/feature-status.md b/docs/public/feature-status.md index 9137193e79..c43e5e03c8 100644 --- a/docs/public/feature-status.md +++ b/docs/public/feature-status.md @@ -72,7 +72,7 @@ Released builds can lag this page. Check **Settings > System > About** or `kande | Local Docker and Sprites executors | Dependency-bound | Require the relevant daemon or provider, compatible host/image, token, platform, and credential-copy configuration. | | SSH executor | Limited | Remote sessions support trusted Linux or macOS hosts on amd64 or arm64, with public-key authentication, SFTP, TCP forwarding, and repository-source materialization. It ignores profile prepare/cleanup scripts and retains the remote task directory for manual cleanup. | | Remote Docker executor | In progress | The type and legacy/direct settings fields are registered, but runtime create/stop return `not yet implemented`, and the current **Settings > Executors** hub does not offer it. | -| GitHub integration | Dependency-bound | Each workspace selects one PAT, named `gh` account, or App registration plus installation for automation and may add a personal App identity. Registrations can be reused deliberately or separated for independent root credentials and bot identities. Managed task access uses repository-bound broker leases; App tokens are minted for that repository, while redeemed PAT/CLI bearer tokens retain their provider scopes. Explicit profile `GITHUB_TOKEN`/`GH_TOKEN` values remain unmanaged overrides. | +| GitHub integration | Dependency-bound | Each workspace selects one PAT, named `gh` account, or App registration plus installation for automation and may add a personal App identity. Registrations can be reused deliberately or separated for independent root credentials and bot identities. Managed task access uses repository-bound broker leases; App tokens are minted for that repository, while redeemed PAT/CLI bearer tokens retain their provider scopes. Explicit profile `GITHUB_TOKEN`/`GH_TOKEN` values remain unmanaged overrides. | | Azure DevOps integration | Limited | Azure DevOps Services supports workspace PAT configuration, read-only work-item and pull-request browsing, feedback, task launch, and durable task PR associations. Azure DevOps Server/TFS, Entra OAuth, writes, watches, and webhooks are not supported; neither `gh` nor `az` is required. | | GitLab integration | Limited | Each workspace can connect one `gitlab.com` or self-managed origin, browse and launch from issues/MRs, link tasks to MRs, run review/issue watches, review and act on linked MRs, assign reviewers, toggle MR notifications, and create/auto-link MRs. Provider permissions and exact host/project identity apply. Browse project narrowing is current-page-only, watches inspect at most their first 50 matches, GitLab has no team review-request equivalent, and issue notification subscription has no UI control. | | Jira integration | Limited | Search/browse, supported transitions, launch presets, task launch, and watches require provider credentials. Launch copies data rather than creating a durable association; each watch poll reads only the first 50 JQL matches. | @@ -80,9 +80,9 @@ Released builds can lag this page. Check **Settings > System > About** or `kande | Sentry integration | Limited | Multi-instance configuration, issue watches, and current-task issue actions are shipped; there is no top-level browser. Watches read only the newest first page, and the status picker allows multiple selections while the backend accepts at most one. | | Slack integration | Experimental | Uses a nonstandard browser-session token/cookie and polling rather than OAuth or a bot installation. A first scan can process up to 30 existing matches, and reactions/replies are best-effort. | | Integration **Enabled** switches | Limited | Jira/Linear/Sentry/Slack switches are browser-local presentation state; they do not stop backend pollers. Pause/delete watches or remove the Slack/provider configuration when polling must stop. | -| Manual, preset/`@every` schedule, authenticated webhook, and GitHub PR-open automations | Limited | Task and Run modes are shipped, but Run cannot answer permission prompts. PR triggers need explicit repositories and ignore the saved Opened checkbox. Webhooks silently retain only the first 1 MiB. **Play** remains clickable while disabled but produces no history. | -| Fixed cron timezone behavior | Limited | The UI accepts fixed cron and timezone values, but the current interval scheduler does not honor the configured timezone and some fixed cron expressions may never fire. Prefer tested presets or `@every` intervals. | -| GitHub push and CI automation triggers | In progress | Trigger types and UI labels exist as coming-soon/stub paths; they are not working production triggers. GitHub's separate opt-in PR CI-fix automation is shipped. | +| Manual, cron/`@every` schedule, authenticated webhook, and GitHub PR-open automations | Limited | Task and Run modes are shipped, but Run cannot answer permission prompts. PR triggers need explicit repositories and ignore the saved Opened checkbox. Webhooks silently retain only the first 1 MiB. **Play** remains clickable while disabled but produces no history. | +| Calendar cron and timezone scheduling | Supported | Fixed calendar cron expressions (pinned minutes/hours, weekday ranges, day-of-month), named descriptors, and `@every` shorthand all fire at their correct next occurrence. The configured timezone is honored including its daylight-saving transitions; an empty timezone means UTC, so schedules are host-clock independent. The scheduler checks every 30 seconds and deduplicates per trigger per minute. | +| GitHub push and CI automation triggers | Dependency-bound | Push and CI-check triggers are webhook-driven end to end (HMAC-verified GitHub App webhook → installation-to-workspace resolution → automation run). They require a workspace GitHub App connection whose installation is subscribed to the `push`/`check_run` events; an App created before this support existed must be reinstalled to receive them. CI matching defaults to the `failure` conclusion, enabling auto-fix-CI flows. GitHub's separate opt-in PR CI-fix automation remains shipped. | | Automation Run mode | Limited | Creates a hidden ephemeral task and starts immediately. It cannot require user input. Automatic worktree/branch reaping applies only when a Worktree runtime ID is present; other executors follow their own lifecycle. | ## MCP, clients, and operation