[codex] restore magiccdp multi-client package#3
Conversation
e6cdedd to
2b18597
Compare
2b18597 to
e1a3825
Compare
There was a problem hiding this comment.
7 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="extension/MagicCDPServer.ts">
<violation number="1" location="extension/MagicCDPServer.ts:1">
P2: Avoid disabling type checking for the entire file with `@ts-nocheck`; it suppresses real type errors across this server module.</violation>
</file>
<file name="bridge/injector.ts">
<violation number="1" location="bridge/injector.ts:1">
P1: Avoid file-wide `@ts-nocheck`; it disables all TypeScript safety in this module and can hide real type errors.</violation>
</file>
<file name="client/js/MagicCDPClient.ts">
<violation number="1" location="client/js/MagicCDPClient.ts:1">
P2: Avoid file-level `@ts-nocheck` here; it disables all static checks for a core client implementation and can hide real type regressions.</violation>
<violation number="2" location="client/js/MagicCDPClient.ts:86">
P2: Validate custom command names with an exact `Domain.method` shape; `split('.', 2)` currently accepts extra-dot names and registers misleading aliases.</violation>
</file>
<file name="types/magic.ts">
<violation number="1" location="types/magic.ts:79">
P2: Do not silently downgrade unsupported payload schema specs to `z.unknown()`, because it disables runtime validation without surfacing a schema error.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:6">
P1: Exported runtime entrypoints target `.ts` source files instead of built `.js`, which will break normal Node imports of this package.</violation>
<violation number="2" location="package.json:34">
P2: The dependency was downgraded and pinned to an older Zod version (`4.3.6`), which diverges from the repo’s `^4.4.1` baseline.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -1,3 +1,4 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
P2: Avoid disabling type checking for the entire file with @ts-nocheck; it suppresses real type errors across this server module.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At extension/MagicCDPServer.ts, line 1:
<comment>Avoid disabling type checking for the entire file with `@ts-nocheck`; it suppresses real type errors across this server module.</comment>
<file context>
@@ -1,3 +1,4 @@
+// @ts-nocheck
// MagicCDPServer: lives inside an extension service worker. Owns the registry
// of custom commands and event bindings, and emits events through the binding
</file context>
| const [domain, method] = name.split(".", 2); | ||
| if (!domain || !method) throw new Error(`Custom command must use Domain.method format, got ${name}`); |
There was a problem hiding this comment.
P2: Validate custom command names with an exact Domain.method shape; split('.', 2) currently accepts extra-dot names and registers misleading aliases.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/js/MagicCDPClient.ts, line 86:
<comment>Validate custom command names with an exact `Domain.method` shape; `split('.', 2)` currently accepts extra-dot names and registers misleading aliases.</comment>
<file context>
@@ -63,6 +64,39 @@ type ClientOptions = {
+};
+
+function defineCustomCommandMethod(client: MagicCDPClient, name: string) {
+ const [domain, method] = name.split(".", 2);
+ if (!domain || !method) throw new Error(`Custom command must use Domain.method format, got ${name}`);
+ const target = client as unknown as Record<string, Record<string, unknown>>;
</file context>
| const [domain, method] = name.split(".", 2); | |
| if (!domain || !method) throw new Error(`Custom command must use Domain.method format, got ${name}`); | |
| const parts = name.split("."); | |
| if (parts.length !== 2 || !parts[0] || !parts[1]) throw new Error(`Custom command must use Domain.method format, got ${name}`); | |
| const [domain, method] = parts; |
| @@ -1,3 +1,4 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
P2: Avoid file-level @ts-nocheck here; it disables all static checks for a core client implementation and can hide real type regressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/js/MagicCDPClient.ts, line 1:
<comment>Avoid file-level `@ts-nocheck` here; it disables all static checks for a core client implementation and can hide real type regressions.</comment>
<file context>
@@ -1,3 +1,4 @@
+// @ts-nocheck
// MagicCDPClient (JS): importable, no CLI, no demo code.
//
</file context>
| if (isZodType(schema)) return schema; | ||
| if (Object.values(schema).every(isZodType)) return z.object(schema as MagicPayloadShape).passthrough(); | ||
| if (schema.type === "object") return z.object({}).passthrough(); | ||
| return z.unknown(); |
There was a problem hiding this comment.
P2: Do not silently downgrade unsupported payload schema specs to z.unknown(), because it disables runtime validation without surfacing a schema error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At types/magic.ts, line 79:
<comment>Do not silently downgrade unsupported payload schema specs to `z.unknown()`, because it disables runtime validation without surfacing a schema error.</comment>
<file context>
@@ -58,15 +60,23 @@ export type MagicName = z.infer<typeof MagicNameSchema>;
+ if (isZodType(schema)) return schema;
+ if (Object.values(schema).every(isZodType)) return z.object(schema as MagicPayloadShape).passthrough();
+ if (schema.type === "object") return z.object({}).passthrough();
+ return z.unknown();
}
</file context>
| return z.unknown(); | |
| throw new Error("Unsupported payload schema; pass a Zod schema or Zod shape."); |
There was a problem hiding this comment.
15 issues found across 93 files (changes from recent commits).
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed. cubic prioritises the most important files to review.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="types/cdpmods.ts">
<violation number="1" location="types/cdpmods.ts:239">
P2: `ProtocolResultSchema` puts `z.unknown()` first, so `CDPModsCommandResultSchema` is unreachable and result validation is effectively bypassed.</violation>
<violation number="2" location="types/cdpmods.ts:243">
P2: `ProtocolEventParamsSchema` starts with `z.unknown()`, which makes all later union branches unreachable and disables specific event-shape validation.</violation>
</file>
<file name="client/python/cdpmods/__init__.py">
<violation number="1" location="client/python/cdpmods/__init__.py:1">
P1: Use a relative import in package `__init__.py`; the current absolute import can fail at runtime with `ModuleNotFoundError` when importing `cdpmods`.</violation>
</file>
<file name="client/go/go.mod">
<violation number="1" location="client/go/go.mod:1">
P2: The Go module path was renamed to `cdpmods`, which conflicts with the stated `magiccdp` package name and can break consumers importing the previous module path.</violation>
</file>
<file name="client/python/pyproject.toml">
<violation number="1" location="client/python/pyproject.toml:2">
P2: Package name is inconsistent with the PR’s cross-language naming goal; use `magiccdp` for the Python client as well.</violation>
</file>
<file name="bridge/injector.ts">
<violation number="1" location="bridge/injector.ts:163">
P0: Infinite loop: `for (;;)` has no timeout or deadline. If the service worker target never becomes ready (or `session_id_for_target` isn't provided, making `probeTarget` always return null), this hangs forever. The previous implementation had a `timeoutMs` deadline — reintroduce one.</violation>
</file>
<file name="client/go/CDPModsClient.go">
<violation number="1" location="client/go/CDPModsClient.go:794">
P1: This `for {}` loop has no timeout or deadline. If the extension's service worker target never becomes visible (e.g., manifest error, service worker crash), `Connect()` will block indefinitely. Add a deadline similar to the 15-second one used in `launchChrome`.</violation>
</file>
<file name="types/zod/CacheStorage.ts">
<violation number="1" location="types/zod/CacheStorage.ts:17">
P1: Enforce the `requestCacheNames` protocol invariant that exactly one of `securityOrigin`, `storageKey`, or `storageBucket` is provided; the current schema accepts invalid combinations.</violation>
</file>
<file name="client/python/CDPModsClient.py">
<violation number="1" location="client/python/CDPModsClient.py:49">
P1: `call` is declared `async def` but the class is a synchronous API and `self._client.send()` is synchronous. Callers like `client.Page.navigate(...)` will receive a coroutine object rather than the command result. This should be a plain `def`.</violation>
<violation number="2" location="client/python/CDPModsClient.py:249">
P2: `raw_send` is declared `async def` but only calls the synchronous `_send_frame`. This makes it inconsistent with the synchronous API and forces users to use an event loop for no benefit. Should be a regular `def`.</violation>
<violation number="3" location="client/python/CDPModsClient.py:434">
P1: This `while True` loop has no timeout. If the extension's service worker never registers (broken manifest, load failure, etc.), `connect()` will block the calling thread forever. Add a deadline similar to the 15s timeout used in `_launch_chrome`.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:2">
P2: Package name was changed to `cdpmods`, which conflicts with the stated goal to keep the package name `magiccdp`.</violation>
</file>
<file name="bridge/launcher.ts">
<violation number="1" location="bridge/launcher.ts:70">
P1: Changing `launchChrome` default to `headless = false` can break the client autolaunch path in headless environments.</violation>
</file>
<file name="bridge/translate.ts">
<violation number="1" location="bridge/translate.ts:304">
P1: `unwrapEventIfNeeded` now trusts `payload.event` before verifying the binding prefix, so non-CDPMods `Runtime.bindingCalled` events can be incorrectly accepted as CDPMods events.</violation>
</file>
<file name="bridge/proxy.ts">
<violation number="1" location="bridge/proxy.ts:77">
P1: `Mods.*` commands like `Mods.configure`/`Mods.ping` are no longer routed to the service worker because the new regex only matches `CDPMods.*` and `Custom.*`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -1,4 +1,4 @@ | |||
| module magiccdp | |||
| module cdpmods | |||
There was a problem hiding this comment.
P2: The Go module path was renamed to cdpmods, which conflicts with the stated magiccdp package name and can break consumers importing the previous module path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/go/go.mod, line 1:
<comment>The Go module path was renamed to `cdpmods`, which conflicts with the stated `magiccdp` package name and can break consumers importing the previous module path.</comment>
<file context>
@@ -1,4 +1,4 @@
-module magiccdp
+module cdpmods
go 1.24.7
</file context>
| @@ -1,5 +1,5 @@ | |||
| [project] | |||
| name = "magic-cdp-python-client" | |||
| name = "cdpmods" | |||
There was a problem hiding this comment.
P2: Package name is inconsistent with the PR’s cross-language naming goal; use magiccdp for the Python client as well.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/python/pyproject.toml, line 2:
<comment>Package name is inconsistent with the PR’s cross-language naming goal; use `magiccdp` for the Python client as well.</comment>
<file context>
@@ -1,5 +1,5 @@
[project]
-name = "magiccdp"
+name = "cdpmods"
version = "0.0.0"
requires-python = ">=3.10"
</file context>
| name = "cdpmods" | |
| name = "magiccdp" |
There was a problem hiding this comment.
7 issues found across 102 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed. cubic prioritises the most important files to review.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/go/CDPModClient.go">
<violation number="1" location="client/go/CDPModClient.go:504">
P1: Zip Slip path traversal: `file.Name` from the archive is joined to the target directory without verifying the result stays within `dir`. A crafted `.zip` with `../` entries can write arbitrary files outside the temp directory.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:2">
P2: Package name is inconsistent with the stated `magiccdp` cross-language naming, which can break expected install/import identity.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:29">
P3: The README points to `client/go/demo.go`, but the Go demo file is `client/go/demo/main.go`.</violation>
<violation number="2" location="README.md:164">
P3: The repository layout block has incorrect client file paths for Python and Go.</violation>
</file>
<file name="extension/CDPModServer.ts">
<violation number="1" location="extension/CDPModServer.ts:267">
P2: Each successful `callLoopbackWS` invocation leaves an orphaned `{ once: true }` error listener on the shared WebSocket. These accumulate without bound on long-lived connections until the first error fires and removes them all. Consider using an `AbortController` to remove the listener when the response arrives, or rely solely on the global error/close handlers that already reject all pending promises via `rejectLoopbackPending`.</violation>
</file>
<file name="bridge/launcher.ts">
<violation number="1" location="bridge/launcher.ts:93">
P2: Passing `--load-extension` without waking the extension service worker can make extension discovery flaky.</violation>
</file>
<file name="bridge/translate.ts">
<violation number="1" location="bridge/translate.ts:34">
P2: `bindingNameFor` introduced a lossy `* -> all` rewrite, which can cause `Runtime.bindingCalled` events to be discarded by the new payload-event consistency check.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| defer reader.Close() | ||
| for _, file := range reader.File { | ||
| targetPath := filepath.Join(dir, file.Name) |
There was a problem hiding this comment.
P1: Zip Slip path traversal: file.Name from the archive is joined to the target directory without verifying the result stays within dir. A crafted .zip with ../ entries can write arbitrary files outside the temp directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/go/CDPModClient.go, line 504:
<comment>Zip Slip path traversal: `file.Name` from the archive is joined to the target directory without verifying the result stays within `dir`. A crafted `.zip` with `../` entries can write arbitrary files outside the temp directory.</comment>
<file context>
@@ -0,0 +1,1074 @@
+ }
+ defer reader.Close()
+ for _, file := range reader.File {
+ targetPath := filepath.Join(dir, file.Name)
+ if file.FileInfo().IsDir() {
+ if err := os.MkdirAll(targetPath, 0o755); err != nil {
</file context>
| @@ -1,34 +1,59 @@ | |||
| { | |||
| "name": "magic-cdp-bridge", | |||
| "name": "cdpmod", | |||
There was a problem hiding this comment.
P2: Package name is inconsistent with the stated magiccdp cross-language naming, which can break expected install/import identity.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 2:
<comment>Package name is inconsistent with the stated `magiccdp` cross-language naming, which can break expected install/import identity.</comment>
<file context>
@@ -1,34 +1,59 @@
{
- "name": "magic-cdp-bridge",
+ "name": "cdpmod",
"version": "0.0.0",
"private": true,
</file context>
| "name": "cdpmod", | |
| "name": "magiccdp", |
| ws.send(JSON.stringify(message)); | ||
| return new Promise<ProtocolResult>((resolve, reject) => { | ||
| loopbackPending.set(id, { resolve, reject }); | ||
| ws.addEventListener("error", () => reject(new Error(`CDP socket error ${CDPModServer.loopback_cdp_url}`)), { |
There was a problem hiding this comment.
P2: Each successful callLoopbackWS invocation leaves an orphaned { once: true } error listener on the shared WebSocket. These accumulate without bound on long-lived connections until the first error fires and removes them all. Consider using an AbortController to remove the listener when the response arrives, or rely solely on the global error/close handlers that already reject all pending promises via rejectLoopbackPending.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At extension/CDPModServer.ts, line 267:
<comment>Each successful `callLoopbackWS` invocation leaves an orphaned `{ once: true }` error listener on the shared WebSocket. These accumulate without bound on long-lived connections until the first error fires and removes them all. Consider using an `AbortController` to remove the listener when the response arrives, or rely solely on the global error/close handlers that already reject all pending promises via `rejectLoopbackPending`.</comment>
<file context>
@@ -0,0 +1,821 @@
+ ws.send(JSON.stringify(message));
+ return new Promise<ProtocolResult>((resolve, reject) => {
+ loopbackPending.set(id, { resolve, reject });
+ ws.addEventListener("error", () => reject(new Error(`CDP socket error ${CDPModServer.loopback_cdp_url}`)), {
+ once: true,
+ });
</file context>
| "--remote-debugging-address=127.0.0.1", | ||
| `--remote-debugging-port=${usePort}`, | ||
| ...extraFlags, | ||
| ...extra_args, |
There was a problem hiding this comment.
P2: Passing --load-extension without waking the extension service worker can make extension discovery flaky.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bridge/launcher.ts, line 93:
<comment>Passing `--load-extension` without waking the extension service worker can make extension discovery flaky.</comment>
<file context>
@@ -99,32 +65,32 @@ export async function freePort() {
"--remote-debugging-address=127.0.0.1",
`--remote-debugging-port=${usePort}`,
- ...extraFlags,
+ ...extra_args,
"about:blank",
].filter(Boolean);
</file context>
|
|
||
| export const bindingNameFor = (eventName: string) => BINDING_PREFIX + eventName.replaceAll(".", "_"); | ||
| export const bindingNameFor = (eventName: string) => | ||
| BINDING_PREFIX + eventName.replaceAll(".", "_").replaceAll("*", "all"); |
There was a problem hiding this comment.
P2: bindingNameFor introduced a lossy * -> all rewrite, which can cause Runtime.bindingCalled events to be discarded by the new payload-event consistency check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bridge/translate.ts, line 34:
<comment>`bindingNameFor` introduced a lossy `* -> all` rewrite, which can cause `Runtime.bindingCalled` events to be discarded by the new payload-event consistency check.</comment>
<file context>
@@ -1,45 +1,59 @@
-export const bindingNameFor = (eventName: string) => BINDING_PREFIX + eventName.replaceAll(".", "_");
+export const bindingNameFor = (eventName: string) =>
+ BINDING_PREFIX + eventName.replaceAll(".", "_").replaceAll("*", "all");
export const eventNameFor = (bindingName: string) =>
</file context>
| python/CDPModClient.py + demo.py | ||
| go/CDPModClient.go + demo.go |
There was a problem hiding this comment.
P3: The repository layout block has incorrect client file paths for Python and Go.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 164:
<comment>The repository layout block has incorrect client file paths for Python and Go.</comment>
<file context>
@@ -125,50 +125,50 @@ pnpm run demo:go
- python/MagicCDPClient.py + demo.py
- go/MagicCDPClient.go + demo.go
+ js/CDPModClient.ts + demo.ts
+ python/CDPModClient.py + demo.py
+ go/CDPModClient.go + demo.go
dist/ Built JS output used by the extension and Node CLI scripts
</file context>
| python/CDPModClient.py + demo.py | |
| go/CDPModClient.go + demo.go | |
| python/cdpmod/CDPModClient.py + demo.py | |
| go/CDPModClient.go + demo/main.go |
| It's perfectly compatible with playwright, puppeteer, etc. with no modifications. You can do things like `patchright` does, but generically at the CDP layer instead of having to patch libraries. | ||
|
|
||
| You can send `Magic.*`, `Custom.*`, etc. through standard Playwright/Puppeteer/other-driver-managed CDP sessions; `client/js/demo.ts`, `client/python/demo.py`, and `client/go/demo.go` demonstrate the flow in each language. | ||
| You can send `Mod.*`, `Custom.*`, etc. through standard Playwright/Puppeteer/other-driver-managed CDP sessions; `client/js/demo.ts`, `client/python/demo.py`, and `client/go/demo.go` demonstrate the flow in each language. |
There was a problem hiding this comment.
P3: The README points to client/go/demo.go, but the Go demo file is client/go/demo/main.go.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 29:
<comment>The README points to `client/go/demo.go`, but the Go demo file is `client/go/demo/main.go`.</comment>
<file context>
@@ -12,33 +12,33 @@ CDP is powerful but it's been stretched to many use-cases beyond its initial aud
It's perfectly compatible with playwright, puppeteer, etc. with no modifications. You can do things like `patchright` does, but generically at the CDP layer instead of having to patch libraries.
-You can send `Magic.*`, `Custom.*`, etc. through standard Playwright/Puppeteer/other-driver-managed CDP sessions; `client/js/demo.ts`, `client/python/demo.py`, and `client/go/demo.go` demonstrate the flow in each language.
+You can send `Mod.*`, `Custom.*`, etc. through standard Playwright/Puppeteer/other-driver-managed CDP sessions; `client/js/demo.ts`, `client/python/demo.py`, and `client/go/demo.go` demonstrate the flow in each language.
## Use it
</file context>
| You can send `Mod.*`, `Custom.*`, etc. through standard Playwright/Puppeteer/other-driver-managed CDP sessions; `client/js/demo.ts`, `client/python/demo.py`, and `client/go/demo.go` demonstrate the flow in each language. | |
| You can send `Mod.*`, `Custom.*`, etc. through standard Playwright/Puppeteer/other-driver-managed CDP sessions; `client/js/demo.ts`, `client/python/demo.py`, and `client/go/demo/main.go` demonstrate the flow in each language. |
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/go/demo/main.go">
<violation number="1" location="client/go/demo/main.go:211">
P2: This pong handler can block indefinitely after the first ping because `pongCh` is no longer drained, which causes blocked goroutines on subsequent `Mod.pong` events.</violation>
</file>
<file name="bridge/proxy.ts">
<violation number="1" location="bridge/proxy.ts:160">
P2: Initializing `CDPModClient` without `server: null` causes each proxy connection to run `Mod.configure`, which can reset shared service-worker routes and clobber existing route overrides.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| pongCh := make(chan map[string]any, 16) | ||
| cdp.On("Mod.pong", func(data any) { | ||
| if event, ok := data.(map[string]any); ok { | ||
| pongCh <- event |
There was a problem hiding this comment.
P2: This pong handler can block indefinitely after the first ping because pongCh is no longer drained, which causes blocked goroutines on subsequent Mod.pong events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/go/demo/main.go, line 211:
<comment>This pong handler can block indefinitely after the first ping because `pongCh` is no longer drained, which causes blocked goroutines on subsequent `Mod.pong` events.</comment>
<file context>
@@ -158,10 +194,54 @@ func main() {
+ pongCh := make(chan map[string]any, 16)
+ cdp.On("Mod.pong", func(data any) {
+ if event, ok := data.(map[string]any); ok {
+ pongCh <- event
+ }
+ })
</file context>
| pongCh <- event | |
| select { | |
| case pongCh <- event: | |
| default: | |
| } |
| const cdp = new CDPModClient({ | ||
| cdp_url: upstream, | ||
| extension_path: extensionPath, | ||
| hydrate_aliases: false, | ||
| }); |
There was a problem hiding this comment.
P2: Initializing CDPModClient without server: null causes each proxy connection to run Mod.configure, which can reset shared service-worker routes and clobber existing route overrides.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bridge/proxy.ts, line 160:
<comment>Initializing `CDPModClient` without `server: null` causes each proxy connection to run `Mod.configure`, which can reset shared service-worker routes and clobber existing route overrides.</comment>
<file context>
@@ -231,38 +155,36 @@ async function handleConnection(
- await new Promise((resolve, reject) => {
- upstream.addEventListener("open", resolve, { once: true });
- upstream.addEventListener("error", reject, { once: true });
+ const cdp = new CDPModClient({
+ cdp_url: upstream,
+ extension_path: extensionPath,
</file context>
| const cdp = new CDPModClient({ | |
| cdp_url: upstream, | |
| extension_path: extensionPath, | |
| hydrate_aliases: false, | |
| }); | |
| const cdp = new CDPModClient({ | |
| cdp_url: upstream, | |
| extension_path: extensionPath, | |
| hydrate_aliases: false, | |
| server: null, | |
| }); |
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/python/demo.py">
<violation number="1" location="client/python/demo.py:178">
P2: Debugger-mode error handling now masks malformed `Browser.getVersion` success responses as expected rejections.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| version = expect_object(cdp.send("Browser.getVersion"), "Browser.getVersion") | ||
| if not isinstance(version.get("protocolVersion"), str) or not isinstance(version.get("product"), str): | ||
| raise RuntimeError(f"unexpected Browser.getVersion result {version}") | ||
| print(f"Browser.getVersion -> {version}") | ||
| except Exception as e: | ||
| print(f"Browser.getVersion -> (debugger route rejected: {str(e).splitlines()[0]} )") |
There was a problem hiding this comment.
P2: Debugger-mode error handling now masks malformed Browser.getVersion success responses as expected rejections.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/python/demo.py, line 178:
<comment>Debugger-mode error handling now masks malformed `Browser.getVersion` success responses as expected rejections.</comment>
<file context>
@@ -172,12 +175,12 @@ def on_pong(payload, *_):
try:
- cdp.send("Browser.getVersion")
- raise RuntimeError("Browser.getVersion unexpectedly succeeded in debugger mode")
+ version = expect_object(cdp.send("Browser.getVersion"), "Browser.getVersion")
+ if not isinstance(version.get("protocolVersion"), str) or not isinstance(version.get("product"), str):
+ raise RuntimeError(f"unexpected Browser.getVersion result {version}")
</file context>
| version = expect_object(cdp.send("Browser.getVersion"), "Browser.getVersion") | |
| if not isinstance(version.get("protocolVersion"), str) or not isinstance(version.get("product"), str): | |
| raise RuntimeError(f"unexpected Browser.getVersion result {version}") | |
| print(f"Browser.getVersion -> {version}") | |
| except Exception as e: | |
| print(f"Browser.getVersion -> (debugger route rejected: {str(e).splitlines()[0]} )") | |
| raw_version = cdp.send("Browser.getVersion") | |
| except Exception as e: | |
| print(f"Browser.getVersion -> (debugger route rejected: {str(e).splitlines()[0]} )") | |
| else: | |
| version = expect_object(raw_version, "Browser.getVersion") | |
| if not isinstance(version.get("protocolVersion"), str) or not isinstance(version.get("product"), str): | |
| raise RuntimeError(f"unexpected Browser.getVersion result {version}") | |
| print(f"Browser.getVersion -> {version}") |
Summary
Restores MagicCDP as the multi-client package layout and keeps the package name
magiccdpacross JS, Python, and Go.Changes
client/{js,python,go}.eventSchema,paramsSchema, andresultSchema.Validation
pnpm installpnpm typecheck