Make the deferred-request acknowledgement opt-in, and distinct

The acknowledgement added in "Reply to every debugger request" broke two cases
Nemoumbra pointed out, both of which come down to it reusing the request's own
event name.

A ticketless request is the bad one. {"event":"cpu.resume"} with no ticket drew
an immediate {"event":"cpu.resume"} - byte-identical to the broadcast that fires
when the game actually resumes. A client waiting for that broadcast concluded
the game was running while it was still stopped. Before, it correctly got
nothing until the resume really happened.

input.buttons.press is broken even with a ticket: it answers with the request's
own event name *and* ticket once the button has been held for the requested
frames, so the acknowledgement was indistinguishable from the real completion
and a client resolved on the first of the two. The claim in that commit that
the two are easy to tell apart was simply wrong for this handler.

So the acknowledgement is now off by default - the wire behaviour for every
existing client is exactly what it was - and a client that wants it asks, with
client.config.set {"acknowledgeDeferred": true}. It then arrives as its own
event rather than an echo:

  -> {"event":"cpu.resume","ticket":7}
  <- {"event":"deferred","for":"cpu.resume","ticket":7}
  <- {"event":"cpu.resume"}

which is unambiguous in both cases above. That still gets the original goal -
correlating any request to a reply without hardcoding which events answer
immediately, including ones added later - just without imposing it on clients
that never asked.

Also documents the ticket convention this rests on: send one when you care
about the answer, leave it off to say you aren't waiting. wsdbg followed that
convention badly, silently inserting a ticket into a raw JSON line that
deliberately omitted one; it now sends raw lines exactly as written and simply
doesn't wait on those. It opts into acknowledgements at connect, so --sync
keeps working.

pspautotests 314/314, UnitTest 55/55.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9

Core/Debugger/WebSocket.cpp

