fix: Target.attachToTarget returns unique session id per call

Target.attachToTarget was reusing the existing bc.session_id when one
was already set (via the `orelse` pattern). The CDP spec requires a
fresh unique sessionId per attachToTarget call, as a single target can
have multiple sessions attached simultaneously.

This broke Playwright's ctx.newCDPSession(page) flow: the returned
sessionId was the same as the page's existing session, so Playwright's
createChildSession() overwrote _sessions[sid] with a new empty object,
orphaning the page session's pending callbacks and triggering
assert(!object.id).

Changes:
- doAttachtoTarget: always call session_id_gen.next(). First attach
  sets bc.session_id (primary), subsequent attach sets bc.alt_session_id.
- Copy session IDs into fixed buffers (session_id_buf / alt_session_id_buf)
  because session_id_gen.next() reuses its internal buffer and previous
  slices would be invalidated.
- attachToBrowserTarget: also copy into session_id_buf for safety.
- isValidSessionId: accept both primary and alt session IDs.
- callInspector / onInspectorResponse: route V8 inspector replies back
  to the requesting session via inspector_reply_session.
- runtime.zig sendInspector: pass cmd.input.session_id to callInspector.
- Add test: "attachToTarget returns unique sessionId per call" verifying
  two successive attach calls produce different session IDs.

Amp-Thread-ID: https://ampcode.com/threads/T-019d5277-a92a-736b-bcb4-8aa78109c21d
Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019d5277-a92a-736b-bcb4-8aa78109c21d
Co-authored-by: Amp <amp@ampcode.com>

Author: soilSpoon

src/cdp/CDP.zig

