feat(mcp): add playwright preset to hermes mcp add - #68488
Open
ygd58 wants to merge 3 commits into
Open
Conversation
Ports NousResearch#63771 forward onto current main per teknium1's review. Adds @playwright/mcp to the built-in MCP preset registry alongside the existing codex preset. Per review: browser detection now reuses hermes_cli.browser_connect.get_chrome_debug_candidates() -- the same platform-aware resolver CDP-attach uses -- instead of a bespoke shutil.which() name list, so macOS .app bundles and Windows install directories (including the WSL /mnt/c mirror) are correctly detected as a present system browser rather than misclassified as absent. - codex preset preserved alongside playwright - --no-sandbox added only on Linux when euid==0 (root/VPS/Docker) - --browser=chromium fallback added only when get_chrome_debug_candidates() finds no system browser - Dropped the incorrect 'Fixes NousResearch#19760' reference (that issue is the unrelated, closed Nix npmDepsHash build failure) 9/9 new tests pass in TestPlaywrightMcpPreset; 67/67 in the full tests/hermes_cli/test_mcp_config.py file.
alt-glitch
added
type/feature
…eset Per teknium1's review of NousResearch#63771 ('add macOS/Windows non-PATH tests'), only the macOS .app-bundle case was added. Add the matching Windows standard-install-directory case, confirming the platform-aware get_chrome_debug_candidates() resolver -- not a bare shutil.which() name list -- is what determines browser availability on Windows too. 68/68 tests pass in tests/hermes_cli/test_mcp_config.py.
teknium1
reviewed
Jul 30, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for carrying the Playwright preset forward and reusing the existing platform-aware browser resolver.
Problems
- Blocking:
hermes_cli/mcp_config.py:59adds a bareos.geteuid()call. The PR's required Windows footguns CI job failed on this exact line becauseos.geteuiddoes not exist on native Windows, and the repository checker requires an explicithasattr/getattrguard even when the expression is short-circuited bysys.platform == "linux".
Suggested changes
- Make the root check explicitly Windows-safe with
getattrorhasattr, preserving the current Linux-root behavior, then rerunpython scripts/check-windows-footguns.py --all.
This is an automated hermes-sweeper review.
| extra: list[str] = [] | ||
|
|
||
| # Root/AppArmor safety flag (Linux-only). | ||
| if sys.platform == "linux" and os.geteuid() == 0: |
Contributor
There was a problem hiding this comment.
Blocking: the required Windows-footguns CI job rejects this bare os.geteuid() reference because that attribute is absent on native Windows. Please use an explicit getattr/hasattr guard while retaining the Linux-root condition.
teknium1
added
the
sweeper:blast-moderate
Follow-up per review of NousResearch#68488. sys.platform == "linux" short-circuits the bare os.geteuid() call at runtime, but the required windows-footguns CI checker (scripts/check- windows-footguns.py) rejects any bare os.geteuid/getuid/getgid reference statically, regardless of the surrounding guard -- it can't prove the short-circuit at the AST level, only recognizes an explicit hasattr/getattr guard. Fixed to hasattr(os, "geteuid") and os.geteuid() == 0, matching the established pattern already used for the identical Linux-root check elsewhere in the codebase (hermes_cli/main.py:5736). Verified: python scripts/check-windows-footguns.py --all now passes clean (0 footguns, 788 files scanned). 68/68 tests pass in the full tests/hermes_cli/test_mcp_config.py file (no regression).
Contributor
Author
|
Fixed -- changed to |
SummaryFifteen PRs address or reference this complex: twelve Nix changes range from one-line TUI/web/desktop hash refreshes through hash-check automation to the merged Related pull requests
Duplicates#17133 is the hash-only subset of #17174; #19764 and #19766 contain the same TUI hash change; #48213 and #48883 contain substantially the same Suggested consolidationKeep #68488 open with a salvage path: retain its platform-aware resolver reuse, preserved Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup19768 ["PRs duplicating each other"]
P19768["PR #19768 (closed)"]
P63771["PR #63771 (closed)"]
P68488["PR #68488 (open)"]
end
class P19768 closed
class P63771 closed
class P68488 open
class P68488 target
click P19768 "https://github.com/NousResearch/hermes-agent/pull/19768"
click P63771 "https://github.com/NousResearch/hermes-agent/pull/63771"
click P68488 "https://github.com/NousResearch/hermes-agent/pull/68488"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 15 pull requests and 5 issues in this complex. Each diff was read against this issue; Assessment working set: 79 kB of PR diffs, 45 kB of issue/PR text, 24 kB of discussion (47 comments), 47 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
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
4 participants
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.
Context
Ports #63771 forward onto current main per @teknium1's review.
Fix
Adds
@playwright/mcpto the built-in MCP preset registry alongside the existingcodexpreset. Browser detection now reuseshermes_cli.browser_connect.get_chrome_debug_candidates()-- the same platform-aware resolver CDP-attach uses -- instead of a bespokeshutil.which()name list, so macOS.appbundles and Windows install directories (including the WSL/mnt/cmirror) are correctly detected rather than misclassified as absent.codexpreset preserved alongsideplaywright--no-sandboxadded only on Linux wheneuid==0(root/VPS/Docker)--browser=chromiumfallback added only when no system browser is foundFixes #19760reference (unrelated, closed Nix build issue)Verification
9/9 new tests pass in
TestPlaywrightMcpPreset; 67/67 in the fulltests/hermes_cli/test_mcp_config.pyfile.