feat(api): introduce RevertActor lifecycle RPC by shrutiyam-glitch · Pull Request #1675 · agent-substrate/substrate

shrutiyam-glitch · 2026-09-16T15:59:29Z

Summary

This PR introduces the RevertActor RPC for actor lifecycle management. It includes the API definition, the corresponding workflow execution logic, observability metrics, and updates to the authorization model to support reverting actors.

Fixes #1556
Docs updated in #1711

Commit-wise Changes

1. Add RevertActor RPC (1145d99)

Introduces the new RevertActor RPC to the API definitions.
Updates the corresponding protobuf bindings (affecting files like ateapi.pb.go and ateapi_pb2.py).

2. Add the revertActor workflow and observability metrics (dcb1fae)

Implements the core revertActor workflow logic, designed to be idempotent and re-enterable. It progresses through the following steps:
- Mark Reverting: Validates that the actor is in a revertable state (RUNNING, PAUSED, or CRASHED) and transitions its state to REVERTING.
- Discard Worker: Safely tears down the execution environment by terminating the workload, detaching volumes, and releasing the assigned worker.
- Collect In-Progress Snapshot: Cleans up external object storage by deleting any objects a previous suspend operation was partway through writing.
- Finalize: Commits the actor to SUSPENDED and strips all node-local and in-progress state pointers (clearing WorkerAssignment, LocalSnapshotInfo, etc.), returning the actor to its untouched external snapshot.
Instruments the workflow with lifecycle operation metrics (e.g., updating ate.actor.lifecycle.operation.duration to track revert operations).
Note/TODO: Currently, when reverting a paused actor, the workflow drops the pointer to the node-local state but does not actually prune the local checkpoint bytes from the node (this is tracked in DeleteActor never cleans the actor's node directory #641).

3. Add can_revert to the authorization model (2fc53ae)

Adds the can_revert permission to the auth model, mirroring the shape of can_suspend (editor tier of the parent atespace, plus a direct grant so a machine identity can revert the actor it drives without holding an atespace role).

4. Serve RevertActor and add the CLI verb (f04fab4)

Wires the Control.RevertActor service method to the workflow (replacing the generated stub that previously answered Unimplemented).
Adds the "ate revert actor" CLI command, making the feature usable end-to-end.
Implements Terminate for the fake atelet. This was necessary because reverting an actor from the RUNNING state is the first path to reach this call in functional tests (previously, delete tests skipped this step as they ran against actors with no worker assignment).

5. Add a manual verify script for RevertActor (8022db3)

Adds a script to manually exercise RevertActor against a real control plane, since unit and functional tests only run against a fake atelet.
Tests reverting from CRASHED, RUNNING, and PAUSED states, and verifies that attempting to revert a SUSPENDED actor is properly rejected.
Simulates a crash by deleting the worker pod the actor runs on to verify the workflow can handle the absence of a worker to terminate.

Tests pass
Appropriate changes to documentation are included in the PR

RevertActor is a new actor lifecycle RPC, so the model needs a permission for it. Give it the same shape as can_suspend: the editor tier of the parent atespace, plus a direct grant so a machine identity can revert the actor it drives without holding an atespace role. Revert is the recovery counterpart of suspend and discards execution the same way, so anyone trusted to suspend an actor is trusted to revert it. Viewers and cluster viewers are asserted false, matching the other data plane verbs. Nothing in Go reads these relations yet. This is the model and its test suite only, as with can_suspend and can_resume.

The workflow existed but nothing reached it: Control.RevertActor was still the generated stub answering Unimplemented. Wire the service method to the workflow and add "ate revert actor", so the feature is usable end to end. The fake atelet gained a Terminate implementation. It had none, because no functional test had ever reached that call: delete is the only other caller and every delete test runs against an actor with no worker assignment, which skips the step. Revert from RUNNING is the first path that gets there, and without the method the embedded UnimplementedAteomHerderServer answers Unimplemented, which revert treats as a real failure.

RevertActor's unit and functional tests run against a fake atelet, so nothing exercises the verb against a real control plane. This script reverts from CRASHED, RUNNING and PAUSED, and checks that a SUSPENDED actor is rejected. The crash is produced by deleting the worker pod the actor runs on, which is the case that leaves revert with no worker to terminate through. It needs a live cluster and an existing template, so it is not wired into make verify.

Add TestRevertActor_FromPaused and TestRevertActor_FromCrashed to verify RevertActor recovers paused and crashed actors to SUSPENDED at their last external snapshot and allows subsequent resumes

Zoe Zhao (zoez7) · 2026-09-16T22:49:38Z

+	}
+
+	// TODO: Check if local checkpoints need to be pruned #641
+	// w.ensureLocalCheckpointsPruned(leaseCtx, actor)


