fix(backups): verify pause and resume against the original game process

agent/cmd/main.go

@@ -138,36 +138,39 @@ func main() {
 		modsSpec = capSpec.Mods
 	}
 
-	var rconClient interface {
-		Exec(cmd string) (string, error)
-	}
+	var transport rcon.Executor
 	switch {
 	case !rconEnabled:
 		// The game declares no RCON (e.g. consoleMode pty/none). Dialing
 		// 127.0.0.1:25575 would just fail on every request; the Disabled
 		// client makes that explicit so handlers degrade cleanly.
-		rconClient = rcon.Disabled{}
+		transport = rcon.Disabled{}
 	case strings.EqualFold(rconProtocol, "telnet"):
-		rconClient = rcon.NewTelnet(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
+		transport = rcon.NewTelnet(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
 	case strings.EqualFold(rconProtocol, "websocket"):
-		rconClient = rcon.NewWebSocket(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
+		transport = rcon.NewWebSocket(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
 	case strings.EqualFold(rconProtocol, "battleye"):
-		rconClient = rcon.NewBattlEye(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
+		transport = rcon.NewBattlEye(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
 	case strings.EqualFold(rconProtocol, "satisfactory"):
-		rconClient = rcon.NewSatisfactory(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
+		transport = rcon.NewSatisfactory(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
 	case strings.EqualFold(rconProtocol, "palworld"):
-		rconClient = rcon.NewPalworld(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
+		transport = rcon.NewPalworld(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
 	case strings.EqualFold(rconProtocol, "nuclearoption"):
-		rconClient = rcon.NewNuclearOption(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
+		transport = rcon.NewNuclearOption(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
 	case strings.EqualFold(rconProtocol, "rest"):
-		rconClient = rcon.NewREST(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile), rcon.WithGame(gameName))
+		transport = rcon.NewREST(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile), rcon.WithGame(gameName))
 	case strings.EqualFold(rconProtocol, "cli"):
-		rconClient = rcon.NewCLI(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile), rcon.WithPipePath(cliPipe))
+		transport = rcon.NewCLI(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile), rcon.WithPipePath(cliPipe))
 	default:
 		// "source", empty, or anything unrecognized: back-compat default.
-		rconClient = rcon.New(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
+		transport = rcon.New(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile))
 	}
 
+	commands := rcon.NewCommands(transport)
+	rconClient := transport
+	if quiesceSpec != nil {
+		rconClient = commands
+	}
 	r := chi.NewRouter()
 	r.Use(middleware.RequestID, middleware.Recoverer)
 
@@ -197,7 +200,7 @@ func main() {
 		protected.Group(func(bounded chi.Router) {
 			bounded.Use(middleware.Timeout(30 * time.Second))
 			players.Mount(bounded, rconClient, gameName, playerActions)
-			quiesce.Mount(bounded, rconClient, gameName, quiesceSpec)
+			quiesce.Mount(bounded, commands, gameName, quiesceSpec)
 			lifecycle.Mount(bounded, rconClient, gameName, lifecycleSpec)
 			actions.Mount(bounded, rconClient, gameName, actionSpecs)
 			statusSource.Mount(bounded)
@@ -213,11 +216,17 @@ func main() {
 	})
 	// Backups bind save-off/save-on to the process they prepared. The service
 	// may already route to a replacement pod, which must reject the old UID.
-	r.Route("/v1/pods/{uid}", func(protected chi.Router) {
-		protected.Use(authCheck.Middleware, target.Guard(envOr("GAMEPLANE_POD_UID", "")), middleware.Timeout(30*time.Second))
-		protected.Get("/identity", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) })
-		quiesce.Mount(protected, rconClient, gameName, quiesceSpec)
-	})
+	for _, version := range []string{"v1", "v2"} {
+		r.Route("/"+version+"/pods/{uid}", func(protected chi.Router) {
+			protected.Use(authCheck.Middleware, target.Guard(envOr("GAMEPLANE_POD_UID", "")), middleware.Timeout(30*time.Second))
+			protected.Get("/identity", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) })
+			if version == "v2" {
+				quiesce.MountVerified(protected, commands, gameName, quiesceSpec)
+			} else {
+				quiesce.Mount(protected, commands, gameName, quiesceSpec)
+			}
+		})
+	}
 
 	srv := &http.Server{
 		Addr:              addr,

agent/internal/quiesce/quiesce.go

@@ -5,28 +5,29 @@
 // The command sequences are per-game and declared by the module's
 // template (spec.capabilities.quiesce), e.g. Minecraft toggles
 // auto-save with "save-off" + "save-all flush" / "save-on" over RCON.
-// Games that declare nothing get a best-effort no-op so the backup
-// pipeline can proceed unconditionally — backups should never fail
-// because a game can't be paused.
+// A verified pause reports unsupported before issuing any RCON command when
+// the agent cannot check the declared sequence. The operator decides whether
+// that permits a backup to proceed based on any earlier pause attempt.
 package quiesce
 
 import (
 	"fmt"
 	"log/slog"
 	"net/http"
 	"regexp"
+	"slices"
+	"strings"
 
 	"github.com/go-chi/chi/v5"
 
 	"github.com/GameplanePanel/gameplane/agent/internal/caps"
 	"github.com/GameplanePanel/gameplane/agent/internal/httpjson"
+	"github.com/GameplanePanel/gameplane/agent/internal/rcon"
 )
 
 // Rcon is the slice of *rcon.Client we actually use. Defined here as
 // an interface so tests can swap in a recording fake.
-type Rcon interface {
-	Exec(cmd string) (string, error)
-}
+type Rcon = rcon.Executor
 
 // Quiescer encapsulates the game-specific RCON sequence for pausing
 // writes (Quiesce) and resuming them (Unquiesce). Implementations
@@ -39,11 +40,8 @@ type Quiescer interface {
 	Unquiesce(rc Rcon) error
 }
 
-// Pick builds the quiescer from the module's declared sequences
-// (spec.capabilities.quiesce). A template that declares no sequence (or
-// only half of one) gets a no-op quiescer: backups proceed
-// unconditionally and we never pause a game we can't resume. Quiesce is
-// module-driven, with no per-game special-casing in the agent.
+// Pick builds the legacy quiescer from spec.capabilities.quiesce. It reports
+// unsupported unless the template supplies both a pause and a resume sequence.
 func Pick(spec *caps.Quiesce) Quiescer {
 	if spec != nil && len(spec.Quiesce) > 0 && len(spec.Unquiesce) > 0 {
 		return newDeclaredQuiescer(spec)
@@ -53,54 +51,99 @@ func Pick(spec *caps.Quiesce) Quiescer {
 
 type response struct {
 	Quiesced bool   `json:"quiesced"`
+	Outcome  string `json:"outcome,omitempty"`
 	Reason   string `json:"reason,omitempty"`
 }
 
-// Mount registers POST /quiesce and POST /unquiesce on r.
-func Mount(r chi.Router, rc Rcon, game string, spec *caps.Quiesce) {
-	q := Pick(spec)
-	r.Post("/quiesce", func(w http.ResponseWriter, _ *http.Request) {
-		if !q.Supported() {
-			httpjson.Write(w, http.StatusOK, response{Quiesced: false, Reason: "game does not support quiesce"})
-			return
-		}
-		if err := q.Quiesce(rc); err != nil {
-			slog.Warn("quiesce failed", "game", game, "err", err)
-			httpjson.Write(w, http.StatusBadGateway, response{Quiesced: false, Reason: "rcon error"})
-			return
-		}
-		httpjson.Write(w, http.StatusOK, response{Quiesced: true})
-	})
-	r.Post("/unquiesce", func(w http.ResponseWriter, _ *http.Request) {
-		if !q.Supported() {
-			httpjson.Write(w, http.StatusOK, response{Quiesced: false, Reason: "game does not support quiesce"})
-			return
-		}
-		if err := q.Unquiesce(rc); err != nil {
-			// Unquiesce failure is more dangerous than quiesce failure
-			// (the game is now stuck with auto-save off), but we still
-			// must not block the controller — surface 502 so the
-			// operator can record an event.
-			slog.Warn("unquiesce failed", "game", game, "err", err)
-			httpjson.Write(w, http.StatusBadGateway, response{Quiesced: true, Reason: "unquiesce rcon error"})
-			return
-		}
-		httpjson.Write(w, http.StatusOK, response{Quiesced: false})
-	})
+// Mount preserves the legacy declaration-driven endpoints for older clients.
+func Mount(r chi.Router, rc *rcon.Commands, game string, spec *caps.Quiesce) {
+	mount(r, rc, game, Pick(spec), false)
+}
+
+// MountVerified serves the verified Minecraft Java save-off, save-all flush and
+// save-on sequence. An unsupported response means this request sent no command.
+// A previous request may still have paused the game before losing its reply.
+func MountVerified(r chi.Router, rc *rcon.Commands, game string, spec *caps.Quiesce) {
+	var q Quiescer = unsupportedQuiescer{}
+	if game == "minecraft-java" && spec != nil &&
+		slices.Equal(spec.Quiesce, []string{"save-off", "save-all flush"}) &&
+		slices.Equal(spec.Unquiesce, []string{"save-on"}) {
+		declared := newDeclaredQuiescer(spec)
+		declared.rollbackFirst = true
+		q = declared
+	}
+	mount(r, rc, game, q, true)
+}
+
+func mount(r chi.Router, rc *rcon.Commands, game string, q Quiescer, verified bool) {
+	handle := func(path, outcome string, paused bool, run func(Rcon) error) {
+		r.Post(path, func(w http.ResponseWriter, req *http.Request) {
+			if !q.Supported() {
+				httpjson.Write(w, http.StatusOK, response{Outcome: "unsupported", Reason: "game does not support quiesce"})
+				return
+			}
+			err := rc.Sequence(req.Context(), func(transport rcon.Executor) error {
+				if verified {
+					transport = minecraftReplies{transport}
+				}
+				return run(transport)
+			})
+			if err != nil {
+				slog.Warn("backup command failed", "path", path, "game", game, "err", err)
+				httpjson.Write(w, http.StatusBadGateway, response{Quiesced: !paused, Reason: "backup command failed"})
+				return
+			}
+			ack := response{Quiesced: paused}
+			if verified {
+				ack.Outcome = outcome
+			}
+			httpjson.Write(w, http.StatusOK, ack)
+		})
+	}
+	handle("/quiesce", "paused", true, q.Quiesce)
+	handle("/unquiesce", "resumed", false, q.Unquiesce)
+}
+
+type minecraftReplies struct{ Rcon }
+
+// Vanilla Java 1.21.4 en_us commands.save.* messages. RCON can concatenate
+// the two flush messages with no separator. Never accept a success substring
+// embedded in an error, or a reply to the opposite command.
+func (m minecraftReplies) Exec(cmd string) (string, error) {
+	reply, err := m.Rcon.Exec(cmd)
+	if err != nil {
+		return "", err
+	}
+	out := strings.TrimSpace(reply)
+	var ok bool
+	switch cmd {
+	case "save-off":
+		ok = out == "Automatic saving is now disabled" || out == "Saving is already turned off"
+	case "save-on":
+		ok = out == "Automatic saving is now enabled" || out == "Saving is already turned on"
+	case "save-all flush":
+		out = strings.TrimSpace(strings.TrimPrefix(out, "Saving the game (this may take a moment!)"))
+		ok = out == "Saved the game"
+	}
+	if !ok {
+		return "", fmt.Errorf("game did not acknowledge %q", cmd)
+	}
+	return reply, nil
 }
 
 // --- Declared (module-driven) ------------------------------------------
 
 type declaredQuiescer struct {
-	quiesce   []string
-	unquiesce []string
-	failRE    *regexp.Regexp
+	quiesce       []string
+	unquiesce     []string
+	failRE        *regexp.Regexp
+	rollbackFirst bool
 }
 
 // newDeclaredQuiescer compiles the declared sequences. An invalid
 // FailurePattern is logged and ignored — the commands still run, the
 // output check just can't.
-func newDeclaredQuiescer(spec *caps.Quiesce) Quiescer {
+func newDeclaredQuiescer(spec *caps.Quiesce) declaredQuiescer {
 	q := declaredQuiescer{quiesce: spec.Quiesce, unquiesce: spec.Unquiesce}
 	if spec.FailurePattern != "" {
 		re, err := regexp.Compile("(?i)" + spec.FailurePattern)
@@ -122,10 +165,9 @@ func (q declaredQuiescer) Quiesce(rc Rcon) error {
 			err = fmt.Errorf("quiesce command %q reported failure", cmd)
 		}
 		if err != nil {
-			// Roll auto-save (or whatever the game paused) back on so we
-			// don't leave the world frozen — but only if any pausing
-			// command already ran.
-			if i > 0 {
+			// A timed-out save-off may already have reached the game.
+			// The verified save-on contract accepts "already on" on retry.
+			if i > 0 || q.rollbackFirst {
 				_ = q.Unquiesce(rc)
 			}
 			return err
@@ -137,7 +179,11 @@ func (q declaredQuiescer) Quiesce(rc Rcon) error {
 func (q declaredQuiescer) Unquiesce(rc Rcon) error {
 	var firstErr error
 	for _, cmd := range q.unquiesce {
-		if _, err := rc.Exec(cmd); err != nil && firstErr == nil {
+		out, err := rc.Exec(cmd)
+		if err == nil && q.failRE != nil && q.failRE.MatchString(out) {
+			err = fmt.Errorf("unquiesce command %q reported failure", cmd)
+		}
+		if err != nil && firstErr == nil {
 			firstErr = err
 		}
 	}

agent/internal/quiesce/quiesce_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/go-chi/chi/v5"
 
 	"github.com/GameplanePanel/gameplane/agent/internal/caps"
+	"github.com/GameplanePanel/gameplane/agent/internal/rcon"
 	"github.com/GameplanePanel/gameplane/agent/internal/target"
 )
 
@@ -83,7 +84,7 @@ func TestUnsupportedQuiescerNoOp(t *testing.T) {
 
 func newTestRouter(rc Rcon, spec *caps.Quiesce) *chi.Mux {
 	r := chi.NewRouter()
-	Mount(r, rc, "testgame", spec)
+	Mount(r, rcon.NewCommands(rc), "testgame", spec)
 	return r
 }
 
@@ -94,7 +95,7 @@ func TestPodBoundRoutesRejectReplacementBeforeRCON(t *testing.T) {
 			r := chi.NewRouter()
 			r.Route("/v1/pods/{uid}", func(bound chi.Router) {
 				bound.Use(target.Guard(actual))
-				Mount(bound, rc, "test", minecraftQuiesceSpec())
+				Mount(bound, rcon.NewCommands(rc), "test", minecraftQuiesceSpec())
 			})
 			for _, path := range []string{"quiesce", "unquiesce"} {
 				response := httptest.NewRecorder()