SP2: reconcile SyncPlay flat wire frame-shape

src/Server/WebSocket/Connection.php

@@ -96,6 +96,25 @@ public function sendMessage(string $type, array $data = []): void
         ]);
     }
 
+    /**
+     * Sends a flat canonical message without wrapping payload under 'data'.
+     *
+     * Used for SyncPlay messages which use the flat canonical wire format:
+     * {type, ...payload, timestamp} instead of {type, data: {...}, timestamp}.
+     *
+     * @param string $type The message type/event name
+     * @param array<string, mixed> $payload The flat event payload (sent directly, not under 'data')
+     * @return void
+     */
+    public function sendFlat(string $type, array $payload): void
+    {
+        $this->send(array_merge(
+            ['type' => $type],
+            $payload,
+            ['timestamp' => time()]
+        ));
+    }
+
     /**
      * Closes the connection.
      *

src/Server/WebSocket/ConnectionInterface.php

@@ -41,6 +41,15 @@ public function send(string|array $data): void;
      */
     public function sendMessage(string $type, array $data = []): void;
 
+    /**
+     * Sends a flat canonical message without wrapping payload under 'data'.
+     *
+     * @param string $type The message type/event name
+     * @param array<string, mixed> $payload The flat event payload
+     * @return void
+     */
+    public function sendFlat(string $type, array $payload): void;
+
     /**
      * Closes the connection.
      *

src/Server/WebSocket/MessageHandler.php

@@ -108,6 +108,10 @@ public function onAny(callable $callback): void
      * Parses the JSON message, extracts event type and payload,
      * and dispatches to the appropriate handler.
      *
+     * Supports two message formats:
+     * - Flat canonical (SyncPlay): {type, protocol_version, timestamp, ...payload}
+     * - Deprecated Tizen envelope (dashboard): {type, data: {...}, timestamp}
+     *
      * @param Connection $connection The connection that sent the message
      * @param string $data Raw message data (expected JSON)
      * @return void
@@ -124,7 +128,42 @@ public function handle(Connection $connection, string $data): void
         }
 
         $event = $message['type'];
-        $payload = $message['data'] ?? [];
+
+        // Handle subscribe_dashboard event - keep BC with deprecated {type,data} envelope
+        if ($event === 'subscribe_dashboard') {
+            $payload = $message['data'] ?? [];
+            $this->connections->add($connection);
+            $payloadMap = [];
+            if (is_array($payload)) {
+                foreach ($payload as $pKey => $pValue) {
+                    if (is_string($pKey)) {
+                        $payloadMap[$pKey] = $pValue;
+                    }
+                }
+            }
+            $this->handleSubscribeDashboard($connection, $payloadMap);
+            return;
+        }
+
+        // For flat canonical messages (SyncPlay), use the whole message as payload.
+        // Tolerant unwrap for deprecated {type,data} envelope for BC.
+        // Syncplay messages have protocol_version but no 'data' key.
+        $hasDataKey = array_key_exists('data', $message);
+        $payload = $hasDataKey
+            ? $message['data']
+            : $message;
+
+        // Validate protocol_version for flat canonical messages
+        if (!$hasDataKey && isset($message['protocol_version'])) {
+            $protocolVersion = $message['protocol_version'];
+            if (!is_int($protocolVersion) || $protocolVersion > \Phlix\Session\SyncPlay\Messages::PROTOCOL_VERSION) {
+                $connection->sendFlat(\Phlix\Session\SyncPlay\Messages::TYPE_ERROR, [
+                    'error_code' => 'PROTOCOL_VERSION_MISMATCH',
+                    'message' => 'Unsupported protocol version',
+                ]);
+                return;
+            }
+        }
 
         $this->connections->add($connection);
 
@@ -141,19 +180,6 @@ public function handle(Connection $connection, string $data): void
             // Wildcard handler
             ($this->callbacks['*'])($connection, $event, $payload);
         }
-
-        // Handle subscribe_dashboard event
-        if ($event === 'subscribe_dashboard') {
-            $payloadMap = [];
-            if (is_array($payload)) {
-                foreach ($payload as $pKey => $pValue) {
-                    if (is_string($pKey)) {
-                        $payloadMap[$pKey] = $pValue;
-                    }
-                }
-            }
-            $this->handleSubscribeDashboard($connection, $payloadMap);
-        }
     }
 
     /**

src/Session/SyncPlay/SyncPlayManager.php

@@ -705,7 +705,8 @@ private function handleChatMessage(Connection $connection, array $payload): void
     private function handleTimePing(Connection $connection, array $payload): void
     {
         $pong = $this->timeSync->processPing($payload);
-        $connection->sendMessage(Messages::TYPE_TIME_PONG, $pong);
+        $message = Messages::timePong($pong['client_time'], $pong['server_time']);
+        $connection->send($message);
     }
 
     /**
@@ -730,7 +731,7 @@ private function handleGroupCreate(Connection $connection, array $payload): void
         $result = $this->createGroup($groupName, $password, $memberId, $memberName);
 
         if ($result['success'] === true) {
-            $connection->sendMessage(Messages::TYPE_GROUP_STATE, [
+            $connection->sendFlat(Messages::TYPE_GROUP_STATE, [
                 'group' => $result['group'],
                 'your_id' => $memberId,
             ]);
@@ -759,7 +760,7 @@ private function handleGroupJoin(Connection $connection, array $payload): void
         $result = $this->joinGroup($groupId, $memberId, $memberName, $password);
 
         if ($result['success'] === true) {
-            $connection->sendMessage(Messages::TYPE_GROUP_STATE, [
+            $connection->sendFlat(Messages::TYPE_GROUP_STATE, [
                 'group' => $result['group'],
                 'your_id' => $memberId,
             ]);
@@ -846,8 +847,8 @@ private function broadcastToGroup(string $groupId, string $type, array $data, ar
      */
     private function sendError(Connection $connection, string $code, string $message): void
     {
-        $connection->sendMessage(Messages::TYPE_ERROR, [
-            'code' => $code,
+        $connection->sendFlat(Messages::TYPE_ERROR, [
+            'error_code' => $code,
             'message' => $message,
         ]);
     }

tests/Unit/Server/WebSocket/ConnectionPoolTest.php

@@ -94,6 +94,7 @@ public function getLastActivity(): int { return $this->lastActivity; }
             public function send($data): void {}
             public function close(): void {}
             public function sendMessage($type, $data = []): void {}
+            public function sendFlat($type, $payload): void {}
             public function updateActivity(): void { $this->lastActivity = time(); }
             public function set(string $key, mixed $value): void { $this->sessionData[$key] = $value; }
             public function get(string $key, mixed $default = null): mixed { return $this->sessionData[$key] ?? $default; }

tests/Unit/Server/WebSocket/MessageHandlerFrameShapeTest.php

@@ -0,0 +1,219 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Phlix\Tests\Unit\Server\WebSocket;
+
+use PHPUnit\Framework\TestCase;
+use Phlix\Server\WebSocket\Connection;
+use Phlix\Server\WebSocket\ConnectionPool;
+use Phlix\Server\WebSocket\MessageHandler;
+use Phlix\Session\SyncPlay\Messages;
+use Workerman\Connection\TcpConnection;
+use Workerman\Worker;
+
+/**
+ * Unit tests for frame shape handling (SP2 - flat canonical wire format).
+ *
+ * @covers \Phlix\Server\WebSocket\MessageHandler
+ * @covers \Phlix\Server\WebSocket\Connection
+ */
+class MessageHandlerFrameShapeTest extends TestCase
+{
+    /**
+     * Tracks sent messages for verification.
+     *
+     * @var array<int, array{type: string, data?: array, timestamp: int}>
+     */
+    private array $sentMessages = [];
+
+    /**
+     * Creates a Connection with a mock TcpConnection that captures sent data.
+     */
+    private function createConnection(): Connection
+    {
+        $mockTcp = $this->createMock(TcpConnection::class);
+        $mockTcp->method('send')->willReturnCallback(function ($data) {
+            $this->sentMessages[] = is_string($data)
+                ? json_decode($data, true)
+                : $data;
+        });
+
+        return new class($mockTcp) extends Connection {
+            public function __construct(TcpConnection $connection)
+            {
+                parent::__construct($connection);
+            }
+        };
+    }
+
+    /**
+     * Creates a MessageHandler with a ConnectionPool for testing.
+     */
+    private function createMessageHandler(): MessageHandler
+    {
+        $pool = ConnectionPool::getInstance();
+        $pool->clear();
+        return new MessageHandler($pool);
+    }
+
+    protected function tearDown(): void
+    {
+        $this->sentMessages = [];
+        parent::tearDown();
+    }
+
+    /**
+     * @covers \Phlix\Server\WebSocket\Connection::sendFlat
+     */
+    public function testSendFlatProducesFlatCanonicalEnvelope(): void
+    {
+        $connection = $this->createConnection();
+
+        $connection->sendFlat('syncplay_group_state', [
+            'group' => ['group_id' => 'sp_abc123', 'name' => 'Test Group'],
+            'your_id' => 'member_1',
+        ]);
+
+        $this->assertCount(1, $this->sentMessages);
+        $sent = $this->sentMessages[0];
+
+        // Must have type at top level
+        $this->assertEquals('syncplay_group_state', $sent['type']);
+
+        // Must have payload keys at top level (NOT under 'data')
+        $this->assertArrayHasKey('group', $sent);
+        $this->assertArrayHasKey('your_id', $sent);
+        $this->assertArrayHasKey('timestamp', $sent);
+
+        // Must NOT have 'data' key
+        $this->assertArrayNotHasKey('data', $sent);
+
+        // Verify group and your_id are correct
+        $this->assertEquals('sp_abc123', $sent['group']['group_id']);
+        $this->assertEquals('member_1', $sent['your_id']);
+    }
+
+    /**
+     * @covers \Phlix\Server\WebSocket\Connection::sendMessage
+     */
+    public function testSendMessageProducesDeprecatedEnvelope(): void
+    {
+        $connection = $this->createConnection();
+
+        $connection->sendMessage('syncplay_group_state', [
+            'group' => ['group_id' => 'sp_abc123'],
+            'your_id' => 'member_1',
+        ]);
+
+        $this->assertCount(1, $this->sentMessages);
+        $sent = $this->sentMessages[0];
+
+        // Deprecated envelope has 'data' key
+        $this->assertEquals('syncplay_group_state', $sent['type']);
+        $this->assertArrayHasKey('data', $sent);
+        $this->assertArrayHasKey('timestamp', $sent);
+
+        // Payload is under 'data'
+        $this->assertArrayHasKey('group', $sent['data']);
+        $this->assertArrayHasKey('your_id', $sent['data']);
+    }
+
+    /**
+     * @covers \Phlix\Server\WebSocket\MessageHandler::handle
+     */
+    public function testHandlePassesFlatMessageToSyncplayHandler(): void
+    {
+        $handler = $this->createMessageHandler();
+        $connection = $this->createConnection();
+
+        $receivedPayload = null;
+        $handler->on('syncplay_group_create', function ($conn, $payload) use (&$receivedPayload) {
+            $receivedPayload = $payload;
+        });
+
+        // Flat canonical message (no 'data' key, has protocol_version)
+        $flatMessage = json_encode([
+            'type' => 'syncplay_group_create',
+            'protocol_version' => 1,
+            'member_id' => 'member_1',
+            'member_name' => 'Test User',
+            'group_name' => 'Test Group',
+            'timestamp' => 1234567890,
+        ]);
+
+        $handler->handle($connection, $flatMessage);
+
+        // The handler should receive the FULL flat message as payload
+        // (message minus 'type' field, so it has protocol_version, member_id, etc.)
+        $this->assertNotNull($receivedPayload);
+        $this->assertIsArray($receivedPayload);
+        $this->assertEquals(1, $receivedPayload['protocol_version']);
+        $this->assertEquals('member_1', $receivedPayload['member_id']);
+        $this->assertEquals('Test User', $receivedPayload['member_name']);
+        $this->assertEquals('Test Group', $receivedPayload['group_name']);
+    }
+
+    /**
+     * @covers \Phlix\Server\WebSocket\MessageHandler::handle
+     */
+    public function testHandlePreservesDeprecatedEnvelopeBC(): void
+    {
+        $handler = $this->createMessageHandler();
+        $connection = $this->createConnection();
+
+        // Deprecated Tizen envelope with 'data' key
+        $deprecatedMessage = json_encode([
+            'type' => 'subscribe_dashboard',
+            'data' => ['session_id' => 'sess_123'],
+            'timestamp' => 1234567890,
+        ]);
+
+        $handler->handle($connection, $deprecatedMessage);
+
+        // subscribe_dashboard is handled by handleSubscribeDashboard() which sends
+        // DASHBOARD_NOW_PLAYING message. The callback registered via on() is NOT
+        // invoked (this is the original behavior for BC).
+        // We verify the message was sent with the correct structure.
+        $this->assertNotEmpty($this->sentMessages);
+        $sent = $this->sentMessages[0];
+        $this->assertEquals('dashboard_now_playing', $sent['type']);
+        $this->assertArrayHasKey('data', $sent);
+        $this->assertArrayHasKey('subscribed', $sent['data']);
+        $this->assertTrue($sent['data']['subscribed']);
+    }
+
+    /**
+     * @covers \Phlix\Server\WebSocket\MessageHandler::handle
+     */
+    public function testHandleRejectsUnsupportedProtocolVersion(): void
+    {
+        $handler = $this->createMessageHandler();
+        $connection = $this->createConnection();
+
+        $receivedPayload = null;
+        $handler->on('syncplay_group_create', function ($conn, $payload) use (&$receivedPayload) {
+            $receivedPayload = $payload;
+        });
+
+        // Message with future protocol version
+        $futureMessage = json_encode([
+            'type' => 'syncplay_group_create',
+            'protocol_version' => 999, // Future version
+            'member_id' => 'member_1',
+            'timestamp' => 1234567890,
+        ]);
+
+        $handler->handle($connection, $futureMessage);
+
+        // Handler should NOT be called
+        $this->assertNull($receivedPayload);
+
+        // Error should be sent with error_code (not 'code')
+        $this->assertCount(1, $this->sentMessages);
+        $errorMsg = $this->sentMessages[0];
+        $this->assertEquals(Messages::TYPE_ERROR, $errorMsg['type']);
+        $this->assertArrayHasKey('error_code', $errorMsg);
+        $this->assertEquals('PROTOCOL_VERSION_MISMATCH', $errorMsg['error_code']);
+    }
+}