@@ -232,13 +232,13 @@ void HandleDebuggerRequest(const http::ServerRequest &request) {
 			eventFunc->second(req);
 			if (!req.Finish()) {
 				// The handler arranged something that finishes later - a step, a resume, a stats
-				// feed - rather than answering now. Acknowledge it anyway, so that *every* request
-				// gets exactly one reply. Without this a client can't tell "accepted, wait for the
-				// event" from "dropped on the floor", and any request/response correlation has to
-				// special-case a list of events that don't answer. The event that actually reports
-				// the result (cpu.stepping, and so on) still follows.
-				req.Respond();
-				req.Finish();
+				// feed - rather than answering now. A client that asked for it gets told so, so it
+				// can tell "accepted, wait for the event" from "dropped on the floor" without
+				// carrying a hardcoded list of the events that don't answer. Everyone else sees
+				// exactly what they saw before; see client.config.set for why it can't be the
+				// default.
+				if (client_info.acknowledgeDeferred)
+					ws->Send(DebuggerDeferredEvent(event, root));
 				// Poll more frequently for a second in case this triggers something.
 				highActivity = 1000;
 			}

Core/Debugger/WebSocket/CPUCoreSubscriber.cpp

@@ -66,8 +66,8 @@ static DebugInterface *CPUFromRequest(DebuggerRequest &req) {
 //
 // No parameters.
 //
-// Response (same event name) with no extra data, acknowledging the request. The CPU may not be
-// stepping yet at that point - a "cpu.stepping" event follows once it is.
+// No immediate response (only a "deferred" event, if the client asked for those via
+// client.config.set).  Once CPU is stepping, a "cpu.stepping" event will be sent.
 void WebSocketCPUStepping(DebuggerRequest &req) {
 	if (!currentDebugMIPS->isAlive()) {
 		return req.Fail("CPU not started");
@@ -84,8 +84,8 @@ void WebSocketCPUStepping(DebuggerRequest &req) {
 //
 // No parameters.
 //
-// Response (same event name) with no extra data, acknowledging the request. A "cpu.resume"
-// event follows once the CPU is actually running again.
+// No immediate response (only a "deferred" event, if the client asked for those via
+// client.config.set).  Once CPU is running again, a "cpu.resume" event will be sent.
 void WebSocketCPUResume(DebuggerRequest &req) {
 	if (!currentDebugMIPS->isAlive()) {
 		return req.Fail("CPU not started");

Core/Debugger/WebSocket/ClientConfigSubscriber.cpp

@@ -22,6 +22,8 @@
 DebuggerSubscriber *WebSocketClientConfigInit(DebuggerEventHandlerMap & map) {
 	map["broadcast.config.get"] = &WebSocketBroadcastConfigGet;
 	map["broadcast.config.set"] = &WebSocketBroadcastConfigSet;
+	map["client.config.get"] = &WebSocketClientConfigGet;
+	map["client.config.set"] = &WebSocketClientConfigSet;
 
 	return nullptr;
 }
@@ -51,6 +53,44 @@ void WebSocketBroadcastConfigGet(DebuggerRequest & req) {
 	json.end();
 }
 
+// Request the current per-connection client settings (client.config.get)
+//
+// No parameters.
+//
+// Response (same event name):
+//  - acknowledgeDeferred: boolean, whether "deferred" events are being sent.
+void WebSocketClientConfigGet(DebuggerRequest &req) {
+	JsonWriter &json = req.Respond();
+	json.writeBool("acknowledgeDeferred", req.client->acknowledgeDeferred);
+}
+
+// Update the per-connection client settings (client.config.set)
+//
+// Parameters (all optional):
+//  - acknowledgeDeferred: boolean, whether to send a "deferred" event when a request is accepted
+//    but only finishes later (cpu.resume, cpu.stepInto and friends, gpu.stats.feed,
+//    input.buttons.press.)  Defaults to false.
+//
+// Response (same event name):
+//  - acknowledgeDeferred: boolean, the setting as it now stands.
+//
+// Turning this on lets a client tell "accepted, the result comes in a later event" from "dropped
+// on the floor", without hardcoding which events those are.  It can't be the default, and can't
+// simply reuse the request's own event name, because that's what the later event already uses:
+// a client waiting for a bare {"event":"cpu.resume"} would believe the game was running while it
+// was still stopped, and input.buttons.press answers with the request's own ticket when the press
+// completes, so an early reply under that name would be indistinguishable even with a ticket.
+void WebSocketClientConfigSet(DebuggerRequest &req) {
+	JsonWriter &json = req.Respond();
+
+	bool acknowledgeDeferred = req.client->acknowledgeDeferred;
+	if (!req.ParamBool("acknowledgeDeferred", &acknowledgeDeferred, DebuggerParamType::OPTIONAL))
+		return;
+	req.client->acknowledgeDeferred = acknowledgeDeferred;
+
+	json.writeBool("acknowledgeDeferred", req.client->acknowledgeDeferred);
+}
+
 // Update the current client broadcast configuration (broadcast.config.set)
 //
 // Parameters:

Core/Debugger/WebSocket/ClientConfigSubscriber.h

@@ -23,3 +23,5 @@ DebuggerSubscriber *WebSocketClientConfigInit(DebuggerEventHandlerMap &map);
 
 void WebSocketBroadcastConfigSet(DebuggerRequest &req);
 void WebSocketBroadcastConfigGet(DebuggerRequest &req);
+void WebSocketClientConfigSet(DebuggerRequest &req);
+void WebSocketClientConfigGet(DebuggerRequest &req);

Core/Debugger/WebSocket/GPUStatsSubscriber.cpp

@@ -175,8 +175,8 @@ void WebSocketGPUStatsState::Get(DebuggerRequest &req) {
 // Parameters:
 //  - enable: optional boolean, pass false to stop the feed.
 //
-// Response (same event name) with no extra data, acknowledging the request. Stats events then
-// arrive each frame (as gpu.stats.get.)
+// No immediate response (only a "deferred" event, if the client asked for those via
+// client.config.set).  Events sent each frame (as gpu.stats.get.)
 //
 // Note: info and timing will be accurate after the first frame.
 void WebSocketGPUStatsState::Feed(DebuggerRequest &req) {

Core/Debugger/WebSocket/SteppingSubscriber.cpp

@@ -85,8 +85,8 @@ static DebugInterface *CPUFromRequest(DebuggerRequest &req, uint32_t *threadID =
 // Parameters:
 //  - thread: optional number indicating the thread id to plan stepping on.
 //
-// Response (same event name) with no extra data on success. A cpu.stepping event follows once
-// the step completes.
+// No immediate response on success (only a "deferred" event, if the client asked for those via
+// client.config.set).  A cpu.stepping event will be sent once complete.
 // May fail (same-thread case only) if too many steps are already queued, which means a client is
 // firing them faster than they can possibly be carried out.
 //
@@ -144,7 +144,8 @@ void WebSocketSteppingState::Into(DebuggerRequest &req) {
 // Parameters:
 //  - thread: optional number indicating the thread id to plan stepping on.
 //
-// Response (same event name) with no extra data. A cpu.stepping event follows once complete.
+// No immediate response (only a "deferred" event, if the client asked for those via
+// client.config.set).  A cpu.stepping event will be sent once complete.
 //
 // Note: any thread can wake the cpu when it hits the next instruction currently.
 void WebSocketSteppingState::Over(DebuggerRequest &req) {
@@ -199,7 +200,8 @@ void WebSocketSteppingState::Over(DebuggerRequest &req) {
 // Parameters:
 //  - thread: optional number indicating the thread id to plan stepping on.
 //
-// Response (same event name) with no extra data. A cpu.stepping event follows once complete.
+// No immediate response (only a "deferred" event, if the client asked for those via
+// client.config.set).  A cpu.stepping event will be sent once complete.
 //
 // Note: any thread can wake the cpu when it hits the next instruction currently.
 void WebSocketSteppingState::Out(DebuggerRequest &req) {
@@ -252,7 +254,8 @@ void WebSocketSteppingState::Out(DebuggerRequest &req) {
 // Parameters:
 //  - address: number parameter for destination.
 //
-// Response (same event name) with no extra data. A cpu.stepping event follows once complete.
+// No immediate response (only a "deferred" event, if the client asked for those via
+// client.config.set).  A cpu.stepping event will be sent once complete.
 void WebSocketSteppingState::RunUntil(DebuggerRequest &req) {
 	if (!currentDebugMIPS->isAlive()) {
 		return req.Fail("CPU not started");
@@ -337,7 +340,8 @@ void WebSocketSteppingState::RunUntilTime(DebuggerRequest &req) {
 //
 // No parameters.
 //
-// Response (same event name) with no extra data. A cpu.stepping event follows once complete.
+// No immediate response (only a "deferred" event, if the client asked for those via
+// client.config.set).  A cpu.stepping event will be sent once complete.
 void WebSocketSteppingState::HLE(DebuggerRequest &req) {
 	if (!currentDebugMIPS->isAlive()) {
 		return req.Fail("CPU not started");

Core/Debugger/WebSocket/WebSocketUtils.h

@@ -39,6 +39,38 @@ struct WebSocketClientInfo {
 	std::string name;
 	std::string version;
 	std::map<std::string, bool> disallowed;
+	// Whether to send a "deferred" event for requests that only finish later. Off by default,
+	// and it has to stay that way: an extra message would break a client that correlates purely
+	// by ticket, or one that waits for the bare event name a resume or a button press answers
+	// with. See client.config.set.
+	bool acknowledgeDeferred = false;
+};
+
+// Sent, if the client asked for it, when a handler accepted a request but arranged to finish it
+// later - a resume, a step, a button held for some frames. Deliberately not named after the
+// request: the event that reports the actual outcome uses that name, and for input.buttons.press
+// it carries the same ticket too, so anything less distinct would be impossible to tell apart.
+struct DebuggerDeferredEvent {
+	DebuggerDeferredEvent(const char *n, const JsonGet data) : name(n) {
+		const JsonNode *value = data ? data.get("ticket") : nullptr;
+		if (value)
+			ticketRaw = json_stringify(value);
+	}
+
+	const char *name;
+	std::string ticketRaw;
+
+	operator std::string() const {
+		JsonWriter j;
+		j.begin();
+		j.writeString("event", "deferred");
+		j.writeString("for", name);
+		if (!ticketRaw.empty()) {
+			j.writeRaw("ticket", ticketRaw);
+		}
+		j.end();
+		return j.str();
+	}
 };
 
 struct DebuggerErrorEvent {

Tools/wsdbg/src/main.rs

@@ -224,6 +224,40 @@ fn connect(host: &str, port: u16) -> Result<WebSocket<TcpStream>> {
     Ok(socket)
 }
 
+// Sent once per connection, before anything else, and its reply swallowed rather than printed -
+// it's plumbing the user didn't ask for and shouldn't have to read past.
+fn request_deferred_acks(socket: &mut WebSocket<TcpStream>) -> Result<()> {
+    let ticket = next_ticket();
+    socket.send(Message::Text(
+        serde_json::json!({
+            "event": "client.config.set",
+            "acknowledgeDeferred": true,
+            "ticket": ticket,
+        })
+        .to_string()
+        .into(),
+    ))?;
+
+    socket.get_ref().set_read_timeout(Some(Duration::from_millis(50)))?;
+    let deadline = Instant::now() + Duration::from_secs(2);
+    while Instant::now() < deadline {
+        match socket.read() {
+            Ok(Message::Text(t)) => {
+                let v = serde_json::from_str::<serde_json::Value>(&t).unwrap_or_default();
+                if v.get("ticket").and_then(|t| t.as_u64()) == Some(ticket) {
+                    return Ok(());
+                }
+                // Anything else this early is a broadcast we'd have printed anyway.
+                print_incoming(&t);
+            }
+            Ok(_) => {}
+            Err(e) if is_would_block(&e) => {}
+            Err(e) => return Err(e.into()),
+        }
+    }
+    Ok(())
+}
+
 fn print_incoming(text: &str) {
     let parsed = serde_json::from_str::<serde_json::Value>(text);
     if compact() {
@@ -334,9 +368,10 @@ fn is_would_block(e: &tungstenite::Error) -> bool {
 // Returns false if the request went unanswered within the timeout, so one-shot mode can exit
 // non-zero rather than looking successful after having printed nothing useful.
 //
-// Every request is answered (PPSSPP acknowledges even the ones whose result arrives later), so
-// waiting for this request's ticket beats sleeping for a fixed --wait: a quick question returns
-// immediately instead of padding every invocation, and a slow one isn't cut off early.
+// With deferred acks turned on (see request_deferred_acks) every request is answered, including
+// the ones whose result arrives later, so waiting for this request's ticket beats sleeping for a
+// fixed --wait: a quick question returns immediately instead of padding every invocation, and a
+// slow one isn't cut off early.
 fn run_one_shot(
     mut socket: WebSocket<TcpStream>,
     json_text: String,
@@ -394,14 +429,16 @@ fn print_help() {
 fn handle_repl_line(socket: &mut WebSocket<TcpStream>, line: &str) -> Result<Option<(u64, String)>> {
     if line.starts_with('{') {
         // A raw JSON line is the only way to send nested parameters, so it can't be a
-        // second-class citizen. Reject it outright if it isn't valid JSON or has no event, and
-        // add a ticket when it doesn't carry one - without a ticket --sync has nothing to match
-        // on, so it used to skip waiting entirely and let the next line's response be attributed
-        // to this one, silently desynchronising the rest of the script.
-        let mut parsed: serde_json::Value =
+        // second-class citizen. Reject it outright if it isn't valid JSON or has no event - but
+        // send it exactly as written otherwise. Omitting the ticket is the documented way to say
+        // "I'm not waiting for an answer", so quietly inserting one would send something the
+        // author didn't write. --sync simply doesn't wait on such a line and moves on to the next
+        // (it must not wait for "the next message that happens to arrive" and attribute that -
+        // that's what desynchronised the rest of a script before tickets were matched properly).
+        let parsed: serde_json::Value =
             serde_json::from_str(line).map_err(|e| anyhow!("Not valid JSON: {e}"))?;
         let obj = parsed
-            .as_object_mut()
+            .as_object()
             .ok_or_else(|| anyhow!("A raw message must be a JSON object"))?;
         let event = obj
             .get("event")
@@ -409,19 +446,18 @@ fn handle_repl_line(socket: &mut WebSocket<TcpStream>, line: &str) -> Result<Opt
             .ok_or_else(|| anyhow!("A raw message needs a string 'event' field"))?
             .to_string();
         let ticket = match obj.get("ticket") {
-            Some(t) => t
-                .as_u64()
-                .ok_or_else(|| anyhow!("'ticket' must be a non-negative integer to be matchable"))?,
-            None => {
-                let t = next_ticket();
-                obj.insert("ticket".to_string(), serde_json::Value::from(t));
-                t
-            }
+            Some(t) => Some(
+                t.as_u64()
+                    .ok_or_else(|| anyhow!("'ticket' must be a non-negative integer to be matchable"))?,
+            ),
+            None => None,
         };
-        let json_text = serde_json::Value::Object(obj.clone()).to_string();
-        println!("-> (ticket {ticket}) {json_text}");
-        socket.send(Message::Text(json_text.into()))?;
-        return Ok(Some((ticket, event)));
+        match ticket {
+            Some(t) => println!("-> (ticket {t}) {line}"),
+            None => println!("-> (no ticket, not waiting for a reply) {line}"),
+        }
+        socket.send(Message::Text(line.to_string().into()))?;
+        return Ok(ticket.map(|t| (t, event)));
     }
 
     let mut parts = split_shell_words(line)?.into_iter();
@@ -759,11 +795,18 @@ fn main() -> Result<()> {
     let args = Args::parse();
     COMPACT.store(args.compact, Ordering::Relaxed);
 
-    let socket = connect(&args.host, args.port).with_context(|| {
+    let mut socket = connect(&args.host, args.port).with_context(|| {
         "Could not connect. Is PPSSPP running with the WebSocket debugger enabled? \
          (Settings > Tools > Developer Tools > Allow remote debugger, or launch with --debugger)"
     })?;
 
+    // Ask to be told when a request was accepted but finishes later (cpu.resume and friends).
+    // Off by default server-side, since the extra message would confuse a client that correlates
+    // purely by ticket - but it's exactly what lets --sync match every request to a reply without
+    // knowing which events answer immediately. Ignore the error from an older PPSSPP that doesn't
+    // know the event; --sync just falls back to timing out on those, as it did before.
+    request_deferred_acks(&mut socket).ok();
+
     if let Some(raw) = &args.raw {
         // Recover the ticket if the caller supplied one; there's nothing to wait on otherwise.
         let ticket = serde_json::from_str::<serde_json::Value>(raw)

docs/WebSocketDebugger.md

@@ -55,15 +55,38 @@ Responses use the *same* event name as the request:
 ```json
 { "event": "cpu.status", "ticket": 1, ... }
 ```
-Responses are not always immediate - some handlers respond asynchronously.
 
-**Every request gets exactly one reply**: either a response, or an `error`. A
-handler whose real result arrives later (`cpu.stepInto`, `cpu.resume`,
-`gpu.stats.feed`, ...) is acknowledged with an empty response carrying your
-ticket, and the event reporting the actual outcome (`cpu.stepping`, ...)
-follows separately. So a client can always correlate request to reply without
-keeping a list of events that don't answer, and "no reply" unambiguously means
-the request is still being processed.
+**The ticket convention**: send one whenever you care about the answer. Since
+a response reuses the request's event name, a ticket is the only thing that
+distinguishes *your* answer from an unsolicited broadcast of the same name, or
+from the answer to an identical request you sent a moment earlier. Conversely,
+for a request that doesn't answer immediately (below), leaving the ticket off
+says you aren't waiting for anything.
+
+Responses are not always immediate. `cpu.resume`, `cpu.stepInto`,
+`cpu.stepOver`, `cpu.stepOut`, `cpu.runUntil`, `cpu.runUntilTime`,
+`cpu.nextHLE`, `cpu.stepping` and `gpu.stats.feed` send nothing back at the
+time of the request; what follows later is the broadcast that reports the
+actual outcome (`cpu.stepping` / `cpu.resume`), which carries no ticket.
+`input.buttons.press` is the odd one out - it answers with the request's own
+event name *and* ticket, but only once the button has been held for the
+requested number of frames.
+
+If you would rather not track which those are, ask to be told explicitly:
+
+```json
+-> { "event": "client.config.set", "acknowledgeDeferred": true }
+-> { "event": "cpu.resume", "ticket": 7 }
+<- { "event": "deferred", "for": "cpu.resume", "ticket": 7 }
+<- { "event": "cpu.resume" }
+```
+
+With that on, every request draws exactly one immediate reply - a response, an
+`error`, or a `deferred` - so a client can correlate without a hardcoded list,
+including for events added in future versions. It is off by default and must
+stay that way: an extra message would break a client that correlates purely by
+ticket, and it can't reuse the request's event name because for
+`input.buttons.press` that is exactly what the real, later answer looks like.
 
 Errors look like this:
 ```json
@@ -104,7 +127,9 @@ Sent without you asking, whenever the underlying state changes:
 A client can opt out of specific broadcast categories with
 `broadcast.config.set` (`{"disallowed": {"logger": true, "game": true, "stepping": true, "input": true}}`),
 see `ClientConfigSubscriber.cpp`. `gpu.stats.feed` (see below) works the same
-way for periodic GPU stats.
+way for periodic GPU stats. `client.config.set` in the same file carries
+per-connection settings that aren't about broadcasts - currently just
+`acknowledgeDeferred`, described under "Message protocol" above.
 
 ## Request/response event catalog
 
@@ -131,7 +156,7 @@ file - this is just an index.
 | GPU buffers | `gpu.buffer.screenshot`, `gpu.buffer.renderColor/renderDepth/renderStencil`, `gpu.buffer.texture`, `gpu.buffer.clut` | `GPUBufferSubscriber.cpp` |
 | Input injection | `input.buttons.send`, `input.buttons.press`, `input.analog.send` | `InputSubscriber.cpp` |
 | Replay | `replay.begin/abort/flush/execute/status`, `replay.time.get/set` | `ReplaySubscriber.cpp` |
-| Client config | `broadcast.config.get/set` | `ClientConfigSubscriber.cpp` |
+| Client config | `broadcast.config.get/set`, `client.config.get/set` | `ClientConfigSubscriber.cpp` |
 | Log channels | `log.channels.list`, `log.channel.set` - query/change a log channel's level (string: `notice`/`error`/`warning`/`info`/`debug`/`verbose`) and/or enabled state; the `log` event itself (the passive message stream, unaffected by this) keeps its existing numeric `level`, see `LogBroadcaster.cpp` | `LogConfigSubscriber.cpp` |
 
 ## Enabling it