37b56ba11c
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
304 lines
11 KiB
Go
304 lines
11 KiB
Go
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)
|
|
}
|
|
}
|