Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions agent/rpc/client_grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ func (c *client) Update(ctx context.Context, workflowID string, state rpc.StepSt
req.State.ExitCode = int32(state.ExitCode)
req.State.Error = state.Error
req.State.Canceled = state.Canceled
req.State.Skipped = state.Skipped
for {
_, err = c.client.Update(ctx, req)
if err == nil {
Expand Down
13 changes: 11 additions & 2 deletions rpc/proto/woodpecker.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rpc/proto/woodpecker.proto
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ message StepState {
int32 exit_code = 5;
string error = 6;
bool canceled = 7;
bool skipped = 8;
}

message WorkflowState {
Expand Down
2 changes: 1 addition & 1 deletion rpc/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type (
ExitCode int `json:"exit_code"`
Error string `json:"error"`
Canceled bool `json:"canceled"`
Skipped bool `json:"Skipped"`
Skipped bool `json:"skipped"`
}

// WorkflowState defines the workflow state.
Expand Down
36 changes: 19 additions & 17 deletions server/pipeline/step_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ func UpdateStepStatus(ctx context.Context, store store.Store, step *model.Step,

switch step.State {
case model.StatusPending:
// Handle skip before anything else — skipped steps never started,
// so we must not set Started or transition through Running.
if state.Skipped {
step.State = model.StatusSkipped
if state.Finished != 0 {
step.Finished = state.Finished
}
return store.StepUpdate(step)
}

// Transition from pending to running when started
if state.Finished == 0 {
step.State = model.StatusRunning
Expand All @@ -43,10 +53,6 @@ func UpdateStepStatus(ctx context.Context, store store.Store, step *model.Step,
step.Started = time.Now().Unix()
}

if state.Skipped {
step.State = model.StatusSkipped
}

// Handle direct transition to finished if step setup error happened
if state.Exited || state.Error != "" {
step.Finished = state.Finished
Expand All @@ -56,18 +62,15 @@ func UpdateStepStatus(ctx context.Context, store store.Store, step *model.Step,
step.ExitCode = state.ExitCode
step.Error = state.Error

if !state.Skipped {
if state.ExitCode == 0 && state.Error == "" {
step.State = model.StatusSuccess
} else {
step.State = model.StatusFailure

if step.Failure == model.FailureCancel {
// cancel the pipeline
err := cancelPipelineFromStep(ctx, store, step)
if err != nil {
return err
}
if state.ExitCode == 0 && state.Error == "" {
step.State = model.StatusSuccess
} else {
step.State = model.StatusFailure

if step.Failure == model.FailureCancel {
err := cancelPipelineFromStep(ctx, store, step)
if err != nil {
return err
}
}
}
Expand All @@ -89,7 +92,6 @@ func UpdateStepStatus(ctx context.Context, store store.Store, step *model.Step,
step.State = model.StatusFailure

if step.Failure == model.FailureCancel {
// cancel the pipeline
err := cancelPipelineFromStep(ctx, store, step)
if err != nil {
return err
Expand Down
30 changes: 30 additions & 0 deletions server/pipeline/step_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,36 @@ func TestUpdateStepStatus(t *testing.T) {
assert.Equal(t, 1, step.ExitCode)
assert.Equal(t, "canceled", step.Error)
})
})

t.Run("Skipped", func(t *testing.T) {
t.Parallel()

// This mirrors exactly what the agent sends when executor.go detects
// OnSuccess=false or OnFailure=false — only Skipped is set, everything
// else is zero/false (no Started, no Finished, not Exited).
t.Run("PendingToSkipped_AgentPayload", func(t *testing.T) {
t.Parallel()
step := &model.Step{State: model.StatusPending}
// Exact payload from: traceStep(&backend.State{Skipped: true}, nil, step)
// Started=0, Finished=0, Exited=false, Skipped=true
state := rpc.StepState{
Skipped: true,
Exited: false,
Finished: 0,
Started: 0,
}

err := UpdateStepStatus(t.Context(), mockStoreStep(t), step, state)

assert.NoError(t, err)
// Must be Skipped, NOT Running (the bug: Finished==0 triggers StatusRunning first)
assert.Equal(t, model.StatusSkipped, step.State)
// Started must NOT be set — skipped steps never ran
assert.Equal(t, int64(0), step.Started)
// Finished must NOT be set — skipped steps never ran
assert.Equal(t, int64(0), step.Finished)
})

t.Run("PendingToSkipped", func(t *testing.T) {
t.Parallel()
Expand Down
1 change: 1 addition & 0 deletions server/rpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func (s *WoodpeckerServer) Update(c context.Context, req *proto.UpdateRequest) (
Error: req.GetState().GetError(),
ExitCode: int(req.GetState().GetExitCode()),
Canceled: req.GetState().GetCanceled(),
Skipped: req.GetState().GetSkipped(),
}
res := new(proto.Empty)
err := s.peer.Update(c, req.GetId(), state)
Expand Down