Skip to content

Commit 3921ac8

Browse files
committed
Add a CheckPrerequisite step to each workflow
1 parent d858ef8 commit 3921ac8

9 files changed

Lines changed: 715 additions & 10 deletions

cmd/ateapi/internal/controlapi/workflow.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ type WorkflowStep[Params any, Context any] interface {
4444
// If it returns true, the engine skips Execute() and fast-forwards to the next step.
4545
IsComplete(ctx context.Context, params Params, wCtx Context) (bool, error)
4646

47+
// CheckPrerequisite validates that the current state permits executing this
48+
// step (e.g. the actor's status allows this state-machine edge). The engine
49+
// calls it only when IsComplete returned false, immediately before Execute,
50+
// so completed steps of a retried workflow fast-forward without
51+
// re-validation. Return a gRPC status error with
52+
// codes.FailedPrecondition to abort the workflow if prereqs are not met.
53+
CheckPrerequisite(ctx context.Context, params Params, wCtx Context) error
54+
4755
// Execute performs the step's business logic and persists any state changes.
4856
// If an error is returned, the workflow stops and relies on the client to retry.
4957
Execute(ctx context.Context, params Params, wCtx Context) error
@@ -79,6 +87,13 @@ func RunWorkflow[Params any, Context any](ctx context.Context, params Params, wC
7987
continue
8088
}
8189

90+
if err := step.CheckPrerequisite(ctx, params, wCtx); err != nil {
91+
span.RecordError(err)
92+
span.SetStatus(codes.Error, err.Error())
93+
span.End()
94+
return fmt.Errorf("prerequisite not met at step %s: %w", step.Name(), err)
95+
}
96+
8297
err = runStep(ctx, params, wCtx, step)
8398
if err != nil {
8499
span.RecordError(err)

cmd/ateapi/internal/controlapi/workflow_pause.go

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1"
2828
listersv1alpha1 "github.com/agent-substrate/substrate/pkg/client/listers/api/v1alpha1"
2929
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
30+
"google.golang.org/grpc/codes"
31+
"google.golang.org/grpc/status"
3032
"k8s.io/apimachinery/pkg/util/wait"
3133
)
3234

@@ -52,6 +54,9 @@ func (s *LoadActorForPauseStep) IsComplete(ctx context.Context, input *PauseInpu
5254
// Always run to get the freshest state
5355
return false, nil
5456
}
57+
func (s *LoadActorForPauseStep) CheckPrerequisite(ctx context.Context, input *PauseInput, state *PauseState) error {
58+
return nil
59+
}
5560
func (s *LoadActorForPauseStep) Execute(ctx context.Context, input *PauseInput, state *PauseState) error {
5661
actor, err := s.store.GetActor(ctx, input.Atespace, input.ActorID)
5762
if err != nil {
@@ -79,11 +84,14 @@ func (s *MarkPausingStep) IsComplete(ctx context.Context, input *PauseInput, sta
7984
// Fast forward if we've already marked our intent or if we are further along.
8085
return state.Actor.GetStatus() == ateapipb.Actor_STATUS_PAUSING || state.Actor.GetStatus() == ateapipb.Actor_STATUS_PAUSED, nil
8186
}
82-
func (s *MarkPausingStep) Execute(ctx context.Context, input *PauseInput, state *PauseState) error {
87+
func (s *MarkPausingStep) CheckPrerequisite(ctx context.Context, input *PauseInput, state *PauseState) error {
88+
// The pause edge only exists from RUNNING; PAUSING/PAUSED are fast-forwarded by IsComplete.
8389
if state.Actor.GetStatus() != ateapipb.Actor_STATUS_RUNNING {
84-
return nil
90+
return status.Errorf(codes.FailedPrecondition, "MarkPausingStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorID, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RUNNING)
8591
}
86-
92+
return nil
93+
}
94+
func (s *MarkPausingStep) Execute(ctx context.Context, input *PauseInput, state *PauseState) error {
8795
state.Actor.Status = ateapipb.Actor_STATUS_PAUSING
8896
state.Actor.InProgressSnapshot = fmt.Sprintf("%s-%s-%s", state.Actor.GetActorId(), time.Now().Format(time.RFC3339), rand.Text())
8997
return s.store.UpdateActor(ctx, state.Actor, state.Actor.GetVersion())
@@ -101,6 +109,12 @@ func (s *CallAteletPauseStep) IsComplete(ctx context.Context, input *PauseInput,
101109
// If we are already PAUSED, we've already called Atelet
102110
return state.Actor.GetStatus() == ateapipb.Actor_STATUS_PAUSED, nil
103111
}
112+
func (s *CallAteletPauseStep) CheckPrerequisite(ctx context.Context, input *PauseInput, state *PauseState) error {
113+
if state.Actor.GetStatus() != ateapipb.Actor_STATUS_PAUSING {
114+
return status.Errorf(codes.FailedPrecondition, "CallAteletPauseStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorID, state.Actor.GetStatus(), ateapipb.Actor_STATUS_PAUSING)
115+
}
116+
return nil
117+
}
104118
func (s *CallAteletPauseStep) Execute(ctx context.Context, input *PauseInput, state *PauseState) error {
105119
if state.Actor.GetAteomPodNamespace() == "" || state.Actor.GetAteomPodName() == "" {
106120
if err := crashActor(ctx, s.store, state.Actor.Atespace, state.Actor.ActorId); err != nil {
@@ -152,8 +166,13 @@ type FinalizePausedStep struct {
152166

153167
func (s *FinalizePausedStep) Name() string { return "FinalizePaused" }
154168
func (s *FinalizePausedStep) IsComplete(ctx context.Context, input *PauseInput, state *PauseState) (bool, error) {
155-
// The workflow is completely done ONLY if the status is PAUSED *and* we've successfully freed the worker.
156-
return state.Actor.GetStatus() == ateapipb.Actor_STATUS_PAUSED && state.Actor.GetAteomPodNamespace() == "", nil
169+
return state.Actor.GetStatus() == ateapipb.Actor_STATUS_PAUSED, nil
170+
}
171+
func (s *FinalizePausedStep) CheckPrerequisite(ctx context.Context, input *PauseInput, state *PauseState) error {
172+
if state.Actor.GetStatus() != ateapipb.Actor_STATUS_PAUSING {
173+
return status.Errorf(codes.FailedPrecondition, "FinalizePausedStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorID, state.Actor.GetStatus(), ateapipb.Actor_STATUS_PAUSING)
174+
}
175+
return nil
157176
}
158177
func (s *FinalizePausedStep) Execute(ctx context.Context, input *PauseInput, state *PauseState) error {
159178
latestActor, err := s.store.GetActor(ctx, input.Atespace, input.ActorID)
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package controlapi
16+
17+
import (
18+
"context"
19+
"testing"
20+
21+
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store/storetest"
22+
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
23+
"google.golang.org/grpc/codes"
24+
"google.golang.org/grpc/status"
25+
)
26+
27+
// TestPauseActorWorkflow exercises the pause workflow end-to-end against
28+
// seeded actor statuses, covering both the rejected and the idempotent-success
29+
// paths. The atelet dialer is nil, so any step that unexpectedly reaches it
30+
// panics.
31+
func TestPauseActorWorkflow(t *testing.T) {
32+
tests := []struct {
33+
name string
34+
seedStatus ateapipb.Actor_Status
35+
// wantErr true means PauseActor must fail with FailedPrecondition.
36+
wantErr bool
37+
// wantStatus is the stored status after the call.
38+
wantStatus ateapipb.Actor_Status
39+
}{
40+
{
41+
// Pausing a SUSPENDED actor is rejected by MarkPausingStep's
42+
// CheckPrerequisite and the actor's status is left untouched.
43+
name: "not running rejected",
44+
seedStatus: ateapipb.Actor_STATUS_SUSPENDED,
45+
wantErr: true,
46+
wantStatus: ateapipb.Actor_STATUS_SUSPENDED,
47+
},
48+
{
49+
// Pausing a PAUSED actor succeeds idempotently via IsComplete
50+
// fast-forward without calling atelet.
51+
name: "already paused succeeds",
52+
seedStatus: ateapipb.Actor_STATUS_PAUSED,
53+
wantStatus: ateapipb.Actor_STATUS_PAUSED,
54+
},
55+
}
56+
for _, tc := range tests {
57+
t.Run(tc.name, func(t *testing.T) {
58+
ctx := context.Background()
59+
st, cleanup := storetest.SetupTestStore(t)
60+
defer cleanup()
61+
w := newTestActorWorkflow(t, st, "ns", "tmpl1")
62+
63+
seedWorkflowActor(t, ctx, st, "team-a", "id1", "ns", "tmpl1", tc.seedStatus)
64+
65+
actor, err := w.PauseActor(ctx, "team-a", "id1")
66+
if tc.wantErr {
67+
if got := status.Code(err); got != codes.FailedPrecondition {
68+
t.Fatalf("status.Code(err) = %v, want %v (err: %v)", got, codes.FailedPrecondition, err)
69+
}
70+
} else {
71+
if err != nil {
72+
t.Fatalf("PauseActor failed: %v", err)
73+
}
74+
if actor.GetStatus() != tc.wantStatus {
75+
t.Errorf("returned status = %v, want %v", actor.GetStatus(), tc.wantStatus)
76+
}
77+
}
78+
79+
got, err := st.GetActor(ctx, "team-a", "id1")
80+
if err != nil {
81+
t.Fatalf("GetActor failed: %v", err)
82+
}
83+
if got.GetStatus() != tc.wantStatus {
84+
t.Errorf("stored status = %v, want %v", got.GetStatus(), tc.wantStatus)
85+
}
86+
})
87+
}
88+
}
89+
90+
// TestPauseSteps_CheckPrerequisite verifies each pause step's CheckPrerequisite
91+
// against every actor status: nil for the step's allowed statuses,
92+
// FailedPrecondition for all others.
93+
func TestPauseSteps_CheckPrerequisite(t *testing.T) {
94+
tests := []struct {
95+
name string
96+
step WorkflowStep[*PauseInput, *PauseState]
97+
// allowed lists the statuses CheckPrerequisite accepts; nil means
98+
// every status is accepted.
99+
allowed map[ateapipb.Actor_Status]bool
100+
}{
101+
{
102+
// Loading has no prerequisite: it is allowed from every status.
103+
name: "LoadActorForPauseStep",
104+
step: &LoadActorForPauseStep{},
105+
allowed: nil,
106+
},
107+
{
108+
// Pausing is allowed only from RUNNING.
109+
name: "MarkPausingStep",
110+
step: &MarkPausingStep{},
111+
allowed: map[ateapipb.Actor_Status]bool{
112+
ateapipb.Actor_STATUS_RUNNING: true,
113+
},
114+
},
115+
{
116+
// The checkpoint call is allowed only from PAUSING (PAUSED is
117+
// fast-forwarded by IsComplete).
118+
name: "CallAteletPauseStep",
119+
step: &CallAteletPauseStep{},
120+
allowed: map[ateapipb.Actor_Status]bool{
121+
ateapipb.Actor_STATUS_PAUSING: true,
122+
},
123+
},
124+
{
125+
// Finalizing is allowed only from PAUSING: a persisted PAUSED
126+
// actor always has its worker pod fields cleared and is
127+
// fast-forwarded by IsComplete.
128+
name: "FinalizePausedStep",
129+
step: &FinalizePausedStep{},
130+
allowed: map[ateapipb.Actor_Status]bool{
131+
ateapipb.Actor_STATUS_PAUSING: true,
132+
},
133+
},
134+
}
135+
for _, tc := range tests {
136+
t.Run(tc.name, func(t *testing.T) {
137+
ctx := context.Background()
138+
for _, st := range allActorStatuses {
139+
err := tc.step.CheckPrerequisite(ctx, &PauseInput{ActorID: "id1"}, &PauseState{Actor: &ateapipb.Actor{Status: st}})
140+
assertPrerequisiteResult(t, st, err, tc.allowed == nil || tc.allowed[st])
141+
}
142+
})
143+
}
144+
}

cmd/ateapi/internal/controlapi/workflow_resume.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ func (s *LoadActorForResumeStep) IsComplete(ctx context.Context, input *ResumeIn
6161
// Always run this step to get the latest state from the DB
6262
return false, nil
6363
}
64+
func (s *LoadActorForResumeStep) CheckPrerequisite(ctx context.Context, input *ResumeInput, state *ResumeState) error {
65+
return nil
66+
}
6467
func (s *LoadActorForResumeStep) Execute(ctx context.Context, input *ResumeInput, state *ResumeState) error {
6568
actor, err := s.store.GetActor(ctx, input.Atespace, input.ActorID)
6669
if err != nil {
@@ -113,8 +116,22 @@ type AssignWorkerStep struct {
113116
func (s *AssignWorkerStep) Name() string { return "AssignWorker" }
114117

115118
func (s *AssignWorkerStep) IsComplete(ctx context.Context, input *ResumeInput, state *ResumeState) (bool, error) {
119+
// Only RUNNING is past this step. RESUMING intentionally re-runs because
120+
// a retry must be able to release a stale worker whose pool became
121+
// ineligible and pick a fresh one.
116122
return state.Actor.GetStatus() == ateapipb.Actor_STATUS_RUNNING, nil
117123
}
124+
func (s *AssignWorkerStep) CheckPrerequisite(ctx context.Context, input *ResumeInput, state *ResumeState) error {
125+
// The resume edge exists from SUSPENDED and PAUSED.
126+
// RESUMING is allowed for retrying this step.
127+
switch state.Actor.GetStatus() {
128+
case ateapipb.Actor_STATUS_SUSPENDED, ateapipb.Actor_STATUS_PAUSED, ateapipb.Actor_STATUS_RESUMING:
129+
return nil
130+
default:
131+
return status.Errorf(codes.FailedPrecondition, "AssignWorkerStep prerequisite not met for Actor: %s (got: %v, want %s, %s or %s)", input.ActorID, state.Actor.GetStatus(), ateapipb.Actor_STATUS_SUSPENDED, ateapipb.Actor_STATUS_PAUSED, ateapipb.Actor_STATUS_RESUMING)
132+
}
133+
}
134+
118135
func (s *AssignWorkerStep) Execute(ctx context.Context, input *ResumeInput, state *ResumeState) error {
119136
workers, err := s.workerCache.Workers()
120137
if err != nil {
@@ -252,6 +269,12 @@ func (s *CallAteletRestoreStep) Name() string { return "CallAteletRestore" }
252269
func (s *CallAteletRestoreStep) IsComplete(ctx context.Context, input *ResumeInput, state *ResumeState) (bool, error) {
253270
return state.Actor.GetStatus() == ateapipb.Actor_STATUS_RUNNING, nil
254271
}
272+
func (s *CallAteletRestoreStep) CheckPrerequisite(ctx context.Context, input *ResumeInput, state *ResumeState) error {
273+
if state.Actor.GetStatus() != ateapipb.Actor_STATUS_RESUMING {
274+
return status.Errorf(codes.FailedPrecondition, "CallAteletRestoreStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorID, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RESUMING)
275+
}
276+
return nil
277+
}
255278
func (s *CallAteletRestoreStep) Execute(ctx context.Context, input *ResumeInput, state *ResumeState) error {
256279
ateletConn, err := s.dialer.DialForWorker(state.Actor.GetAteomPodNamespace(), state.Actor.GetAteomPodName())
257280
if err != nil {
@@ -356,6 +379,12 @@ func (s *FinalizeRunningStep) Name() string { return "FinalizeRunning" }
356379
func (s *FinalizeRunningStep) IsComplete(ctx context.Context, input *ResumeInput, state *ResumeState) (bool, error) {
357380
return state.Actor.GetStatus() == ateapipb.Actor_STATUS_RUNNING, nil
358381
}
382+
func (s *FinalizeRunningStep) CheckPrerequisite(ctx context.Context, input *ResumeInput, state *ResumeState) error {
383+
if state.Actor.GetStatus() != ateapipb.Actor_STATUS_RESUMING {
384+
return status.Errorf(codes.FailedPrecondition, "FinalizeRunningStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorID, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RESUMING)
385+
}
386+
return nil
387+
}
359388
func (s *FinalizeRunningStep) Execute(ctx context.Context, input *ResumeInput, state *ResumeState) error {
360389
latestActor, err := s.store.GetActor(ctx, input.Atespace, input.ActorID)
361390
if err != nil {

0 commit comments

Comments
 (0)