fix(sandbox): point boxlite install advice at the running environment - #822
Conversation
The missing-boxlite advice told users to run pip install 'raven[sandbox]', which is wrong twice over: the raven name on package indexes resolves to an unrelated project, and a global pip install bypasses the uv environment raven actually runs in. Both places a user can hit the advice now build it with boxlite_install_hint(), which resolves the pinned boxlite requirement from the installed distribution metadata and names the running interpreter, e.g. uv pip install --python <venv>/bin/python boxlite==0.9.5. The init-failure messages that fire after the import already succeeded no longer suggest a reinstall; they keep the platform bullets naming the actual causes. The zh catalog entry follows the reworded onboarding line, with the command printed untranslatable on its own line.
|
Review of head 94219d5. The direction is right and CI is green. Two one-line fixes before merge; two follow-ups filed as issues, neither blocks this PR.
R1
monkeypatch.setattr("sys.executable", "/home/someone/.local/share/uv/tools/raven/bin/python")
# ... same questionary / _probe_boxlite / _failure_choice patches as the existing test ...
onboard_commands._step2_sandbox(skip=False, non_interactive=False)
assert boxlite_install_hint() in capsys.readouterr().outChecked on this head: red without the fix (the output holds R2
F1 (#844)
F2 (#845)
|
|
Thanks for the detailed review. R1 and R2 are addressed in 323d8b7:
Happy to adjust further if anything's off. |
…xlite_cli Address review R1/R2. R1: the onboarding hint is copy-pasteable, so print it with soft_wrap -- console.print hard-wraps at console width otherwise, and a long uv tool path splits the command in two. agents_commands.py already prints its copy-pasteable commands this way. Adds a test next to the missing-advice test that pins a long interpreter path and asserts the hint arrives in one piece. R2: scripts/boxlite_cli.py still advised `pip install raven[sandbox]` in two places; the script runs from a checkout, so print the documented `uv sync --extra sandbox` instead.
25caef9 to
323d8b7
Compare
|
Merged in 0cf70f5, and #640 is closed with it. Thank you, @yudongyouqing! This was a careful fix. The new advice installs boxlite into the interpreter Raven actually runs under, with the version taken from the package metadata instead of hard-coded, and the init-failure messages no longer tell people to reinstall something that is already installed. You also turned R1 and R2 around quickly, and the new test fails without the fix, which is exactly what we want. Thanks again, and you are very welcome to pick up more issues here. |
Summary
pip install 'raven[sandbox]', which is wrong twice over: theravenname on package indexes resolves to an unrelated project, and a global pip install bypasses the uv environment raven actually runs in (fix: raven boxlite recommends to install pip install raven[sandbox] is sloppy #640).boxlite_install_hint()toraven.sandbox: it resolves the pinned boxlite requirement from the installed distribution metadata (falling back to the bare name when metadata is absent) and renders a copy-pasteable command targeting the running interpreter, e.g.uv pip install --python /path/to/venv/bin/python boxlite==0.9.5.build_executor's ImportError message uses the same hint.Type
Verification
Commands run on macOS, Python 3.12, on top of a clean checkout of main:
uv run pytest tests/test_sandbox_unit.py tests/test_cli_onboard_commands.py tests/test_i18n_boundary.py tests/test_i18n_prompts.py -q-> 467 passed. The new tests (TestBoxliteInstallHint, TestInitFailureAdvice, one TestBuildExecutor case, one onboard screen case) were each watched failing before the fix.uv run --frozen --python 3.12 --all-extras pytest -q-> 27002 passed, 109 skipped, 21 failed. The 21 failures reproduce identically on unmodified main (verified by stashing this change):test_agents_research_tools.py(13),test_agents_research_verbatim_sink.py(5),test_simulation_scenario.pyandtest_simulation_suite.py(2),test_rpc_files.py::test_a_host_without_libreoffice_says_so(1, LibreOffice not installed locally). None touch the changed modules.uv run ruff checkandruff format --checkon the six touched files -> clean.make lint-imports-> 10 kept, 0 broken.make check-source-language-> pass.Risk
Security impact considered
Backward compatibility considered
Rollback path is clear for risky changes
User-visible change: three messages read differently (the onboarding hint, the build_executor ImportError, two executor init-failure messages). No API, config, or behavior change beyond the wording. The hint assumes uv on PATH, which every documented install path already requires.
Rollback is a straight revert of this commit.
Related Issues
Fixes #640