From 84557cb4e6e86598181d5646212de1d1024c82f3 Mon Sep 17 00:00:00 2001
From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Date: Thu, 19 Mar 2026 16:42:32 -0700
Subject: [PATCH] fix(cdp): send Target.detachedFromTarget event on detach

detachFromTarget and setAutoAttach(false) both null bc.session_id
without notifying the client. Per the CDP spec, detaching a session
must fire a Target.detachedFromTarget event so the driver stops
sending messages on the stale session ID.

Capture the session_id before nulling it and fire the event in both
code paths. Add tests covering the event emission and the no-session
edge case.

Fixes #1819

This contribution was developed with AI assistance (Claude Code + Codex).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/cdp/domains/target.zig | 55 +++++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/src/cdp/domains/target.zig b/src/cdp/domains/target.zig
index 1ff23849..51af7c53 100644
--- a/src/cdp/domains/target.zig
+++ b/src/cdp/domains/target.zig
@@ -394,15 +394,13 @@ fn sendMessageToTarget(cmd: anytype) !void {
 }
 
 fn detachFromTarget(cmd: anytype) !void {
-    // TODO check if sessionId/targetId match.
-    // const params = (try cmd.params(struct {
-    //     sessionId: ?[]const u8,
-    //     targetId: ?[]const u8,
-    // })) orelse return error.InvalidParams;
-
     if (cmd.browser_context) |bc| {
+        if (bc.session_id) |session_id| {
+            try cmd.sendEvent("Target.detachedFromTarget", .{
+                .sessionId = session_id,
+            }, .{});
+        }
         bc.session_id = null;
-        // TODO should we send a Target.detachedFromTarget event?
     }
 
     return cmd.sendResult(null, .{});
@@ -427,8 +425,12 @@ fn setAutoAttach(cmd: anytype) !void {
     if (cmd.cdp.target_auto_attach == false) {
         // detach from all currently attached targets.
         if (cmd.browser_context) |bc| {
+            if (bc.session_id) |session_id| {
+                try cmd.sendEvent("Target.detachedFromTarget", .{
+                    .sessionId = session_id,
+                }, .{});
+            }
             bc.session_id = null;
-            // TODO should we send a Target.detachedFromTarget event?
         }
         try cmd.sendResult(null, .{});
         return;
@@ -759,9 +761,11 @@ test "cdp.target: detachFromTarget" {
         try ctx.expectSentResult(.{ .targetId = bc.target_id.? }, .{ .id = 10 });
 
         try ctx.processMessage(.{ .id = 11, .method = "Target.attachToTarget", .params = .{ .targetId = bc.target_id.? } });
-        try ctx.expectSentResult(.{ .sessionId = bc.session_id.? }, .{ .id = 11 });
+        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.? } });
+        try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = session_id }, .{});
         try testing.expectEqual(null, bc.session_id);
         try ctx.expectSentResult(null, .{ .id = 12 });
 
@@ -769,3 +773,36 @@ test "cdp.target: detachFromTarget" {
         try ctx.expectSentResult(.{ .sessionId = bc.session_id.? }, .{ .id = 13 });
     }
 }
+
+test "cdp.target: detachFromTarget without session" {
+    var ctx = testing.context();
+    defer ctx.deinit();
+    _ = try ctx.loadBrowserContext(.{ .id = "BID-9" });
+    {
+        // detach when no session is attached should not send event
+        try ctx.processMessage(.{ .id = 10, .method = "Target.detachFromTarget" });
+        try ctx.expectSentResult(null, .{ .id = 10 });
+        try ctx.expectSentCount(0);
+    }
+}
+
+test "cdp.target: setAutoAttach false sends detachedFromTarget" {
+    var ctx = testing.context();
+    defer ctx.deinit();
+    const bc = try ctx.loadBrowserContext(.{ .id = "BID-9" });
+    {
+        try ctx.processMessage(.{ .id = 10, .method = "Target.createTarget", .params = .{ .browserContextId = "BID-9" } });
+        try testing.expectEqual(true, bc.target_id != null);
+        try ctx.expectSentResult(.{ .targetId = bc.target_id.? }, .{ .id = 10 });
+
+        try ctx.processMessage(.{ .id = 11, .method = "Target.attachToTarget", .params = .{ .targetId = bc.target_id.? } });
+        const session_id = bc.session_id.?;
+        try ctx.expectSentResult(.{ .sessionId = session_id }, .{ .id = 11 });
+
+        // setAutoAttach false should fire detachedFromTarget event
+        try ctx.processMessage(.{ .id = 12, .method = "Target.setAutoAttach", .params = .{ .autoAttach = false, .waitForDebuggerOnStart = false } });
+        try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = session_id }, .{});
+        try testing.expectEqual(null, bc.session_id);
+        try ctx.expectSentResult(null, .{ .id = 12 });
+    }
+}