Skip to content

fix(admin): don't treat an enclosing repo as an editable install - #562

Open
ddy4633 wants to merge 1 commit into
browser-use:mainfrom
ddy4633:fix-repo-dir-ancestor-walk
Open

fix(admin): don't treat an enclosing repo as an editable install#562
ddy4633 wants to merge 1 commit into
browser-use:mainfrom
ddy4633:fix-repo-dir-ancestor-walk

Conversation

@ddy4633

@ddy4633 ddy4633 commented Jul 26, 2026

Copy link
Copy Markdown

The bug

_repo_dir() in src/browser_harness/admin.py walked every ancestor of the package looking for a .git directory:

for p in Path(__file__).resolve().parents:
    if (p / ".git").is_dir():
        return p

A wheel installed anywhere inside a git repository therefore gets classified as an editable clone. Two ordinary layouts hit this:

  • a venv inside the user's own project — project/.venv/lib/python3.x/site-packages/browser_harness/
  • uv tool install under a dotfiles-managed $HOME

run_update() then takes the clone branch: it runs git status and git pull --ff-only against the user's own repository, never performs the package upgrade, and still prints update complete. So browser-harness --update -y silently does nothing except touch an unrelated repo — and reports success.

The fix

Only two directories can plausibly hold this package as source: the package's own parent (flat layout) and its grandparent (src layout). Checking just those keeps editable-clone detection working for real clones while no longer claiming enclosing repositories.

How to test

python -m pytest tests/unit/test_admin.py -q     # 44 passed
python -m pytest tests/ -q                       # 119 passed

Five tests added. Three fail without the fix:

FAILED tests/unit/test_admin.py::test_repo_dir_ignores_repo_above_a_site_packages_install
FAILED tests/unit/test_admin.py::test_repo_dir_ignores_dotfiles_repo_above_a_tool_install
FAILED tests/unit/test_admin.py::test_run_update_of_installed_wheel_never_pulls_an_enclosing_repo

The other two pin the behaviour that must not change — a genuine editable clone in both flat and src layout is still detected.

Note

While reading the surrounding modules I noticed a separate, unrelated issue worth a maintainer's design input rather than a drive-by patch: in video_render.py, _start_export hard-codes Chrome's downloadPath to the recording directory and passes only webm.name, while the poll loop at :440-445 watches output.with_suffix(".webm") derived from --output. Any --output outside the recording directory times out after duration + 30s and orphans a fully-recorded file. There are two reasonable fixes (download into output.parent, or force the output into the recording dir), so I've left it alone — happy to file it as an issue if that's useful.


Summary by cubic

Fixes editable-install detection so wheels installed inside a git repo aren’t mistaken for editable clones. This prevents browser-harness --update -y from running git pull on a user’s repo and ensures the package actually upgrades.

  • Bug Fixes
    • _repo_dir() now only checks the package’s parent and grandparent for .git (flat and src layouts) instead of walking all ancestors.
    • run_update() for wheel installs no longer touches enclosing repos; it performs the upgrade (e.g., uv tool upgrade browser-harness).

Written for commit 1e49f5f. Summary will update on new commits.

Review in cubic

`_repo_dir()` walked every ancestor of the package looking for `.git`, so a
wheel installed anywhere inside a git repository was classified as an editable
clone. Two common layouts hit this:

- a venv inside the user's own project (`project/.venv/lib/.../browser_harness`)
- `uv tool install` under a dotfiles-managed `$HOME`

In both cases `browser-harness --update -y` ran `git status` and
`git pull --ff-only` against the *user's* repository, skipped the actual
package upgrade, and still printed "update complete."

Only two directories can hold this package as source: the package's parent
(flat layout) and its grandparent (src layout). Checking just those keeps
editable-clone detection working while no longer claiming unrelated repos.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

@Kastan97 Kastan97 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ollie (CTO) Review: APPROVED

What this does: Fixes wheel-install detection by checking for site-packages in the path rather than walking up to any enclosing .git directory. Prevents accidental git operations on enclosing repos when browser-harness is installed as a dependency.

Good to merge once CI passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants