fix: creating a room failed with a blank "Failed to copy room"

The 2023 client sends rooms/{id}/clone through the same RecNet rooms
helper as "save room" and "restore subroom save", which reads the
{success, value, error} envelope. The clone handler answered with the
bare room details, so the client found no success flag and rejected the
reply with an empty error, even though the room had been created.

Clone now returns the envelope with the full room details in value, and
refusals (unknown source, cloning not allowed) are a 200 with
success=false and a message instead of a bodyless 404/403.

DorkNet.Server.Tests/PrivateRoomInstanceTests.cs

@@ -42,7 +42,7 @@ public sealed class PrivateRoomInstanceTests : IClassFixture<DorkNetServerFactor
         var text = await resp.Content.ReadAsStringAsync();
         Assert.True(resp.IsSuccessStatusCode, $"{(int)resp.StatusCode}: {text}");
         using var json = JsonDocument.Parse(text);
-        var cloneId = json.RootElement.GetProperty("RoomId").GetInt64();
+        var cloneId = json.RootElement.GetProperty("value").GetProperty("RoomId").GetInt64();
 
         await using var scope = _factory.Services.CreateAsyncScope();
         var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();

DorkNet.Server.Tests/RoomCloneShapeTests.cs

@@ -10,13 +10,15 @@ using Microsoft.Extensions.DependencyInjection;
 namespace DorkNet.Server.Tests;
 
 /// <summary>
-/// The 2023 "copy room" flow POSTs <c>rooms/{id}/clone</c> and deserialises the
+/// The 2023 "copy room" flow POSTs <c>rooms/{id}/clone</c> and reads the reply
-/// reply into <c>FGCPNAACHIK</c> — the SAME type <c>GET rooms/{id}</c> returns
+/// as the <c>{success, value, error}</c> envelope (the same RecNet rooms helper
-/// (RecNet.Runtime/NLDBPDCNNCF.txt:4676 declares
+/// as "save room"), whose <c>value</c> is <c>FGCPNAACHIK</c> — the SAME type
+/// <c>GET rooms/{id}</c> returns (RecNet.Runtime/NLDBPDCNNCF.txt:4676 declares
 /// <c>Task&lt;FGCPNAACHIK&gt; GDHIIAHCBMN(Int64, String, CancellationToken)</c>,
-/// with the route literal on :4802).
+/// with the route literal on :4802). A bare room with no <c>success</c> is
+/// rejected with an empty error.
 ///
-/// So the two responses must be shape-identical. When they drift, the server
+/// So <c>value</c> and the GET response must be shape-identical. When they drift, the server
 /// still answers 200 and the client throws while reading it — and because the
 /// clone promise's exception is never observed, it surfaces from the finalizer
 /// thread seconds later as a bare "Failed to copy room: Exception of type
@@ -63,8 +65,12 @@ public sealed class RoomCloneShapeTests : IClassFixture<DorkNetServerFactory>
         }
 
         var getBody = await ReadJsonAsync(client, HttpMethod.Get, $"/rooms/{sourceId}");
-        var cloneBody = await ReadJsonAsync(client, HttpMethod.Post, $"/rooms/{sourceId}/clone",
+        var cloneEnvelope = await ReadJsonAsync(client, HttpMethod.Post, $"/rooms/{sourceId}/clone",
             new FormUrlEncodedContent([new KeyValuePair<string, string>("name", $"Copy{Guid.NewGuid():N}"[..20])]));
+        // The client reads clone through the {success, value, error} envelope.
+        Assert.True(cloneEnvelope.GetProperty("success").GetBoolean(), cloneEnvelope.ToString());
+        Assert.Equal("", cloneEnvelope.GetProperty("error").GetString());
+        var cloneBody = cloneEnvelope.GetProperty("value");
 
         var expected = getBody.EnumerateObject().Select(p => p.Name).ToHashSet(StringComparer.Ordinal);
         var actual = cloneBody.EnumerateObject().Select(p => p.Name).ToHashSet(StringComparer.Ordinal);
@@ -157,7 +163,7 @@ public sealed class RoomCloneShapeTests : IClassFixture<DorkNetServerFactory>
 
         var clone = await ReadJsonAsync(client, HttpMethod.Post, $"/rooms/{sourceId}/clone",
             new FormUrlEncodedContent([new KeyValuePair<string, string>("name", $"Copy{Guid.NewGuid():N}"[..20])]));
-        var cloneId = clone.GetProperty("RoomId").GetInt64();
+        var cloneId = clone.GetProperty("value").GetProperty("RoomId").GetInt64();
 
         await using (var scope = factory.Services.CreateAsyncScope())
         {
@@ -203,6 +209,24 @@ public sealed class RoomCloneShapeTests : IClassFixture<DorkNetServerFactory>
             Task.FromResult(_objects.ContainsKey($"{bucket}/{key}"));
     }
 
+    [Fact]
+    public async Task Clone_refusal_is_a_200_envelope_with_a_message()
+    {
+        using var client = Client();
+        var session = await GameClientSessionFactory.CreateAsync(client, _factory.ApexDomain);
+        client.DefaultRequestHeaders.Authorization =
+            new AuthenticationHeaderValue("Bearer", session.AccessToken);
+
+        // No room has this id, so the clone is refused; the client shows
+        // `error`, which a bare 404 would leave empty.
+        var envelope = await ReadJsonAsync(client, HttpMethod.Post, "/rooms/987654321987/clone",
+            new FormUrlEncodedContent([new KeyValuePair<string, string>("name", "NoSuchSource")]));
+
+        Assert.False(envelope.GetProperty("success").GetBoolean());
+        Assert.False(string.IsNullOrEmpty(envelope.GetProperty("error").GetString()));
+        Assert.Equal(JsonValueKind.Null, envelope.GetProperty("value").ValueKind);
+    }
+
     private static async Task<JsonElement> ReadJsonAsync(
         HttpClient client, HttpMethod method, string path, HttpContent? content = null)
     {

DorkNet.Server.Tests/RoomSave2023Tests.cs

@@ -267,8 +267,10 @@ public sealed class RoomSave2023Tests : IClassFixture<DorkNetServerFactory>
         Assert.True(resp.IsSuccessStatusCode, $"clone of {sourceId} -> {(int)resp.StatusCode}: {text}");
 
         using var json = JsonDocument.Parse(text);
-        var root = json.RootElement;
+        // {success, value, error} envelope around the full room-details
-        // Full room-details shape (FGCPNAACHIK), not a status wrapper.
+        // shape (FGCPNAACHIK).
+        Assert.True(json.RootElement.GetProperty("success").GetBoolean(), text);
+        var root = json.RootElement.GetProperty("value");
         foreach (var key in new[] { "RoomId", "SubRooms", "Roles" })
             Assert.True(root.TryGetProperty(key, out _), $"clone response missing '{key}': {text}");
         return root.GetProperty("RoomId").GetInt64();

DorkNet.Server/Controllers/API/Rooms/V2/RoomsModerationController.cs

@@ -1375,13 +1375,13 @@ public class RoomsModerationController(
     {
         var newName = await ReadCloneNameAsync();
         var source = await db.Rooms.FirstOrDefaultAsync(r => r.Id == roomId);
-        if (source is null) return NotFound();
+        if (source is null) return CloneResult(null, "That room doesn't exist.");
         // RRO/AG rooms are first-party templates meant to be cloned — the
         // room-creation "base room" picker starts a new build by cloning one
         // (RecCenter, MakerRoom, …). Their seed rows have CloningAllowed=false,
-        // so without the IsAGRoom exception every base-room create 403s.
+        // so without the IsAGRoom exception every base-room create is refused.
         if (!source.CloningAllowed && !source.IsAGRoom && source.CreatorPlayerId != Me)
-            return Forbid();
+            return CloneResult(null, "You can't clone this room!");
 
         if (string.IsNullOrWhiteSpace(newName))
             newName = $"{source.Name}Copy";
@@ -1517,17 +1517,22 @@ public class RoomsModerationController(
 
         await GiveCloneItsOwnBlobsAsync(clone, sourceScenes, clonedScenes, Me);
 
-        // The 2023 client's clone (RecNet.Runtime NLDBPDCNNCF.GDHIIAHCBMN)
+        // The value is the FULL room-details object (FGCPNAACHIK), the exact
-        // deserializes the response as the FULL room-details object
+        // shape GET rooms/{id} returns for the 2023 client — not the slim
-        // (FGCPNAACHIK — the same type get-by-id / rename return on the
+        // list-room shape, which made the strict reader fail.
-        // roomserver host), NOT the slim list-room shape. This is the
+        return CloneResult(RoomsController.BuildRoomServerDetails(clone, clonedScenes));
-        // roomserver-family path (rooms/{id}/clone), so use
-        // BuildRoomServerDetails — the exact shape GET rooms/{id} returns for
-        // the 2023 client. Returning ToWireRoom (or the wrong details variant)
-        // made the strict reader fail → "Failed to copy room: Failed to clone
-        // room".
-        return Ok(RoomsController.BuildRoomServerDetails(clone, clonedScenes));
     }
+    /// <summary>Reply for <c>rooms/{id}/clone</c>. The 2023 client sends the
+    /// clone through the same RecNet rooms helper as "save room" and "restore
+    /// subroom save" (NLDBPDCNNCF.BDBHCEGKGDC with an op name), which reads the
+    /// legacy <c>{success, value, error}</c> envelope. A bare room-details body
+    /// has no <c>success</c>, so the client rejects it with an empty error:
+    /// "Failed to copy room: Exception of type 'CEMNLBKJABA' was thrown", even
+    /// though the room was created. Refusals are 200 with success=false and a
+    /// message the client shows.</summary>
+    private OkObjectResult CloneResult(object? room, string error = "") =>
+        Ok(new { success = error.Length == 0, value = room, error });
+
     /// <summary>Give a freshly-cloned room its OWN copy of the source's save
     /// data instead of pointing its scenes at the source's blob.
     ///

docs/recroom-2023-client-api-complete.md

@@ -659,7 +659,7 @@ Diffed all 45 real HTTP routes of the 2023-03-21 client's rooms subsystem agains
 | GET + POST | `rooms/{0}/bans` | GET: none. POST 'ban players': form fields id (repeated Int32 account ids) + banMask (Int32 enum DCDPJPHBHOA) | GET → [IBHAKOOKEEE {"AccountId": Int32, "BannedByAccountId": Int32, "BanStartTime": DateTime string}]. POST → none parsed. |
 | POST | `rooms/{0}/bans/import` | form field: sourceRoomId (Int64) | none parsed; return 200 |
 | DELETE | `rooms/{0}/bans/{1}` | form field: banMask (Int32 enum) | none parsed; return 200 |
-| POST | `rooms/{0}/clone` | x-www-form-urlencoded field: name (String) — new room name | FGCPNAACHIK FULL room details of the clone (not a status wrapper); client 404→empty-body surfaces as message-less 'Failed to copy room' |
+| POST | `rooms/{0}/clone` | x-www-form-urlencoded field: name (String) — new room name | `{success, value, error}` envelope with `value` = FGCPNAACHIK FULL room details of the clone (same helper as "save room"; a bare room has no `success` and is rejected with an empty error). Client 404→empty-body also surfaces as message-less 'Failed to copy room' |
 | PUT | `rooms/{0}/cloning` | form field: cloningAllowed (Boolean) | FGCPNAACHIK full room details |
 | PUT | `rooms/{0}/comments` | form field: disable (Boolean) — key is 'disable', maps to DisableRoomComments | FGCPNAACHIK full room details |
 | PUT | `rooms/{0}/creator` | form field: accountId (Int32) | FGCPNAACHIK full room details |

docs/recroom-2023-room-save.md

@@ -89,8 +89,15 @@ body — the same `roomserver/` prefix as every other room mutation from
 that client. A bare-only `rooms/{id}/clone` route 404s with an empty
 body, which the client's promise layer surfaces as a message-less
 `Failed to copy room: Exception of type 'CEMNLBKJABA' was thrown`.
-The response must be the FULL room-details object (`FGCPNAACHIK`,
+The response is the legacy `{success, value, error}` envelope, the same
-same shape as `GET roomserver/rooms/{id}`), not a status wrapper.
+one "save room" and "restore subroom save" use: the clone goes through the
+same RecNet rooms helper (`NLDBPDCNNCF.BDBHCEGKGDC`, which takes an op
+name). `value` is the FULL room-details object (`FGCPNAACHIK`, same shape
+as `GET roomserver/rooms/{id}`). A bare room-details body has no `success`,
+so the client rejects it with an empty error and shows the same
+message-less `Failed to copy room`, even though the room was created.
+Refusals (unknown source, cloning not allowed) are HTTP 200 with
+`success: false` and the message in `error`.
 
 Blob semantics on clone (`RoomsModerationController.BareClone`):