A resume that moves an actor finishes its snapshot upload even if the request times out - #2940

🤖 This handoff (mu, callerLeft, done, the double select, and logDetachAfterCallerLeft) and finishDetach can be simplified:

1. Log in logOp instead of logDetachAfterCallerLeft:

   • The only reason Detach tracks callerLeft under mu with a second select is because logOp (background.go:104) has case !background: that skips logging when a foreground Detach fails (even though logOp already logs "Snapshot operation finished" when a foreground Detach succeeds with background=false).
   • Also, in production UploadLocalSnapshot uploads via the object-store gRPC plugin (s.uploadSnapshot), which on context cancellation returns a wrapped status.Error(codes.Canceled, ...). errors.Is(err, context.Canceled) is false for gRPC status errors, and logDetachAfterCallerLeft runs after endOp has already canceled opCtx, so an outlived detach canceled by Terminate (Forget) or atelet shutdown during a plugin upload will log at WARN instead of INFO (and without snapshot, generation, and elapsed).
   • Removing case !background: from logOp (or checking !background && callerCtx.Err() == nil in logOp before endOp cancels opCtx) handles all detach logging in one place and eliminates mu, callerLeft, the second select, and logDetachAfterCallerLeft.

2. Reuse op.done instead of a second result channel and closure:

   • beginOpLocked already allocates op := &actorOp{..., done: make(chan struct{})}, and finishDetach closes op.done on return after endOp.
   • If actorOp stores state *ateletpb.ActorSnapshotState and err error (populated before close(op.done)), Detach shrinks to:

     op, run, err := t.beginDetachLocked(t.ctx, a, false)
     if err != nil {
         t.foregroundMu.Unlock()
         t.unlock(actorUID)
         return nil, err
     }
     t.workersDone.Go(run)
     t.foregroundMu.Unlock()
     t.unlock(actorUID)
     
     select {
     case <-op.done:
         return op.state, t.detachCallError(ctx, op.err)
     case <-ctx.Done():
         return nil, ctx.Err()
     }

3. Merge the back-to-back updateOp + endOp lock acquisitions at the end of finishDetach (detach.go:192-210):

   • At the end of finishDetach, it locks the actor via t.updateOp (lines 192–202), unlocks to call logOp and recordDetach (lines 205–209), and immediately re-locks the actor via t.endOp (line 210, whose only caller is finishDetach).
   • Matching backgroundUpload (upload.go:124-165), finishDetach can call logOp and recordDetach first (while opCtx is still live), then lock actorUID once to call endOpLocked, update/save a, requeueLocked, and queueReportLocked.

🤖 Done for 1 and 2. logOp now logs a failed foreground detach too, at a level set by the op's context, so one that Terminate cancels logs at Info. The detach returns its result on op.done, and the mutex, the second select, and the separate log are gone.

For 3, I'd send the lock merge as its own PR, since it restructures finishDetach as it was before this change. Does that work for you?

Yes, sending the lock merge in finishDetach as its own PR works for me.

🤖 Thanks. I'll send it once this merges.

Yes, sending the lock merge in finishDetach as its own PR works for me.