Skip to content

fix(installer): quote browser env override paths - #57449

Open
tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-browser-env-path-quoting
Open

tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-browser-env-path-quoting

Conversation

@tianma-if

Copy link
Copy Markdown
Contributor

Fixes #57247.

Summary

  • quote AGENT_BROWSER_EXECUTABLE_PATH when installers persist explicit browser overrides to .env
  • keep the stale Snap override cleanup working for quoted and unquoted values
  • add regression coverage for shell-sourcing spaced browser paths plus the PowerShell writer shape

Duplicate check

Tests

  • .venv/bin/python -m pytest tests/test_install_sh_browser_install.py -q -k "browser_override or snap"
  • .venv/bin/python -m pytest tests/test_install_sh_browser_install.py -q
  • .venv/bin/python -m ruff check tests/test_install_sh_browser_install.py
  • scripts/run_tests.sh tests/test_install_sh_browser_install.py -q

Note: pwsh is not installed in this local environment, so the PowerShell installer assertion is text-level coverage.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles tool/browser Browser automation (CDP, Playwright) labels Jul 3, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Competing open fixes for #57247 (unquoted AGENT_BROWSER_EXECUTABLE_PATH breaks shell source on spaced paths) — for a maintainer to pick one:

Same goal, different quoting mechanism — related_to, not a duplicate.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for addressing the real spaced-path installer failure: current main still writes the POSIX override unquoted at scripts/install.sh:2109.

Problems

  • The proposed single-quote splice is shell-safe but not safe for Hermes's runtime .env loader. Hermes calls python-dotenv at hermes_cli/env_loader.py:154-158 and :245-247; the linked discussion on fix(install): quote AGENT_BROWSER_EXECUTABLE_PATH for spaced paths #57249 documents that an apostrophe-containing path causes this form to drop the key under the pinned parser version. Please use escaped double quotes, consistent with hermes_cli/config.py:7610-7623.
  • The added tests inspect installer source text, including the PowerShell check. AGENTS.md:1358-1392 explicitly bans source-reading tests. They do not validate either the PowerShell writer or the runtime dotenv path.

Suggested changes

  • Cover a spaced path containing an apostrophe through both shell source and dotenv loading from a temporary HERMES_HOME.
  • Replace source-shape assertions with executable behavior coverage.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
@teknium1 teknium1 added the area/install-update Installer, updater, packaging, wheels, doctor label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows tool/browser Browser automation (CDP, Playwright) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Installer writes unquoted AGENT_BROWSER_EXECUTABLE_PATH override into .env — spaced paths break shell source

3 participants