Skip to content

Add DialForNode and always finalize in FinalizeSuspendedStep - #813

Merged
Julian Gutierrez Oschmann (juli4n) merged 1 commit into
agent-substrate:mainfrom
dberkov:dial-for-node-and-finalize-refactor
Aug 8, 2026
Merged

Add DialForNode and always finalize in FinalizeSuspendedStep#813
Julian Gutierrez Oschmann (juli4n) merged 1 commit into
agent-substrate:mainfrom
dberkov:dial-for-node-and-finalize-refactor

Conversation

@dberkov

Copy link
Copy Markdown
Collaborator

Part of #791 (PR 3 of the implementation plan).

Two behavior-preserving refactors needed before suspending a PAUSED actor, which has no worker assignment:

Dialer — extract dialAteletOnNode from DialForWorker's tail (byNode index lookup → single-atelet check → per-pod-UID connection cache → pinned-mTLS dial) and expose DialForNode(nodeName) for actors whose state is pinned to a node without a worker pod (a PAUSED actor's local snapshot). A node with no atelet in the informer cache returns the new sentinel ErrNoAteletOnNode — documented as retryable (atelet restarting / informer lag), distinct from the crash-worthy ErrWorkerPodNotFound. DialForWorker behavior is unchanged apart from error wrapping in the zero-atelets case.

