fix MCP path resolution (addresses #10), CDP connection limit, Playwright signature#12
Open
staylor wants to merge 1 commit into
Open
fix MCP path resolution (addresses #10), CDP connection limit, Playwright signature#12staylor wants to merge 1 commit into
staylor wants to merge 1 commit into
Conversation
This was referenced May 5, 2026
9365870 to
07bd045
Compare
Three small unrelated factual corrections, grouped because they are all docs-only one-liners. 1. MCP path resolution (addresses point 2 of lightpanda-io#10) - Claude Code CLI form: "$(command -v lightpanda)" resolves the absolute path at install time, regardless of where the binary lives. - JSON config form: literal /absolute/path/to/lightpanda placeholder with instruction to fill it in via command -v lightpanda. No per-OS examples (those would presuppose the install method, which varies). This does NOT close lightpanda-io#10 — only the second bullet of greenrd's issue. Point 1 (the 'skills' field in marketplace.json) is a different file this PR doesn't touch. 2. CDP connection limit The Important Notes bullet said 'Only 1 CDP connection per process'. Current Lightpanda defaults to 16 and exposes --cdp-max-connections N to tune it. Updates the bullet and adds the flag to the lightpanda serve options list. 3. Playwright connectOverCDP signature Both Playwright overloads (string and object) work in playwright-core 1.58, but Playwright's own JSDoc Usage examples and published docs use the string form (chromium.connectOverCDP('ws://...')). This is a style preference, not a bug — happy to drop or split out.
07bd045 to
3a1a1b5
Compare
Member
|
I tried to address point 1. into #15 but I agree your PR is better regarding that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Three small unrelated factual corrections to
SKILL.md, grouped because they're all docs-only one-liners.1. MCP path resolution (addresses point 2 of #10)
The current MCP setup snippets hard-code
$HOME/.local/bin/lightpanda, which has two problems:~/.local/bin.$HOMEdoesn't expand inside JSON config strings — this is the second bullet in greenrd's issue Fix Claude Code plugin #10.Fix:
"$(command -v lightpanda)"resolves the absolute path at install time, regardless of where the binary lives. A one-line comment reminds users to install Lightpanda first (the Install section above MCP setup)./absolute/path/to/lightpandaplaceholder with explicit instruction to fill it in viacommand -v lightpandaafter install. No per-OS path examples (those would presuppose the install method, which varies).This does not close #10 — only the second bullet. Point 1 ("the
skillsfield is supposed to point to a parent directory of a directory containing a SKILL.md file") is about.claude-plugin/marketplace.json, which this PR doesn't touch.2. CDP connection limit is now 16, not 1
The Important Notes bullet currently says:
This hasn't been true for a while — current Lightpanda defaults to 16 simultaneous connections per process and exposes
--cdp-max-connections Nto tune it. Updates the bullet text and adds the flag to thelightpanda serveoptions list.3. Playwright
connectOverCDPsnippet uses the canonical signatureBoth Playwright overloads (
connectOverCDP(endpointURL)andconnectOverCDP({ endpointURL })) are valid and both work inplaywright-core@1.58. The current example uses the object form. Switching to the string form matches what Playwright's own JSDoc Usage examples and published API docs lead with.This one is a style preference, not a bug. Happy to drop it if you'd prefer the object form, or split it into its own PR.
Verified
lightpanda serve --cdp-max-connections 32 --port 9222accepted by current Lightpanda binary.chromium.connectOverCDP('ws://127.0.0.1:9222')works underplaywright-core@1.58.2.command -v lightpandaresolves to brew's/opt/homebrew/bin/lightpandaon Apple Silicon (verified directly). The Linux path is the install script's deterministic install location (~/.local/bin/lightpandaby default, override viaLIGHTPANDA_DIR) — verified by construction, not direct execution.Stack
Independent of #11 (install.sh fixes) and #13 (
--block-private-networksdefaults) — all three branch offmaindirectly and can be reviewed/merged in any order.