From eb736b2af19cfc564d9e1046d1b35d59e59554cb Mon Sep 17 00:00:00 2001 From: Amp Date: Wed, 19 Aug 2026 23:56:06 +0000 Subject: [PATCH] Model first-party PR synchronization builds Amp-Thread-ID: https://ampcode.com/threads/T-01a01c65-f65e-7751-b99c-7c9a8c43cfb0 Co-authored-by: Samuel Cochran --- docs/cli.md | 4 +- docs/compatibility.md | 6 +- docs/security.md | 2 + internal/buildkite/triggers.go | 9 ++- internal/buildkite/triggers_test.go | 17 ++++++ internal/cli/buildkite_event.go | 57 ++++++++++++------ internal/cli/buildkite_event_test.go | 44 ++++++++------ internal/cli/effective_event.go | 25 ++++++-- internal/cli/effective_event_test.go | 27 +++++++++ internal/cli/path_filters_test.go | 18 ++++-- internal/cli/plugin_test.go | 88 ++++++++++++++++++++++++++++ 11 files changed, 245 insertions(+), 52 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 261ae586..38aba80b 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -239,11 +239,11 @@ Event source precedence is: 1. `buildkite:webhook` metadata reserved by Buildkite 1. A reduced snapshot derived from `BUILDKITE_*` variables when no linked webhook is available -An explicit event path never reads Buildkite metadata. Webhook metadata must be one valid JSON object no larger than 25 MiB. Malformed, unreadable, or oversized data stops upload rather than falling back. The Buildkite repository mapping, commit, and ref remain authoritative for the workload. +An explicit event path never reads Buildkite metadata. Webhook metadata must be one valid JSON object no larger than 25 MiB. Malformed, unreadable, or oversized data stops upload rather than falling back. The Buildkite repository mapping and commit remain authoritative for the workload. Trusted Buildkite branch, tag, or pull request fields determine the event ref. Raw webhook data is not retained in generated plans or pipeline YAML and cannot grant queues, secrets, or tokens. -The selected snapshot establishes one effective GitHub event for applicability, compilation, group conditions, and event-qualified check names; group labels remain static across events. An explicit event path uses its event directly and never re-reads live Buildkite event fields. Linked webhook metadata supplies the GitHub event name, including `merge_group` and `release`. Merge queue builds require matching Buildkite head and base refs and commits. Release builds require matching webhook and Buildkite activities, a valid release payload, and a tag matching both `BUILDKITE_TAG` and `BUILDKITE_BRANCH`. The plugin resolves Buildkite's symbolic release commit from the checked-out `HEAD` before constructing the event. Without linked metadata, the environment fallback preserves `push`, `pull_request`, `workflow_dispatch`, and `schedule` from `BUILDKITE_GITHUB_EVENT` across rebuilds. Otherwise, pull request builds map to `pull_request`; Buildkite `ui` and `api` sources map to `workflow_dispatch`; `schedule` maps to `schedule`; and other sources, including tag builds and `trigger_job`, map to `push`. +The selected snapshot establishes one effective GitHub event for applicability, compilation, group conditions, and event-qualified check names; group labels remain static across events. An explicit event path uses its event directly and never re-reads live Buildkite event fields. Linked webhook metadata supplies the GitHub event name, including `merge_group` and `release`. Buildkite's coalesced first-party pull request push maps to the `pull_request` synchronization described in [Names and triggers](compatibility.md#names-and-triggers). Merge queue builds require matching Buildkite head and base refs and commits. Release builds require matching webhook and Buildkite activities, a valid release payload, and a tag matching both `BUILDKITE_TAG` and `BUILDKITE_BRANCH`. The plugin resolves Buildkite's symbolic release commit from the checked-out `HEAD` before constructing the event. Without linked metadata, the environment fallback preserves the same mapping across rebuilds. Otherwise, pull request builds map to `pull_request`; Buildkite `ui` and `api` sources map to `workflow_dispatch`; `schedule` maps to `schedule`; and other sources, including tag builds and `trigger_job`, map to `push`. Top-level workflows that do not declare the effective event are excluded before event-dependent validation and compilation, then emitted as top-level skipped command steps with no plan artifacts. Reusable-only workflows remain available to local callers. If no directly runnable workflow applies, upload succeeds with a skipped-only pipeline. For applicable workflows, only the selected event contributes a group condition. Event conditions match the `BUILDKITE_GITHUB_EVENT` build environment variable. When it is absent, push, pull request, manual/API, and schedule conditions use the same Buildkite source fallback as event selection. Push branch/tag/path filters, pull request base-branch/activity/path filters, merge group base-branch/activity filters, and release activity filters add their supported constraints. Cross-event trigger conditions are never ORed into that group. diff --git a/docs/compatibility.md b/docs/compatibility.md index f2921efb..b5cc260b 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -116,7 +116,9 @@ Upload selects one authoritative effective event, in this order: 1. The GitHub event name accompanying Buildkite's reserved linked-webhook metadata. 1. A Buildkite environment fallback: `BUILDKITE_GITHUB_EVENT` preserves `push`, `pull_request`, `workflow_dispatch`, and `schedule` across rebuilds. Without one of those values, pull request builds use `pull_request`; `ui` and `api` use `workflow_dispatch`; `schedule` uses `schedule`; and every other source, including `trigger_job`, uses `push`. -An explicit event snapshot never consults contradictory live Buildkite event fields. Linked `merge_group` webhooks must match the Buildkite merge queue head and base refs and commits. Linked `release` webhooks must match the Buildkite event, action, branch, and tag; the plugin normalizes Buildkite's symbolic `HEAD` commit to the checked-out peeled tag commit first. The fallback cannot infer a merge group or release without linked webhook data. The selected snapshot is then used consistently for applicability, event-dependent validation and compilation, the `BUILDKITE_GITHUB_EVENT` group condition with its non-webhook fallback, and the event suffix in its provider check. +Buildkite's GitHub integration coalesces a first-party pull request synchronization into the branch's linked push build. When a GitHub `push` build has a Buildkite pull request number, upload selects one `pull_request` event with `synchronize` activity, the pull request base branch, and `refs/pull//head`. It does not also select `push`. A push without pull request metadata remains a `push`, so push branch and tag filters retain their normal semantics. + +An explicit event snapshot never consults contradictory live Buildkite event fields. Linked `merge_group` webhooks must match the Buildkite merge queue head and base refs and commits. Linked `release` webhooks must match the Buildkite event, action, branch, and tag; the plugin normalizes Buildkite's symbolic `HEAD` commit to the checked-out peeled tag commit first. The fallback cannot infer a merge group or release without linked webhook data. The selected snapshot is then used consistently for applicability, event-dependent validation and compilation, its exact Buildkite group condition, and the event suffix in its provider check. | Event | Supported trigger behavior | | --- | --- | @@ -157,6 +159,8 @@ on: Before upload, the importer compares the pull request merge base with its head using the local checkout. It admits a workflow only when a changed path matches and the linked webhook, commits, synthetic merge, base branch, and workflow file all agree. It uses the checkout's existing Git access for public, private, and fork pull requests. It does not call GitHub or use Buildkite `if_changed`. +A coalesced first-party synchronization has a linked push payload, not a pull request payload. The push payload cannot admit pull request path filters because it covers only that push, not the complete pull request comparison. + | Admitted | Rejected | | --- | --- | | A matching added, modified, deleted, or type-changed path | No local match | diff --git a/docs/security.md b/docs/security.md index f796818f..1c402d28 100644 --- a/docs/security.md +++ b/docs/security.md @@ -28,6 +28,8 @@ Public reusable workflows use the same bounded repository source and cache as pu Push and pull request path-filter admission uses Buildkite's reserved linked-webhook metadata only after binding it to the Buildkite repository and commit and matching local Git history. Missing, shallow, ambiguous, oversized, or mismatched evidence prevents admission. Explicit and generated snapshots cannot grant this admission. This check controls workflow selection; it does not make the selected workflow trusted. +A first-party pull request synchronization may be linked to a push payload. The importer models its trigger from Buildkite pull request metadata but does not treat the push payload as pull request path-filter evidence. + Release ingestion also requires reserved linked-webhook metadata. It binds the webhook activity to `BUILDKITE_GITHUB_ACTION`, the release tag to both `BUILDKITE_TAG` and `BUILDKITE_BRANCH`, and the event SHA to the checked-out commit after the plugin resolves Buildkite's symbolic `HEAD`. Environment fallback cannot invent a release event. Enable **Additional Webhooks** > **Releases** only with **Code** trigger mode. Reusable-workflow call conditions are immutable plan guards evaluated in caller scope. Direct `needs` values come only from producer-attributed, digest-bound result manifests; a missing or changed manifest stops the job with an error. A false guard skips the flattened job before secret retrieval, workflow-token minting, OIDC startup, action materialization, containers, or steps. diff --git a/internal/buildkite/triggers.go b/internal/buildkite/triggers.go index 67334d57..fb6f5550 100644 --- a/internal/buildkite/triggers.go +++ b/internal/buildkite/triggers.go @@ -346,7 +346,7 @@ func LiveEventPredicate(event string) string { fallbackEvent += " || (" + unsupportedEvent + "))" switch event { case "push": - return "(" + predicate + " || (" + fallbackEvent + ` && build.pull_request.id == null && build.source != "ui" && build.source != "api" && build.source != "schedule"))` + return "((" + predicate + ` && build.pull_request.id == null) || (` + fallbackEvent + ` && build.pull_request.id == null && build.source != "ui" && build.source != "api" && build.source != "schedule"))` case "pull_request": return "(" + predicate + " || (" + fallbackEvent + " && build.pull_request.id != null))" case "workflow_dispatch": @@ -360,6 +360,13 @@ func LiveEventPredicate(event string) string { } } +// LivePullRequestPushPredicate matches the push delivery that Buildkite's +// GitHub integration uses for a first-party pull request synchronization. +func LivePullRequestPushPredicate() string { + githubEvent := "build.env(" + yamlScalar("BUILDKITE_GITHUB_EVENT") + ")" + return "(" + githubEvent + ` == "push" && build.pull_request.id != null)` +} + func translateTrigger(t workflow.Trigger, expressions TriggerConditionExpressions, snapshot TriggerEventSnapshot, selected bool) (string, bool, error) { if !SupportedTriggerEvent(t.Event) { return "", false, &UnsupportedTriggerEventError{Event: t.Event} diff --git a/internal/buildkite/triggers_test.go b/internal/buildkite/triggers_test.go index 0402b33f..50d7eaf0 100644 --- a/internal/buildkite/triggers_test.go +++ b/internal/buildkite/triggers_test.go @@ -63,6 +63,23 @@ func TestLiveEventPredicatePreservesNonWebhookMappings(t *testing.T) { } } +func TestLivePushPredicatesPartitionPullRequestBuilds(t *testing.T) { + pullRequestPush := LivePullRequestPushPredicate() + for _, want := range []string{ + `build.env("BUILDKITE_GITHUB_EVENT") == "push"`, + `build.pull_request.id != null`, + } { + if !strings.Contains(pullRequestPush, want) { + t.Errorf("pull request push predicate missing %q: %s", want, pullRequestPush) + } + } + push := LiveEventPredicate("push") + want := `build.env("BUILDKITE_GITHUB_EVENT") == "push" && build.pull_request.id == null` + if !strings.Contains(push, want) { + t.Fatalf("push predicate does not exclude pull request builds: %s", push) + } +} + func TestTranslateTriggerConditionRejectsUnsafeTriggers(t *testing.T) { tests := []struct { name string diff --git a/internal/cli/buildkite_event.go b/internal/cli/buildkite_event.go index 602c31f2..5685dc76 100644 --- a/internal/cli/buildkite_event.go +++ b/internal/cli/buildkite_event.go @@ -103,17 +103,21 @@ func buildkiteEventSource(getenv func(string) string) ([]byte, error) { if githubEvent := strings.TrimSpace(getenv("BUILDKITE_GITHUB_EVENT")); githubEventNamePattern.MatchString(githubEvent) { switch githubEvent { case "push", "pull_request", "workflow_dispatch", "schedule": - event = githubEvent - // Rebuilds retain the original GitHub event even though Buildkite reports - // their source as UI. A push may also be associated with an open pull - // request, so restore its authoritative branch or tag ref. - if event == "push" { - if strings.TrimSpace(tag) != "" { - ref = "refs/tags/" + tag - } else if strings.TrimSpace(branch) != "" { - ref = "refs/heads/" + branch + // Buildkite's GitHub integration coalesces a first-party pull request + // synchronization into its linked push build. Preserve the pull request + // compatibility snapshot when the build carries that PR identity. + if githubEvent != "push" || !buildkitePushRepresentsPullRequest(getenv) { + event = githubEvent + // Rebuilds retain the original GitHub event even though Buildkite + // reports their source as UI. Restore a push's branch or tag ref. + if event == "push" { + if strings.TrimSpace(tag) != "" { + ref = "refs/tags/" + tag + } else if strings.TrimSpace(branch) != "" { + ref = "refs/heads/" + branch + } + payload = map[string]any{"ref": ref} } - payload = map[string]any{"ref": ref} } } } @@ -160,17 +164,23 @@ func buildkiteWebhookEventSource(getenv func(string) string, webhook []byte) ([] if err := decoder.Decode(&snapshot); err != nil { return nil, fmt.Errorf("decode Buildkite compatibility snapshot: %w", err) } + compatibilityPayload := snapshot["payload"] snapshot["payload"] = payload if event := strings.TrimSpace(getenv("BUILDKITE_GITHUB_EVENT")); githubEventNamePattern.MatchString(event) { - snapshot["event"] = event - // Buildkite can associate a push-created build with an open pull - // request. Keep the authoritative execution ref consistent with the - // linked webhook event rather than retaining refs/pull//head. - if event == "push" { - if tag := strings.TrimSpace(getenv("BUILDKITE_TAG")); tag != "" { - snapshot["ref"] = "refs/tags/" + tag - } else if branch := strings.TrimSpace(getenv("BUILDKITE_BRANCH")); branch != "" { - snapshot["ref"] = "refs/heads/" + branch + if event == "push" && buildkitePushRepresentsPullRequest(getenv) { + // A push payload describes only the latest branch update, not the + // complete pull request. Retain the trusted compatibility payload and + // do not promote the push webhook to pull request admission evidence. + snapshot["event"] = "pull_request" + snapshot["payload"] = compatibilityPayload + } else { + snapshot["event"] = event + if event == "push" { + if tag := strings.TrimSpace(getenv("BUILDKITE_TAG")); tag != "" { + snapshot["ref"] = "refs/tags/" + tag + } else if branch := strings.TrimSpace(getenv("BUILDKITE_BRANCH")); branch != "" { + snapshot["ref"] = "refs/heads/" + branch + } } } } @@ -199,6 +209,15 @@ func buildkiteWebhookEventSource(getenv func(string) string, webhook []byte) ([] return result, nil } +func buildkitePushRepresentsPullRequest(getenv func(string) string) bool { + provider, _, _, _, err := parseBuildkiteRepository(getenv("BUILDKITE_REPO")) + if err != nil || provider != "github" || strings.TrimSpace(getenv("BUILDKITE_GITHUB_EVENT")) != "push" { + return false + } + number, err := strconv.Atoi(getenv("BUILDKITE_PULL_REQUEST")) + return err == nil && number > 0 +} + func validateBuildkiteMergeGroup(snapshot map[string]any, getenv func(string) string) error { payload := snapshot["payload"].(map[string]any) mergeGroup, ok := payload["merge_group"].(map[string]any) diff --git a/internal/cli/buildkite_event_test.go b/internal/cli/buildkite_event_test.go index b4e15611..5ca14ffa 100644 --- a/internal/cli/buildkite_event_test.go +++ b/internal/cli/buildkite_event_test.go @@ -394,16 +394,17 @@ func TestBuildkiteEventSourceDoesNotInventReleaseFromEnvironment(t *testing.T) { } } -func TestBuildkiteWebhookPushUsesBranchRefForPullRequestAssociatedBuild(t *testing.T) { +func TestBuildkiteWebhookPushUsesPullRequestCompatibilitySnapshot(t *testing.T) { env := map[string]string{ "BUILDKITE": "true", "BUILDKITE_STEP_KEY": "step", - "BUILDKITE_REPO": "https://github.com/buildkite/buildkite-gha", - "BUILDKITE_COMMIT": strings.Repeat("a", 40), - "BUILDKITE_BRANCH": "feature", - "BUILDKITE_PULL_REQUEST": "42", - "BUILDKITE_GITHUB_EVENT": "push", - } - source, err := buildkiteWebhookEventSource(func(key string) string { return env[key] }, []byte(`{"ref":"refs/heads/feature"}`)) + "BUILDKITE_REPO": "https://github.com/buildkite/buildkite-gha", + "BUILDKITE_COMMIT": strings.Repeat("a", 40), + "BUILDKITE_BRANCH": "feature", + "BUILDKITE_PULL_REQUEST": "42", + "BUILDKITE_PULL_REQUEST_BASE_BRANCH": "main", + "BUILDKITE_GITHUB_EVENT": "push", + } + source, err := buildkiteWebhookEventSource(func(key string) string { return env[key] }, []byte(`{"ref":"refs/heads/feature","push_marker":"discarded"}`)) if err != nil { t.Fatal(err) } @@ -411,20 +412,25 @@ func TestBuildkiteWebhookPushUsesBranchRefForPullRequestAssociatedBuild(t *testi if err := json.Unmarshal(source, &snapshot); err != nil { t.Fatal(err) } - if snapshot["event"] != "push" || snapshot["ref"] != "refs/heads/feature" { - t.Fatalf("snapshot event/ref = %q / %q", snapshot["event"], snapshot["ref"]) + payload := snapshot["payload"].(map[string]any) + pullRequest := payload["pull_request"].(map[string]any) + if snapshot["event"] != "pull_request" || snapshot["ref"] != "refs/pull/42/head" || + payload["action"] != "synchronize" || payload["number"] != float64(42) || payload["push_marker"] != nil || + pullRequest["head"].(map[string]any)["ref"] != "feature" || pullRequest["base"].(map[string]any)["ref"] != "main" { + t.Fatalf("pull request synchronization snapshot = %#v", snapshot) } } -func TestBuildkiteEventSourceRebuiltPushResetsPullRequestPayload(t *testing.T) { +func TestBuildkiteEventSourceRebuiltPushPreservesPullRequestCompatibilitySnapshot(t *testing.T) { env := map[string]string{ "BUILDKITE": "true", "BUILDKITE_STEP_KEY": "step", - "BUILDKITE_REPO": "https://github.com/buildkite/buildkite-gha", - "BUILDKITE_COMMIT": strings.Repeat("a", 40), - "BUILDKITE_BRANCH": "feature", - "BUILDKITE_PULL_REQUEST": "42", - "BUILDKITE_GITHUB_EVENT": "push", - "BUILDKITE_SOURCE": "ui", + "BUILDKITE_REPO": "https://github.com/buildkite/buildkite-gha", + "BUILDKITE_COMMIT": strings.Repeat("a", 40), + "BUILDKITE_BRANCH": "feature", + "BUILDKITE_PULL_REQUEST": "42", + "BUILDKITE_PULL_REQUEST_BASE_BRANCH": "main", + "BUILDKITE_GITHUB_EVENT": "push", + "BUILDKITE_SOURCE": "ui", } source, err := buildkiteEventSource(func(key string) string { return env[key] }) if err != nil { @@ -435,7 +441,9 @@ func TestBuildkiteEventSourceRebuiltPushResetsPullRequestPayload(t *testing.T) { t.Fatal(err) } payload := snapshot["payload"].(map[string]any) - if snapshot["event"] != "push" || snapshot["ref"] != "refs/heads/feature" || payload["ref"] != "refs/heads/feature" || len(payload) != 1 { + pullRequest := payload["pull_request"].(map[string]any) + if snapshot["event"] != "pull_request" || snapshot["ref"] != "refs/pull/42/head" || + payload["action"] != "synchronize" || pullRequest["base"].(map[string]any)["ref"] != "main" { t.Fatalf("rebuilt push snapshot = %#v", snapshot) } } diff --git a/internal/cli/effective_event.go b/internal/cli/effective_event.go index 37fc3461..fa8ed598 100644 --- a/internal/cli/effective_event.go +++ b/internal/cli/effective_event.go @@ -21,9 +21,10 @@ const maxWebhookMetadataBytes = 25 << 20 type effectiveEventOrigin string const ( - effectiveEventFromPath effectiveEventOrigin = "event-path" - effectiveEventFromWebhook effectiveEventOrigin = "buildkite-webhook" - effectiveEventFromBuild effectiveEventOrigin = "buildkite-environment" + effectiveEventFromPath effectiveEventOrigin = "event-path" + effectiveEventFromWebhook effectiveEventOrigin = "buildkite-webhook" + effectiveEventFromBuild effectiveEventOrigin = "buildkite-environment" + effectiveEventFromPullRequestPush effectiveEventOrigin = "buildkite-pull-request-push" ) type effectiveEventSelection struct { @@ -47,10 +48,18 @@ func loadEffectiveEventSource(ctx context.Context, eventPath string, agent trans return nil, "", fmt.Errorf("buildkite:webhook exceeds %d bytes", maxWebhookMetadataBytes) } source, err := buildkiteWebhookEventSource(os.Getenv, webhook) - return source, effectiveEventFromWebhook, err + origin := effectiveEventFromWebhook + if buildkitePushRepresentsPullRequest(os.Getenv) { + origin = effectiveEventFromPullRequestPush + } + return source, origin, err case errors.Is(metadataErr, transport.ErrMetadataUnavailable): source, err := buildkiteEventSource(os.Getenv) - return source, effectiveEventFromBuild, err + origin := effectiveEventFromBuild + if buildkitePushRepresentsPullRequest(os.Getenv) { + origin = effectiveEventFromPullRequestPush + } + return source, origin, err default: return nil, "", metadataErr } @@ -70,7 +79,11 @@ func newEffectiveEvent(source []byte, origin effectiveEventOrigin) (effectiveEve if origin == effectiveEventFromPath { return effective, nil } - effective.TriggerExpressions.EventPredicate = buildkitepipeline.LiveEventPredicate(event.Event) + if origin == effectiveEventFromPullRequestPush { + effective.TriggerExpressions.EventPredicate = buildkitepipeline.LivePullRequestPushPredicate() + } else { + effective.TriggerExpressions.EventPredicate = buildkitepipeline.LiveEventPredicate(event.Event) + } return effective, nil } diff --git a/internal/cli/effective_event_test.go b/internal/cli/effective_event_test.go index 8eeff5bc..7832be3f 100644 --- a/internal/cli/effective_event_test.go +++ b/internal/cli/effective_event_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "reflect" + "strings" "testing" buildkitepipeline "github.com/buildkite/buildkite-gha/internal/buildkite" @@ -43,3 +44,29 @@ func TestNewEffectiveEventSeparatesExpressionsAndSnapshot(t *testing.T) { t.Fatalf("webhook effective event = expressions %#v, snapshot %#v", webhook.TriggerExpressions, webhook.TriggerSnapshot) } } + +func TestNewEffectiveEventUsesPullRequestPushPredicate(t *testing.T) { + env := map[string]string{ + "BUILDKITE": "true", "BUILDKITE_STEP_KEY": "importer", + "BUILDKITE_REPO": "https://github.com/acme/widgets", + "BUILDKITE_COMMIT": strings.Repeat("a", 40), + "BUILDKITE_BRANCH": "feature", + "BUILDKITE_PULL_REQUEST": "42", + "BUILDKITE_PULL_REQUEST_BASE_BRANCH": "main", + "BUILDKITE_GITHUB_EVENT": "push", + } + source, err := buildkiteEventSource(func(key string) string { return env[key] }) + if err != nil { + t.Fatal(err) + } + effective, err := newEffectiveEvent(source, effectiveEventFromPullRequestPush) + if err != nil { + t.Fatal(err) + } + if effective.Event.Event != "pull_request" || effective.Event.Ref != "refs/pull/42/head" || + effective.TriggerExpressions.EventPredicate != buildkitepipeline.LivePullRequestPushPredicate() || + effective.TriggerSnapshot.PullRequestBaseBranch == nil || *effective.TriggerSnapshot.PullRequestBaseBranch != "main" || + effective.TriggerSnapshot.PullRequestAction == nil || *effective.TriggerSnapshot.PullRequestAction != "synchronize" { + t.Fatalf("pull request push effective event = %#v", effective) + } +} diff --git a/internal/cli/path_filters_test.go b/internal/cli/path_filters_test.go index dcefe2a4..58dff313 100644 --- a/internal/cli/path_filters_test.go +++ b/internal/cli/path_filters_test.go @@ -440,13 +440,21 @@ func TestBoundedCommandOutput(t *testing.T) { } func TestPopulateChangedPathsRequiresLinkedWebhook(t *testing.T) { - for _, event := range []string{"push", "pull_request"} { - t.Run(event, func(t *testing.T) { + for _, test := range []struct { + name string + event string + origin effectiveEventOrigin + }{ + {name: "explicit push", event: "push", origin: effectiveEventFromPath}, + {name: "explicit pull request", event: "pull_request", origin: effectiveEventFromPath}, + {name: "pull request push", event: "pull_request", origin: effectiveEventFromPullRequestPush}, + } { + t.Run(test.name, func(t *testing.T) { snapshot := buildkitepipeline.TriggerEventSnapshot{} - populateChangedPaths(&snapshot, compiler.Event{Event: event}, effectiveEventFromPath, []workflowInput{{ - Triggers: []workflow.Trigger{{Event: event, Paths: []string{"src/**"}}}, + populateChangedPaths(&snapshot, compiler.Event{Event: test.event}, test.origin, []workflowInput{{ + Triggers: []workflow.Trigger{{Event: test.event, Paths: []string{"src/**"}}}, }}) - if snapshot.ChangedPaths.Paths != nil || !strings.Contains(snapshot.ChangedPaths.UnavailableReason, event+" path filters require linked Buildkite webhook") { + if snapshot.ChangedPaths.Paths != nil || !strings.Contains(snapshot.ChangedPaths.UnavailableReason, test.event+" path filters require linked Buildkite webhook") { t.Fatalf("changed-path snapshot = %#v", snapshot) } }) diff --git a/internal/cli/plugin_test.go b/internal/cli/plugin_test.go index ed623581..cb6327ec 100644 --- a/internal/cli/plugin_test.go +++ b/internal/cli/plugin_test.go @@ -314,6 +314,94 @@ func TestPluginUsesJSONConfigurationAndOnlyRequiredRuntime(t *testing.T) { } } +func TestPluginModelsFirstPartyPullRequestSynchronizationFromPush(t *testing.T) { + requireImporterHost(t) + repository := writeUploadWorkflowRepository(t, map[string]string{ + "ci.yml": "name: CI\non:\n push:\n branches: [main]\n pull_request:\n branches: [main]\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n", + }) + t.Chdir(repository) + configuration, err := json.Marshal(map[string]any{"workflow": ".github/workflows/ci.yml"}) + if err != nil { + t.Fatal(err) + } + t.Setenv(pluginConfigurationEnvironment, string(configuration)) + setCLIPluginBuildkiteEnvironment(t, "pull-request-push-importer") + sha := strings.Repeat("a", 40) + t.Setenv("BUILDKITE_COMMIT", sha) + t.Setenv("BUILDKITE_BRANCH", "amp/buildkite-gha") + t.Setenv("BUILDKITE_PULL_REQUEST", "583") + t.Setenv("BUILDKITE_PULL_REQUEST_BASE_BRANCH", "main") + t.Setenv("BUILDKITE_GITHUB_EVENT", "push") + runner := &cliCaptureRunner{webhook: []byte(`{ + "ref":"refs/heads/amp/buildkite-gha", + "before":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "after":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "repository":{"full_name":"buildkite/buildkite-gha"}, + "sender":{"login":"octocat"} +}`)} + var stdout, stderr bytes.Buffer + if code := run([]string{"plugin"}, &stdout, &stderr, "dev", runner); code != 0 { + t.Fatalf("run() code = %d, stderr = %q", code, stderr.String()) + } + var pipeline struct { + Steps []struct { + Group string `yaml:"group"` + Condition string `yaml:"if"` + Skip string `yaml:"skip"` + Steps []struct { + Notify []struct { + GitHubCheck struct { + Name string `yaml:"name"` + } `yaml:"github_check"` + } `yaml:"notify"` + } `yaml:"steps"` + } `yaml:"steps"` + } + if err := yaml.Unmarshal(runner.commands[len(runner.commands)-1].stdin, &pipeline); err != nil { + t.Fatal(err) + } + if len(pipeline.Steps) != 1 || pipeline.Steps[0].Group != ":github: workflow ยท CI" || pipeline.Steps[0].Skip != "" || len(pipeline.Steps[0].Steps) != 1 { + t.Fatalf("pull request synchronization pipeline = %#v", pipeline.Steps) + } + condition := pipeline.Steps[0].Condition + for _, want := range []string{ + `build.env("BUILDKITE_GITHUB_EVENT") == "push"`, + `build.pull_request.id != null`, + `"main" =~ /^main$/`, + `"synchronize" == "synchronize"`, + } { + if !strings.Contains(condition, want) { + t.Errorf("pull request synchronization condition missing %q: %s", want, condition) + } + } + if strings.Contains(condition, "amp/buildkite-gha") { + t.Errorf("pull request condition retained the false push branch: %s", condition) + } + check := pipeline.Steps[0].Steps[0].Notify + if len(check) != 1 || check[0].GitHubCheck.Name != "CI / test (pull_request)" { + t.Fatalf("pull request synchronization check = %#v", check) + } + planCount := 0 + for path, contents := range runner.uploaded { + if !strings.HasSuffix(path, ".json") { + continue + } + job, err := plan.Decode(contents) + if err != nil { + t.Fatal(err) + } + planCount++ + if job.Event.Name != "pull_request" || job.Event.Ref != "refs/pull/583/head" || + job.Event.HeadRef != "amp/buildkite-gha" || job.Event.BaseRef != "main" || + job.Event.SHA != sha || job.Event.Actor != "octocat" { + t.Fatalf("pull request synchronization plan event = %#v", job.Event) + } + } + if planCount != 1 { + t.Fatalf("plan count = %d, want 1", planCount) + } +} + func TestPluginIgnoresJobPermissionsForHostedGitHubToken(t *testing.T) { requireImporterHost(t) repository := writeUploadWorkflowRepository(t, map[string]string{