Skip to content

fix(dev-harness): resolve canonical Python venv - #10017

Merged
Git-on-my-level merged 2 commits into
BasedHardware:mainfrom
Git-on-my-level:fix/harness-python-venv
Jul 19, 2026
Merged

fix(dev-harness): resolve canonical Python venv#10017
Git-on-my-level merged 2 commits into
BasedHardware:mainfrom
Git-on-my-level:fix/harness-python-venv

Conversation

@Git-on-my-level

@Git-on-my-level Git-on-my-level commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Centralize local dev-harness interpreter resolution: explicit PYTHON, then backend/.venv/bin/python, legacy backend/venv/bin/python, then python3.
  • Route all harness commands and Makefile harness Python targets through the resolver; dev-init creates the canonical .venv when neither venv exists.
  • Add focused resolver coverage for precedence and fallback behavior.

Root cause and durable guard

Harness commands and Makefile targets had inconsistent hard-coded legacy backend/venv defaults. A single resolver owns precedence now, with a hermetic regression test covering every selection branch.

Verification

  • /Users/dazheng/workspace/omi/backend/.venv/bin/python -m pytest -q scripts/dev-harness/tests/test_python_resolver.py — 2 passed
  • bash -n for the resolver plus all 9 harness consumers — pass
  • git diff --cached --check — pass
  • make preflight — passed (9 deterministic checks)

Failure-Class

Failure-Class: none

Review in cubic

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b26150e6ac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Makefile
@Git-on-my-level

Copy link
Copy Markdown
Collaborator Author

Monitor summary — Codex P2 "Preserve shell quoting for resolved Python"

Addressed by the existing head commit 74c62e05 (fix(dev-harness): quote resolved Python commands):

  • Makefile now invokes the resolved interpreter as "$(PYTHON)" in list-memory-scenarios, seed-memory-scenario, reset-memory-scenario, and run-canonical-promotion, so checkouts whose absolute repo path contains spaces no longer split the venv path.
  • Added regression test test_make_harness_targets_run_resolved_python_from_checkout_with_spaces exercising all four targets from a checkout path containing spaces.

Verification (in the monitor worktree, rebased onto origin/main c55f2925):

  • git diff --check origin/main..HEAD — clean.
  • bash -n on _resolve_python.sh and all 9 harness consumers — pass.
  • python3 -m pytest -q scripts/dev-harness/tests/test_python_resolver.py — 3 passed.
  • GitHub CI on the PR head: all checks terminal (SUCCESS/SKIPPED), none failing.

Thread PRRT_kwDOLkKqys6SDeWW resolved as addressed. No merge performed (still REVIEW_REQUIRED).

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Dev-harness venv resolution fix. Approve-only (tooling).

@Git-on-my-level
Git-on-my-level merged commit c16c21d into BasedHardware:main Jul 19, 2026
22 checks passed
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