FinalizeSuspendedStep — hoist finalization (ActorSnapshot record, LatestSnapshot, the SUSPENDED flip, clearing InProgressSnapshot/WorkerAssignment/LocalSnapshotInfo) out of the if assignment != nil worker-freeing branch so it always runs. Previously, a SUSPENDING actor with no assignment was silently skipped — the workflow reported success while the actor stayed SUSPENDING forever. Unreachable today (CallAteletSuspendStep's prerequisite crashes nil-assignment actors first), but load-bearing for the paused-origin suspend, where no worker exists by design.

Tests:

  • TestDialForNode: no atelet on node → ErrNoAteletOnNode; two atelets → error; happy path asserts pod-UID-pinned credentials and connection caching.
  • TestFinalizeSuspendedStep_NoAssignment: SUSPENDING actor with LocalSnapshotInfo and an in-progress URI but no assignment → SUSPENDED, ActorSnapshot created at the URI, LatestSnapshot set, in-progress fields and LocalSnapshotInfo cleared.

go test -race ./cmd/ateapi/... (incl. the functional suite covering the running-path suspend end-to-end), go vet, gofmt clean. Independent of #810 and #812.

🤖 Generated with Claude Code

Comment thread cmd/ateapi/internal/controlapi/dialer.go Outdated
Two behavior-preserving refactors needed to suspend a PAUSED actor (agent-substrate#791),
which has no worker assignment:

- Dialer: extract dialAteletOnNode from DialForWorker and expose DialForNode
  for actors whose state is pinned to a node without a worker (a PAUSED
  actor's local snapshot). A node with no atelet returns the new retryable
  sentinel ErrNoAteletOnNode, distinct from the crash-worthy
  ErrWorkerPodNotFound.

- FinalizeSuspendedStep: hoist finalization (ActorSnapshot record, SUSPENDED
  flip, clearing in-progress fields) out of the worker-freeing branch so it
  runs even with no assignment. Previously such an actor was silently left
  SUSPENDING while the workflow reported success; unreachable today because
  CallAteletSuspendStep crashes nil-assignment actors first, but load-bearing
  for the paused-origin path.

Part of agent-substrate#791
@dberkov
Dmitry Berkovich (dberkov) force-pushed the dial-for-node-and-finalize-refactor branch from b5814ff to ddb2211 Compare August 8, 2026 19:35
@juli4n

Copy link
Copy Markdown
Collaborator

LGTM. Needs rebase.

@juli4n
Julian Gutierrez Oschmann (juli4n) merged commit 231a9ef into agent-substrate:main Aug 8, 2026
11 checks passed
Haven Xia (HavenXia) pushed a commit that referenced this pull request Aug 11, 2026
Part of #791 — the last planned piece of the [implementation
plan](#791 (comment))
(after #810, #812, #813). #791 stays open until #817 (gVisor
Full-capture → Data-commit conversion, blocked on #790) is done; this PR
covers micro-VM fully and gVisor for scope-matched suspends. Also part
of the actor state machine (#119) and a prerequisite for system upgrade
flows (#473).

`SuspendActor` now accepts a PAUSED actor: instead of checkpointing a
running workload, ateapi dials the atelet on the node holding the pause
snapshot and has it upload the node-local files to object storage, then
finalizes as usual — durable `ActorSnapshot`, `SUSPENDED` status, node
pinning cleared.

Two commits, reviewable independently:

## Commit 1 — atelet: `UploadPausedCheckpoint` RPC (dead code until
commit 2)

- New `AteomHerder` RPC: a pure disk→object-storage copy driven by the
snapshot's self-describing manifest — no ateom involved (the sandbox is
gone).
- **Scope conversion** dispatches per sandbox class
(`narrowFullCaptureToData`): a micro-VM FULL capture narrows to a DATA
upload by carving out `durable-dir.tar` (constant hoisted to
`ateompath`, shared with ateom-microvm); gVisor returns `Unimplemented`
until split checkpoints land (#790); DATA can never widen to FULL; a
scope-less manifest (older atelet) is rejected rather than guessed at.
- **Idempotent retry**: local files gone + remote manifest present ⇒ a
previous invocation committed, succeed; gone on both sides ⇒
unrecoverable (`LOCAL_SNAPSHOT_GONE`, crashes the actor). Upload
failures stay plain retryable errors; the manifest uploads last as the
commit marker, never in parallel.
- The golden atespace is rejected at validation (fully on the
`field.ErrorList` framework): golden actors are never paused.

## Commit 2 — control plane: enable suspend from PAUSED

- `FromPaused` discriminator: PAUSED status, or SUSPENDING with no
worker assignment and a `LocalSnapshotInfo` — the field alone is stale
on resumed-from-pause RUNNING actors, so the nil-assignment conjunct is
load-bearing.
- `MarkSuspendingStep` accepts PAUSED and rejects a Data-captured pause
against a Full commit *before* the actor leaves PAUSED (an upload cannot
fabricate memory), using the `content_scope` recorded at pause (#812)
with an `onPause` fallback.
- `CallAteletSuspendStep` paused branch dials by node
(`DialForAteletOnNode`, #813): missing node record ⇒ crash (the snapshot
can never be found); unreachable atelet ⇒ retryable; atelet's
`LOCAL_SNAPSHOT_GONE` ⇒ crash via `maybeCrashActor`.
`FinalizeSuspendedStep` needed no changes thanks to the #813 hoist.
- Root-cause guard: `MarkPausingStep` rejects pausing golden-atespace
actors.
- Docs: pause states + the new `PAUSED → SUSPENDING` edge in the
architecture state diagram; glossary Suspend entry covers both origins.

## Tests

- **atelet unit**: 10 upload-helper subtests (conversion matrix,
idempotency probe, data-loss crash, retryable upload failure) via a
recording object-storage fake; validation table.
- **control-plane unit**: discriminator table, scope-rejection table
(incl. onPause fallback), paused preconditions (no node ⇒ CRASHED, no
atelet ⇒ `ErrNoAteletOnNode` + still SUSPENDING), golden-pause
rejection, prerequisite matrix updated.
- **functional (envtest)**: `TestSuspendActor_FromPaused` (upload called
with the pause snapshot name, no Checkpoint RPC, SUSPENDED, pinning
cleared, ActorSnapshot at the upload destination) +
retry-after-failed-upload (same destination on retry).
- **e2e (demo suite)**: the lifecycle driver gains a suspend-from-PAUSED
mode; three durable-dir cases — Full/Full, Data/Data, and the micro-VM
Full→Data extraction — assert memory/file counters survive the full
pause→suspend→resume journey and the node pinning is gone.

`go test -race ./...` clean (except the pre-existing macOS-only
`internal/atunnel` unix-socket-path failures, untouched by this PR),
`gofmt`/`go vet` clean, protos regenerated via `hack/protoc.sh`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants