-
Notifications
You must be signed in to change notification settings - Fork 584
feat: add MCP Chrome extension #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
4280890
to
dcce7ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
big change!
* @typedef {{tabId: number}} DebuggerTarget | ||
*/ | ||
|
||
function debugLog(...args) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like a debugging remnant. is there value in commiting it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably doesn't hurt!
|
||
mcpExtensionPage: async ({ mcpExtension, mcpHeadless }, use) => { | ||
if (!mcpExtension) | ||
return await use(undefined); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw an error instead, then you don't have to assert() on it on the calling side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startClient
uses this fixture, so if we remove the undefined
we would lie about the type and throw there? So not sure if that suggestion still applies.
ed6a65c
to
eaf5fa3
Compare
53be395
to
7040a41
Compare
Let's structure your code a bit differently:
|
7040a41
to
0698d71
Compare
this.dispatch({ id: message.id, result: versionInfo }); | ||
return; | ||
} | ||
if (message.method === 'Target.setAutoAttach' && !message.sessionId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If message.sessionId
exists, I assume it will just forward the request to chrome.debugger.sendCommand
, right? I had some issues in the past with Target.setAutoAttach
requests on pages with service workers:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And there was also issues with oopif that I had to address on Target.setAutoAttach
:
No description provided.