The local checkpoints should be pruned by ensureWorkerDiscarded -> ensureAteletTerminated above. is that not the case?

Currently, ensureAteletTerminated runs pruneLocalCheckpoints(actorUID) only for RUNNING actors

For PAUSED actors, worker_assignment is nil (

substrate/cmd/ateapi/internal/controlapi/workflow_delete.go

Lines 122 to 125 in 85ce8ed

if assignment == nil {

slog.InfoContext(ctx, "actor has no worker assignment, skipping atlet terminate request", slog.Any("actor", actorRef))

return nil

}

), so ensureAteletTerminated skips.
so, I guessed it will be a separate step itself and get called from deleteActor flow and revertActor flow.

I see, we should be able to call pruneLocalCheckpoints for paused actors as well, after #1742 is submitted, we can do it in a follow up PR.

Zoe Zhao (zoez7) · 2026-09-16T23:16:44Z

+		return nil, nil, fmt.Errorf("while fetching actor: %w", err)
+	}
+
+	actorTemplate, err := resolveActorTemplate(ctx, w.store, actor)


I think there is an issue with the sequence:

suspended actor on actor template v1 , take a full snapshot, actor.status.current_actor_template_uid == v1

UpdateActor to v2, ResumeActor, now current_actor_template_uid updated to v2

Call RevertActor, now the snapshot is reverted to the one taken on v1, but the actor's current_actor_template_uid is still v2.

Can we move the actor_template_uid as a field of ExternalSnapshot, so the revert will take effect together?

handled this case in a follow up PR - #1713

…iscarded Align function, trace span, and test names with revert semantics of discarding rather than collecting in-progress snapshots, and clarify local checkpoint pruning behavior during revert.

The in-progress snapshot is now recorded as a full URI rather than a name derived from the actor's template, so revert reads the URI directly and no longer needs the template to find the objects. Check that the URI is owned by the actor before deleting its prefix. Derivation used to make that structural; a stored URI can name anything, and this step deletes what it is pointed at.

Adding RevertActor left both unupdated. releaseBoundActor labelled the crash metric with the unknown operation, hiding that a revert was in flight when the worker pod went away. The golden snapshot reconciler fell through to the default branch and requeued forever instead of failing the template. Nothing in the golden flow reverts its actor, so that state means someone else interfered.

shrutiyam-glitch · 2026-09-17T16:52:20Z

The tests are failing because of atelet panic - documented here - #1710.
Temporary fix for this - #1684

Shruti Nair (SHRUTI6991) · 2026-09-17T19:52:13Z

The FGA part looks good to me.

Zoe Zhao (zoez7) · 2026-09-18T15:42:07Z

+	}
+
+	// TODO: Check if local checkpoints need to be pruned #641
+	// w.ensureLocalCheckpointsPruned(leaseCtx, actor)


I see, we should be able to call pruneLocalCheckpoints for paused actors as well, after #1742 is submitted, we can do it in a follow up PR.

Zoe Zhao (zoez7) · 2026-09-18T15:44:44Z

+	ctx, done := stepSpan(ctx, "FinalizeReverted")
+	defer func() { err = done(err) }()
+
+	latestActor, err := w.store.GetActor(ctx, actorRef)


check the latestActor is in Reverting state first?

done in 30870b7

Zoe Zhao (zoez7) · 2026-09-18T15:46:09Z

+	ctx, done := stepSpan(ctx, "DiscardWorker")
+	defer func() { err = done(err) }()
+
+	if actor.GetStatus().GetWorkerAssignment() != nil {


We should check if the worker is still assigned to the same Actor.

done in eaf34c5

Zoe Zhao (zoez7) · 2026-09-18T15:53:41Z

+	}
+	// A suspend records the in-progress URI under the actor's own prefix
+	// before atelet writes the first object, so a URI owned by anything else
+	// is a corrupted record: deleting it would collect another actor's data.


