fix: Target.attachToTarget returns unique session id per call

[soilSpoon]

Problem

Target.attachToTarget returns the existing bc.session_id when one is already set, instead of generating a new unique session ID. The CDP spec requires a fresh, unique sessionId per call — a single target can have multiple sessions attached simultaneously.

How it breaks

When a CDP client (e.g. Playwright) calls Target.attachToTarget from an existing page session (SID-1), lightpanda returns the same SID-1. The client then creates a child session keyed by SID-1, overwriting the original page session's entry. All pending callbacks on the original session become orphaned — responses arrive but no callback exists to handle them.

Captured via CDP proxy:

C→S  id=32  Target.attachToTarget  sessionId=SID-1
S→C  id=32  result={"sessionId":"SID-1"}   ← same id returned

Root cause

In doAttachtoTarget:

// Before:
const session_id = bc.session_id orelse cmd.cdp.session_id_gen.next();
// → when bc.session_id is already set, next() is never called

Chrome reference

Chrome's TargetHandler::AttachToTarget() always creates a new session with base::UnguessableToken::Create() and stores it in an unbounded std::map<string, unique_ptr<Session>>. Multiple sessions per target is explicitly supported.

Fix

1. Always generate a new session ID (target.zig)

// After:
const session_id = cmd.cdp.session_id_gen.next();  // always new

• First attach → stored as bc.session_id (primary)
• Subsequent attach → stored as bc.alt_session_id, leaving primary intact

2. Copy IDs into fixed buffers (target.zig)

session_id_gen uses a zero-allocation Incrementing generator that reuses its internal buffer — the slice returned by next() is invalidated by the next call. Both session_id and alt_session_id are now copied into 14-byte fixed arrays (session_id_buf / alt_session_id_buf).

3. Accept alt session in validation (CDP.zig)

isValidSessionId now checks both session_id and alt_session_id.

4. Route inspector responses to the requesting session (CDP.zig, runtime.zig)

V8 inspector has one session per BrowserContext. When a command arrives via alt_session_id, the response must be tagged with that session, not the primary. callInspector stores the requesting session in inspector_reply_session; onInspectorResponse reads it back. This is safe because callInspector → runMicrotasks is synchronous.

Limitations

• This adds a single alt_session_id slot (primary + 1 alt). Chrome supports unlimited concurrent sessions via std::map. For lightpanda's current single-BrowserContext / single-page architecture, one alt slot is sufficient. A future multi-session refactor would replace this with a session list.
• End-to-end Playwright verification was not possible because connectOverCDP depends on other unimplemented features (e.g. Target.createBrowserContext). The fix is verified via unit tests and raw CDP client testing.

Verification

417 of 417 tests passed

New test: "cdp.target: attachToTarget returns unique sessionId per call"

• Two successive attachToTarget calls return different session IDs
• Primary session_id remains unchanged after second attach
• Events and results carry correct session IDs

[soilSpoon]

While implementing this fix I noticed that BrowserContext currently has only one extra session slot (alt_session_id) beyond the primary. This is sufficient for Playwright's newCDPSession use case, but won't cover scenarios where Target.attachToTarget is called more than twice on the same target.

I saw in #1096 that multi-context support is actively in progress. Could you share where things stand with that work? Specifically:

• Is there a working branch or design doc we could look at?
• Would alt_session_id need to be replaced as part of that work (e.g. with a session list)?

Happy to update this PR to avoid conflicts if needed. Thanks!

[krichprollsch]

Hey @soilSpoon,

You are right about your analysis. Returning the same session ID every time has worked well for us so far, even though we do not follow the specs.

Multi-context support is being added step by step in main. The next step is to share clients into the main thread. There is still a long way to go before multiple contexts are fully supported.

I am not in favor of adding a second alternative session ID. As you said, it only fixes the issue for a 2nd call, but will break if the client asks for more.

Question: did returning the same session ID actually break your use case?

[soilSpoon]

Hey @soilSpoon,

You are right about your analysis. Returning the same session ID every time has worked well for us so far, even though we do not follow the specs.

Multi-context support is being added step by step in main. The next step is to share clients into the main thread. There is still a long way to go before multiple contexts are fully supported.

I am not in favor of adding a second alternative session ID. As you said, it only fixes the issue for a 2nd call, but will break if the client asks for more.

Question: did returning the same session ID actually break your use case?

@krichprollsch

Yes — here's the concrete scenario:

const page = await context.newPage();
await page.goto('https://example.com');
const session = await context.newCDPSession(page);  // Target.attachToTarget → returns SID-1 (same as page session)
await page.goto('https://example.com/other');       // assert(!object.id) crash

Captured via CDP proxy:

C→S  Target.attachToTarget  (from SID-1)
S→C  result: { sessionId: "SID-1" }     ← same ID returned

C→S  Page.navigate  id=38  sessionId=SID-1
S→C  id=38  result={...}  → no callback found → assert

Playwright's createChildSession("SID-1") overwrites _sessions["SID-1"] with a new empty object, orphaning the page session's pending callbacks.

---

You're right that alt_session_id is a half-fix. Given that multi-context is coming, what approach would you prefer here? We could:

1. Use ArrayList for sessions (proper fix, but may conflict with the multi-context redesign)
2. Keep alt_session_id as a temporary patch with a TODO pointing to the multi-context work
3. Drop the PR and wait for multi-context if you expect it soon

Happy to go whichever direction makes the most sense for the roadmap.