diff --git a/cmd/server/openapi/docs.go b/cmd/server/openapi/docs.go index d1b04160459..267b46d91d5 100644 --- a/cmd/server/openapi/docs.go +++ b/cmd/server/openapi/docs.go @@ -5076,6 +5076,10 @@ const docTemplate = `{ "author_email": { "type": "string" }, + "before": { + "description": "previous head commit of the pushed range; GitHub push events only, empty when there is no usable base (force push, tag, new branch, or a ref that did not move)", + "type": "string" + }, "branch": { "type": "string" }, diff --git a/docs/docs/20-usage/72-extensions/40-configuration-extension.md b/docs/docs/20-usage/72-extensions/40-configuration-extension.md index f263a0693c8..b532b71c0bc 100644 --- a/docs/docs/20-usage/72-extensions/40-configuration-extension.md +++ b/docs/docs/20-usage/72-extensions/40-configuration-extension.md @@ -102,6 +102,7 @@ Example request: "author": "myUser", "author_avatar": "https://myforge.com/avatars/d6b3f7787a685fcdf2a44e2c685c7e03", "author_email": "my@email.com", + "before": "1a2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d", "branch": "main", "changed_files": ["some-filename.txt"], "commit": "2fff90f8d288a4640e90f05049fe30e61a14fd50", @@ -144,6 +145,11 @@ Example request: } ``` +`pipeline.before` holds the previous head of a pushed commit range, which an extension can use +as the base for its own changed-file comparison. It is populated for GitHub push events only, +and is absent when the push has no usable base: a force push, a tag, a new branch, or a ref +that did not move. + ### Response The extension should respond with a JSON payload containing the new configuration files in Woodpecker's official YAML format. diff --git a/server/forge/github/github.go b/server/forge/github/github.go index d9a35f7005e..421eccca1d1 100644 --- a/server/forge/github/github.go +++ b/server/forge/github/github.go @@ -55,6 +55,9 @@ const ( // slow GitHub call does not fail with "context deadline exceeded" mid-report. statusReportTimeout = 30 * time.Second githubClientKey contextKey = "github_client" + // The all-zero commit id GitHub reports as the "before" commit of a push + // that creates a ref. It never names a real commit. + zeroSHA = "0000000000000000000000000000000000000000" ) // Opts defines configuration options. @@ -781,6 +784,9 @@ func (c *client) Hook(ctx context.Context, r *http.Request) (*model.Repo, *model return nil, nil, err } } else if pipeline != nil && pipeline.Event == model.EventPush { + if usablePushBase(currCommit, prevCommit) { + pipeline.Before = prevCommit + } // GitHub has removed commit summaries from Events API payloads from 7th October 2025 onwards. pipeline, err = c.loadChangedFilesFromCommits(ctx, repo, pipeline, currCommit, prevCommit) if err != nil { @@ -890,6 +896,15 @@ func (c *client) getTagCommitSHA(ctx context.Context, repo *model.Repo, tagName return tag.GetCommit().GetSHA(), nil } +// usablePushBase reports whether prev and curr name commits a push can be +// compared across. GitHub sends the all-zero SHA when the push creates the +// ref, and repeats curr when the ref did not move, neither of which is a +// usable base. A push carrying no head commit leaves curr empty, which is +// not a usable end of the range either. +func usablePushBase(curr, prev string) bool { + return curr != "" && prev != "" && prev != zeroSHA && prev != curr +} + func (c *client) loadChangedFilesFromCommits(ctx context.Context, tmpRepo *model.Repo, pipeline *model.Pipeline, curr, prev string) (*model.Pipeline, error) { _store, ok := store.TryFromContext(ctx) if !ok { @@ -901,7 +916,7 @@ func (c *client) loadChangedFilesFromCommits(ctx context.Context, tmpRepo *model case curr: log.Error().Msg("GitHub push event contains the same commit before and after, no changes detected") return pipeline, nil - case "0000000000000000000000000000000000000000": + case zeroSHA: prev = "" fallthrough case "": diff --git a/server/forge/github/push_base_test.go b/server/forge/github/push_base_test.go new file mode 100644 index 00000000000..066882b7e44 --- /dev/null +++ b/server/forge/github/push_base_test.go @@ -0,0 +1,172 @@ +// Copyright 2026 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package github + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/google/go-github/v90/github" + github_mock "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "go.woodpecker-ci.org/woodpecker/v3/server/forge/github/fixtures" + "go.woodpecker-ci.org/woodpecker/v3/server/model" + "go.woodpecker-ci.org/woodpecker/v3/server/store" + store_mocks "go.woodpecker-ci.org/woodpecker/v3/server/store/mocks" +) + +func TestUsablePushBase(t *testing.T) { + const curr = "366701fde727cb7a9e7f21eb88264f59f6f9b89c" + + const prev = "2f780193b136b72bfea4eeb640786a8c4450c7a2" + + tests := []struct { + name string + curr string + prev string + want bool + }{ + {name: "distinct previous head is a usable base", curr: curr, prev: prev, want: true}, + {name: "all-zero SHA is not a usable base", curr: curr, prev: zeroSHA}, + {name: "previous head equal to current is not a usable base", curr: curr, prev: curr}, + {name: "empty previous head is not a usable base", curr: curr, prev: ""}, + {name: "empty current head is not a usable base", curr: "", prev: prev}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, usablePushBase(tc.curr, tc.prev)) + }) + } +} + +// jsonHandler serves body as JSON on every request, unlike the single-shot FIFO +// entry WithRequestMatch installs. +func jsonHandler(t *testing.T, body any) http.Handler { + t.Helper() + + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(body); err != nil { + t.Errorf("encoding mocked GitHub response: %v", err) + } + }) +} + +// pushHookWithBefore rewrites the "before" commit of the sample push hook, +// leaving the rest of the payload untouched. GitHub varies only that field +// between an ordinary push, a ref creation and a no-op push. +func pushHookWithBefore(t *testing.T, before string) string { + t.Helper() + + var payload map[string]any + require.NoError(t, json.Unmarshal([]byte(fixtures.HookPush), &payload)) + payload["before"] = before + + raw, err := json.Marshal(payload) + require.NoError(t, err) + return string(raw) +} + +// pushBaseTestClient wires a client whose changed-file lookups are mocked and +// whose store resolves to a fake repo and user, mirroring the harness in +// TestHook. Only pipeline.Before is under test here; the changed-file payloads +// exist so Hook can run to completion, so the endpoints are served by handlers +// that answer any number of times rather than the FIFO queue WithRequestMatch +// installs, which a table of subtests would exhaust. +func pushBaseTestClient(t *testing.T) (*client, context.Context) { + t.Helper() + + changedFile := []*github.CommitFile{{Filename: github.Ptr("main.go")}} + mockedHTTPClient := github_mock.NewMockedHTTPClient( + github_mock.WithRequestMatchHandler( + github_mock.GetReposCommitsByOwnerByRepoByRef, + jsonHandler(t, github.RepositoryCommit{Files: changedFile}), + ), + github_mock.WithRequestMatchHandler( + github_mock.GetReposCompareByOwnerByRepoByBasehead, + jsonHandler(t, github.CommitsComparison{Files: changedFile}), + ), + ) + + gh, err := github.NewClient(github.WithHTTPClient(mockedHTTPClient)) + require.NoError(t, err) + + mockStore := store_mocks.NewMockStore(t) + mockStore.On("GetUser", mock.Anything).Return(&model.User{ID: 1, Login: "6543", AccessToken: "token"}, nil).Maybe() + mockStore.On("GetRepoNameFallback", mock.Anything, mock.Anything, mock.Anything).Return(&model.Repo{ + ID: 1, + ForgeRemoteID: "1", + Owner: "6543", + Name: "hello-world", + UserID: 1, + }, nil).Maybe() + + ctx := context.WithValue(t.Context(), githubClientKey, gh) + ctx = store.InjectToContext(ctx, mockStore) + + return &client{API: defaultAPI, url: defaultURL}, ctx +} + +func TestHookPushBefore(t *testing.T) { + const head = "366701fde727cb7a9e7f21eb88264f59f6f9b89c" + + tests := []struct { + name string + payload string + want string + }{ + { + name: "ordinary push carries the previous head", + payload: fixtures.HookPush, + want: "2f780193b136b72bfea4eeb640786a8c4450c7a2", + }, + { + name: "branch creation drops the all-zero base", + payload: pushHookWithBefore(t, zeroSHA), + }, + { + name: "push that does not move the ref drops the base", + payload: pushHookWithBefore(t, head), + }, + { + name: "force push drops the rewritten base", + payload: fixtures.HookPushForced, + }, + } + + c, ctx := pushBaseTestClient(t) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest("POST", "/hook", strings.NewReader(tc.payload)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-GitHub-Event", "push") + + _, pipeline, err := c.Hook(ctx, req) + require.NoError(t, err) + require.NotNil(t, pipeline) + require.Equal(t, model.EventPush, pipeline.Event) + assert.Equal(t, tc.want, pipeline.Before) + }) + } +} diff --git a/server/model/pipeline.go b/server/model/pipeline.go index 48f1c179e42..0aa83f54139 100644 --- a/server/model/pipeline.go +++ b/server/model/pipeline.go @@ -39,6 +39,7 @@ type Pipeline struct { DeployTo string `json:"deploy_to" xorm:"deploy"` DeployTask string `json:"deploy_task" xorm:"deploy_task"` Commit string `json:"commit" xorm:"commit"` + Before string `json:"before,omitempty" xorm:"before"` // previous head commit of the pushed range; GitHub push events only, empty when there is no usable base (force push, tag, new branch, or a ref that did not move) Branch string `json:"branch" xorm:"branch"` RerunCount int64 `json:"rerun_count" xorm:"rerun_count"` Ref string `json:"ref" xorm:"ref"` diff --git a/server/model/pipeline_test.go b/server/model/pipeline_test.go index e728fbae6ef..df81f2f37c3 100644 --- a/server/model/pipeline_test.go +++ b/server/model/pipeline_test.go @@ -15,9 +15,11 @@ package model import ( + "encoding/json" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPipelineToAPIModel(t *testing.T) { @@ -75,3 +77,29 @@ func TestPipelineToAPIModel(t *testing.T) { }) } } + +func TestPipelineBeforeJSON(t *testing.T) { + t.Run("before is carried in the payload when set", func(t *testing.T) { + raw, err := json.Marshal(Pipeline{ + Commit: "366701fde727cb7a9e7f21eb88264f59f6f9b89c", + Before: "2f780193b136b72bfea4eeb640786a8c4450c7a2", + }) + require.NoError(t, err) + + var payload map[string]any + require.NoError(t, json.Unmarshal(raw, &payload)) + assert.Equal(t, "2f780193b136b72bfea4eeb640786a8c4450c7a2", payload["before"]) + }) + + t.Run("before is omitted from the payload when empty", func(t *testing.T) { + raw, err := json.Marshal(Pipeline{Commit: "366701fde727cb7a9e7f21eb88264f59f6f9b89c"}) + require.NoError(t, err) + + var payload map[string]any + require.NoError(t, json.Unmarshal(raw, &payload)) + // commit has no omitempty, so its presence proves the payload really was + // inspected and "before" is absent by the tag, not by a broken decode. + assert.Contains(t, payload, "commit") + assert.NotContains(t, payload, "before") + }) +} diff --git a/server/store/datastore/pipeline_test.go b/server/store/datastore/pipeline_test.go index 6477244d8b5..903fde477af 100644 --- a/server/store/datastore/pipeline_test.go +++ b/server/store/datastore/pipeline_test.go @@ -122,6 +122,41 @@ func TestPipelines(t *testing.T) { assert.EqualValues(t, pipeline2, GetPipeline) } +// TestPipelineBeforePersists pins Before to a real column. The config +// extension reads it off a pipeline the store loaded, which is what a restart +// hands to config.Fetch, so an in-memory-only field would read empty there and +// silently narrow the compared range instead of failing. +func TestPipelineBeforePersists(t *testing.T) { + const before = "2f780193b136b72bfea4eeb640786a8c4450c7a2" + + repo := &model.Repo{ + UserID: 1, + FullName: "bradrydzewski/test", + Owner: "bradrydzewski", + Name: "test", + } + + store, closer := newTestStore(t, new(model.Repo), new(model.Step), new(model.Pipeline)) + defer closer() + + assert.NoError(t, store.CreateRepo(repo)) + + pipeline := model.Pipeline{ + RepoID: repo.ID, + Status: model.StatusSuccess, + Event: model.EventPush, + Branch: "some-branch", + Commit: "366701fde727cb7a9e7f21eb88264f59f6f9b89c", + Before: before, + } + assert.NoError(t, store.CreatePipeline(&pipeline)) + + // GetPipelineNumber is the accessor the restart path uses. + loaded, err := store.GetPipelineNumber(repo, pipeline.Number) + assert.NoError(t, err) + assert.Equal(t, before, loaded.Before) +} + func TestPipelineListFilter(t *testing.T) { repo := &model.Repo{ UserID: 1,