@@ -263,8 +263,13 @@ fn dispatchCommand(command: *Command, method: []const u8) !void {
 
 fn isValidSessionId(self: *const CDP, input_session_id: []const u8) bool {
     const browser_context = &(self.browser_context orelse return false);
-    const session_id = browser_context.session_id orelse return false;
-    return std.mem.eql(u8, session_id, input_session_id);
+    if (browser_context.session_id) |sid| {
+        if (std.mem.eql(u8, sid, input_session_id)) return true;
+    }
+    if (browser_context.alt_session_id) |alt| {
+        if (std.mem.eql(u8, alt, input_session_id)) return true;
+    }
+    return false;
 }
 
 pub fn createBrowserContext(self: *CDP) ![]const u8 {
@@ -351,7 +356,19 @@ pub const BrowserContext = struct {
     // is all pretty straightforward, but it still needs to be enforced, i.e.
     // if we get a request with a sessionId that doesn't match the current one
     // we should reject it.
-    session_id: ?[]const u8,
+    // session_id_gen.next() reuses its internal buffer, so slices returned
+    // by it are only valid until the next call.  We copy into fixed buffers
+    // so that session IDs remain stable across multiple next() calls.
+    session_id: ?[]const u8 = null,
+    session_id_buf: [14]u8 = undefined,
+
+    // Additional session ID from explicit Target.attachToTarget calls.
+    alt_session_id: ?[]const u8 = null,
+    alt_session_id_buf: [14]u8 = undefined,
+
+    // Tracks which session to reply to for in-flight inspector commands.
+    // Set before callInspector, cleared after runMicrotasks.
+    inspector_reply_session: ?[]const u8 = null,
 
     security_origin: []const u8,
     page_life_cycle_events: bool,
@@ -701,13 +718,17 @@ pub const BrowserContext = struct {
         defer _ = self.cdp.notification_arena.reset(.{ .retain_with_limit = 1024 * 64 });
     }
 
-    pub fn callInspector(self: *const BrowserContext, msg: []const u8) void {
+    pub fn callInspector(self: *BrowserContext, msg: []const u8, reply_session_id: ?[]const u8) void {
+        self.inspector_reply_session = reply_session_id;
         self.inspector_session.send(msg);
         self.session.browser.env.runMicrotasks();
+        self.inspector_reply_session = null;
     }
 
     pub fn onInspectorResponse(ctx: *anyopaque, _: u32, msg: []const u8) void {
-        sendInspectorMessage(@ptrCast(@alignCast(ctx)), msg) catch |err| {
+        const bc: *BrowserContext = @ptrCast(@alignCast(ctx));
+        const target_session = bc.inspector_reply_session orelse bc.session_id;
+        sendInspectorMessageTo(bc, msg, target_session) catch |err| {
             log.err(.cdp, "send inspector response", .{ .err = err });
         };
     }
@@ -733,7 +754,11 @@ pub const BrowserContext = struct {
     // session_id onto it. Second, we're much more client/websocket aware than
     // we should be.
     fn sendInspectorMessage(self: *BrowserContext, msg: []const u8) !void {
-        const session_id = self.session_id orelse {
+        return sendInspectorMessageTo(self, msg, self.session_id);
+    }
+
+    fn sendInspectorMessageTo(self: *BrowserContext, msg: []const u8, session_id: ?[]const u8) !void {
+        const sid = session_id orelse {
             // We no longer have an active session. What should we do
             // in this case?
             return;
@@ -746,7 +771,7 @@ pub const BrowserContext = struct {
 
         // + 1 for the closing quote after the session id
         // + 10 for the max websocket header
-        const message_len = msg.len + session_id.len + 1 + field.len + 10;
+        const message_len = msg.len + sid.len + 1 + field.len + 10;
 
         var buf: std.ArrayList(u8) = .{};
         buf.ensureTotalCapacity(allocator, message_len) catch |err| {
@@ -760,7 +785,7 @@ pub const BrowserContext = struct {
         // -1  because we dont' want the closing brace '}'
         buf.appendSliceAssumeCapacity(msg[0 .. msg.len - 1]);
         buf.appendSliceAssumeCapacity(field);
-        buf.appendSliceAssumeCapacity(session_id);
+        buf.appendSliceAssumeCapacity(sid);
         buf.appendSliceAssumeCapacity("\"}");
         if (comptime IS_DEBUG) {
             std.debug.assert(buf.items.len == message_len);

src/cdp/domains/page.zig

@@ -228,6 +228,7 @@ fn close(cmd: *CDP.Command) !void {
         }, .{});
 
         bc.session_id = null;
+        bc.alt_session_id = null;
     }
 
     bc.session.removePage();

src/cdp/domains/runtime.zig

@@ -47,7 +47,9 @@ fn sendInspector(cmd: *CDP.Command, action: anytype) !void {
     const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
 
     // the result to return is handled directly by the inspector.
-    bc.callInspector(cmd.input.json);
+    // Pass the requesting session id so the response is routed back correctly,
+    // even when an alt_session_id (from Target.attachToTarget) is in use.
+    bc.callInspector(cmd.input.json, cmd.input.session_id);
 }
 
 fn logInspector(cmd: *CDP.Command, action: anytype) !void {

src/cdp/domains/target.zig

@@ -246,16 +246,21 @@ fn attachToTarget(cmd: *CDP.Command) !void {
 
     try doAttachtoTarget(cmd, target_id);
 
-    return cmd.sendResult(.{ .sessionId = bc.session_id }, .{});
+    // Return the newly assigned session id (alt if secondary, primary if first attach).
+    return cmd.sendResult(.{ .sessionId = bc.alt_session_id orelse bc.session_id }, .{});
 }
 
 fn attachToBrowserTarget(cmd: *CDP.Command) !void {
     const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
 
-    const session_id = bc.session_id orelse cmd.cdp.session_id_gen.next();
+    if (bc.session_id == null) {
+        const session_id = cmd.cdp.session_id_gen.next();
+        @memcpy(bc.session_id_buf[0..session_id.len], session_id);
+        bc.session_id = bc.session_id_buf[0..session_id.len];
+    }
 
     try cmd.sendEvent("Target.attachedToTarget", AttachToTarget{
-        .sessionId = session_id,
+        .sessionId = bc.session_id.?,
         .targetInfo = TargetInfo{
             .targetId = bc.id, // We use the browser context is as browser's target id.
             .title = "",
@@ -266,8 +271,6 @@ fn attachToBrowserTarget(cmd: *CDP.Command) !void {
         },
     }, .{});
 
-    bc.session_id = session_id;
-
     return cmd.sendResult(.{ .sessionId = bc.session_id }, .{});
 }
 
@@ -302,6 +305,7 @@ fn closeTarget(cmd: *CDP.Command) !void {
         }, .{});
 
         bc.session_id = null;
+        bc.alt_session_id = null;
     }
 
     bc.session.removePage();
@@ -387,6 +391,7 @@ fn detachFromTarget(cmd: *CDP.Command) !void {
             }, .{});
         }
         bc.session_id = null;
+        bc.alt_session_id = null;
     }
 
     return cmd.sendResult(null, .{});
@@ -417,6 +422,7 @@ fn setAutoAttach(cmd: *CDP.Command) !void {
                 }, .{});
             }
             bc.session_id = null;
+            bc.alt_session_id = null;
         }
         try cmd.sendResult(null, .{});
         return;
@@ -459,14 +465,10 @@ fn setAutoAttach(cmd: *CDP.Command) !void {
 
 fn doAttachtoTarget(cmd: *CDP.Command, target_id: []const u8) !void {
     const bc = cmd.browser_context.?;
-    const session_id = bc.session_id orelse cmd.cdp.session_id_gen.next();
-
-    if (bc.session_id == null) {
-        // extra_headers should not be kept on a new page or tab,
-        // currently we have only 1 page, we clear it just in case
-        bc.extra_headers.clearRetainingCapacity();
-    }
+    const session_id = cmd.cdp.session_id_gen.next();
 
+    // Send the event from the *parent* session (bc.session_id before update),
+    // matching original behaviour: first-attach events are root-level (no sessionId).
     try cmd.sendEvent("Target.attachedToTarget", AttachToTarget{
         .sessionId = session_id,
         .targetInfo = TargetInfo{
@@ -477,7 +479,21 @@ fn doAttachtoTarget(cmd: *CDP.Command, target_id: []const u8) !void {
         },
     }, .{ .session_id = bc.session_id });
 
-    bc.session_id = session_id;
+    // session_id_gen.next() reuses its internal buffer, so we must copy
+    // the value into a stable buffer before it gets overwritten.
+    if (bc.session_id == null) {
+        // First attach: set as primary session.
+        // extra_headers should not be kept on a new page or tab,
+        // currently we have only 1 page, we clear it just in case
+        bc.extra_headers.clearRetainingCapacity();
+        @memcpy(bc.session_id_buf[0..session_id.len], session_id);
+        bc.session_id = bc.session_id_buf[0..session_id.len];
+    } else {
+        // Subsequent attach (e.g. explicit Target.attachToTarget from a CDP client):
+        // keep the primary session_id intact so existing page operations remain valid.
+        @memcpy(bc.alt_session_id_buf[0..session_id.len], session_id);
+        bc.alt_session_id = bc.alt_session_id_buf[0..session_id.len];
+    }
 }
 
 const AttachToTarget = struct {
@@ -671,6 +687,34 @@ test "cdp.target: attachToTarget" {
     }
 }
 
+test "cdp.target: attachToTarget returns unique sessionId per call" {
+    var ctx = try testing.context();
+    defer ctx.deinit();
+    const bc = try ctx.loadBrowserContext(.{ .id = "BID-9" });
+    _ = try bc.session.createPage();
+    bc.target_id = "TID-000000000D".*;
+
+    // First attach — sets primary session_id.
+    try ctx.processMessage(.{ .id = 20, .method = "Target.attachToTarget", .params = .{ .targetId = "TID-000000000D" } });
+    const first_sid = bc.session_id.?;
+    // Event is sent before result; drain the event first.
+    try ctx.expectSentEvent("Target.attachedToTarget", .{ .sessionId = first_sid, .targetInfo = .{ .url = "about:blank", .title = "", .attached = true, .type = "page", .canAccessOpener = false, .browserContextId = "BID-9", .targetId = bc.target_id.? } }, .{});
+    try ctx.expectSentResult(.{ .sessionId = first_sid }, .{ .id = 20 });
+
+    // Second attach — must return a *different* session id (stored as alt_session_id).
+    // The event is tagged with the *primary* session (first_sid), not the new one.
+    try ctx.processMessage(.{ .id = 21, .method = "Target.attachToTarget", .params = .{ .targetId = "TID-000000000D" } });
+    const second_sid = bc.alt_session_id.?;
+    try ctx.expectSentEvent("Target.attachedToTarget", .{ .sessionId = second_sid, .targetInfo = .{ .url = "about:blank", .title = "", .attached = true, .type = "page", .canAccessOpener = false, .browserContextId = "BID-9", .targetId = bc.target_id.? } }, .{ .session_id = first_sid });
+    try ctx.expectSentResult(.{ .sessionId = second_sid }, .{ .id = 21 });
+
+    // The two session ids must differ.
+    try testing.expect(!std.mem.eql(u8, first_sid, second_sid));
+
+    // Primary session must remain unchanged.
+    try testing.expectEqualSlices(u8, first_sid, bc.session_id.?);
+}
+
 test "cdp.target: getTargetInfo" {
     var ctx = try testing.context();
     defer ctx.deinit();
@@ -750,13 +794,18 @@ test "cdp.target: detachFromTarget" {
         const session_id = bc.session_id.?;
         try ctx.expectSentResult(.{ .sessionId = session_id }, .{ .id = 11 });
 
-        try ctx.processMessage(.{ .id = 12, .method = "Target.detachFromTarget", .params = .{ .targetId = bc.target_id.? } });
+        // Second attach to create alt_session_id.
+        try ctx.processMessage(.{ .id = 12, .method = "Target.attachToTarget", .params = .{ .targetId = bc.target_id.? } });
+        try testing.expect(bc.alt_session_id != null);
+
+        try ctx.processMessage(.{ .id = 13, .method = "Target.detachFromTarget", .params = .{ .targetId = bc.target_id.? } });
         try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = session_id }, .{});
         try testing.expectEqual(null, bc.session_id);
-        try ctx.expectSentResult(null, .{ .id = 12 });
+        try testing.expectEqual(null, bc.alt_session_id);
+        try ctx.expectSentResult(null, .{ .id = 13 });
 
-        try ctx.processMessage(.{ .id = 13, .method = "Target.attachToTarget", .params = .{ .targetId = bc.target_id.? } });
-        try ctx.expectSentResult(.{ .sessionId = bc.session_id.? }, .{ .id = 13 });
+        try ctx.processMessage(.{ .id = 14, .method = "Target.attachToTarget", .params = .{ .targetId = bc.target_id.? } });
+        try ctx.expectSentResult(.{ .sessionId = bc.session_id.? }, .{ .id = 14 });
     }
 }