Suggested change

// is a corrupted record: deleting it would collect another actor's data.

// is a corrupted record.

It shouldn't be possible that another actor owns this snapshot.

shrutiyam-glitch added 4 commits September 15, 2026 17:10

Add RevertActor RPC

1145d99

Add the revertActtor workflow and the observability metrics

dcb1fae

shrutiyam-glitch changed the title ~~Revertrpc 1~~ feat(api): introduce RevertActor lifecycle RPC Sep 16, 2026

Merge branch 'main' into revertrpc_1

8c22a30

shrutiyam-glitch requested a review from Zoe Zhao (zoez7) September 16, 2026 16:58

shrutiyam-glitch marked this pull request as ready for review September 16, 2026 19:10

shrutiyam-glitch requested a review from Julian Gutierrez Oschmann (juli4n) September 16, 2026 19:13

Fix test

7200dde

shrutiyam-glitch force-pushed the revertrpc_1 branch from f38cc9a to 7200dde Compare September 16, 2026 19:25

Add e2e test

c23daea

Zoe Zhao (zoez7) reviewed Sep 16, 2026

View reviewed changes

Comment thread cmd/ateapi/internal/controlapi/functionaltest/actor_test.go

Comment thread hack/verify-revert-actor.sh Outdated

Add RevertActor functional tests for PAUSED and CRASHED actors

a66fe23

Add TestRevertActor_FromPaused and TestRevertActor_FromCrashed to verify RevertActor recovers paused and crashed actors to SUSPENDED at their last external snapshot and allows subsequent resumes

Zoe Zhao (zoez7) reviewed Sep 16, 2026

View reviewed changes

shrutiyam-glitch added 4 commits September 16, 2026 17:01

Rename ensureInProgressSnapshotCollected to ensureInProgressSnapshotD…

e0571bd

…iscarded Align function, trace span, and test names with revert semantics of discarding rather than collecting in-progress snapshots, and clarify local checkpoint pruning behavior during revert.

Merge branch 'main' into revertrpc_1

1bba074

shrutiyam-glitch mentioned this pull request Sep 17, 2026

docs: Document RevertActor and drop "terminal" from CRASHED #1711

Open

2 tasks

Merge branch 'main' into revertrpc_1

217cd1f

shrutiyam-glitch mentioned this pull request Sep 17, 2026

feat(ateapi): track actor_template_uid per snapshot on ExternalSnapshot #1713

Open

2 tasks

remove test script

02c938e

Bowei Du (bowei) added kind/feature An enhancement / feature request or implementation area/api User-facing API changes area/api-machinery labels Sep 17, 2026

Zoe Zhao (zoez7) reviewed Sep 18, 2026

View reviewed changes

Comment thread cmd/ateapi/internal/controlapi/workflow_revert.go

shrutiyam-glitch added 3 commits September 18, 2026 09:47

check workerHostsActor before discarding worker in RevertActor

eaf34c5

verify actor is REVERTING before finalizing revert

30870b7

Merge branch 'main' into revertrpc_1

4c0c270

shrutiyam-glitch requested a review from Zoe Zhao (zoez7) September 18, 2026 18:15

Zoe Zhao (zoez7) approved these changes Sep 18, 2026

View reviewed changes

Zoe Zhao (zoez7) added this pull request to the merge queue Sep 18, 2026

Merged via the queue into agent-substrate:main with commit 944abe3 Sep 18, 2026
11 checks passed

	if assignment == nil {
	slog.InfoContext(ctx, "actor has no worker assignment, skipping atlet terminate request", slog.Any("actor", actorRef))
	return nil
	}

	// is a corrupted record: deleting it would collect another actor's data.
	// is a corrupted record.

Conversation

shrutiyam-glitch commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commit-wise Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shrutiyam-glitch commented Sep 17, 2026

Uh oh!

Shruti Nair (SHRUTI6991) commented Sep 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shrutiyam-glitch commented Sep 16, 2026 •

edited

Loading