fix(skills): persist durable uv and pipx interpreters - #3980
azizur100389 wants to merge 3 commits into
Conversation
Co-Authored-By: OpenAI Codex <noreply@openai.com>
|
Thanks for the pull request, @azizur100389. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
Co-Authored-By: OpenAI Codex <noreply@openai.com>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Graphify review — findings
Hardens the graphify skill's Step 1 interpreter detection so it saves only a persistent Python to .graphify_python, one that imports graphify and isn't an ephemeral uv archive-v cache. graphify_find_python probes the uv tool dir and pipx venvs (POSIX and Windows layouts) first, then a literal text-launcher shebang (never .exe or evaluated contents), then python3/python, and exits with an error rather than silently falling back when install or resolution fails. Every later step now quotes "$(cat graphify-out/.graphify_python)" so interpreter paths with spaces work, and the same resolver replaces the old which/head -1 re-resolution for subcommands when .graphify_python is missing.
Worth a look
- Existing .graphify_python is trusted and executed without validation for subcommands —
graphify/skill-agents.md:694· Escalate · high · 2 independent checks- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Workspace sidecar path is trusted as an executable interpreter —
graphify/skill-agents.md· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- save-result still splices question/answer text into double-quoted shell args —
graphify/skill-aider.md:1083· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Removed --break-system-packages fallback breaks pip-only installs on PEP 668 systems —
graphify/skill-aider.md:115· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1891 functions depend on the 1826 functions this change touches.
Health — this change adds coupling hotspots:
- new:
render()— 15 callers, 5 callees - new:
audit_coverage()— 8 callers, 6 callees - new:
run_fragment()— 6 callers, 6 callees - new:
main()— 3 callers, 11 callees - new:
monolith_roundtrip()— 3 callers, 5 callees - new:
test_audit_catches_a_dropped_non_allowlisted_heading()— 0 callers, 6 callees
Verification — 1891 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1891 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
313 of 313 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_blade_extractor.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_cluster_exclude_hubs.py— full-run-safetytests/test_cobol_extractor.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safety- … and 263 more
non-code file(s) changed (
graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.md,graphify/skill-codex.md…) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.md,graphify/skill-codex.md…) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Risky patterns (advisory)
- medium
no-stderr-to-stdoutintools/skillgen/gen.py:1272: stderr redirected to stdout: 'if [ -z "$PYTHON" ] && command -v uv >/dev/null 2>&1; then', - medium
no-stderr-to-stdoutintools/skillgen/gen.py:1274: stderr redirected to stdout: 'if command -v pipx >/dev/null 2>&1; then', - medium
no-stderr-to-stdoutintools/skillgen/gen.py:1275: stderr redirected to stdout: 'if command -v uv >/dev/null 2>&1; then', - medium
no-stderr-to-stdoutintools/skillgen/gen.py:1278: stderr redirected to stdout: 'uv tool install --upgrade graphifyy -q 2>&1 tail -3', - medium
no-stderr-to-stdoutintools/skillgen/gen.py:1280: stderr redirected to stdout: ' "$PYTHON" -m pip install graphifyy -q --break-system-packages 2>&1 tail -3',
Docs that may be stale (advisory)
ARCHITECTURE.md§ Testing (lines 94-102): references changed symbolsbashBENCHMARKS.md§ Reproducing (lines 174-187): references changed symbolsbashCHANGELOG.md§ 0.9.13 (2026-07-12) (lines 768-787): references changed symbolsbashCHANGELOG.md§ 0.9.8 (2026-07-06) (lines 843-853): references changed symbolsbashCHANGELOG.md§ 0.9.7 (2026-07-06) (lines 854-873): references changed symbolsbashCHANGELOG.md§ 0.8.1 (2026-05-15) (lines 1451-1462): references changed symbolsbashCHANGELOG.md§ 0.6.2 (2026-05-01) (lines 1703-1719): references changed symbolsbashCHANGELOG.md§ 0.3.21 (2026-04-09) (lines 1992-1996): references changed symbolsbashCONTRIBUTING.md§ Development Setup (lines 35-59): references changed symbolsbashREADME.md§ (preamble) (lines 1-73): references changed symbolsbash
…and 10 more.
· 1 grounded finding(s) anchored inline below; 5 more finding(s) on lines outside this diff (see the check run).
| '''"$REAL_PYTHON" -c 'import os, sys; sys.executable=os.environ["FAKE_PYTHON_PATH"]; exec(sys.argv[1])' "$2"''') | ||
|
|
||
|
|
||
| def run_fragment(tmp_path, fragment, *, uv_root="", pipx_root="", launcher=b"MZlauncher", shebang=None, valid_system=False): |
There was a problem hiding this comment.
run_fragment()
fans out to 6 callees (efferent coupling); 6 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Co-Authored-By: OpenAI Codex <noreply@openai.com>
This branch has not been deployed
1 participant
Closes #3943.
Step 1 and the subcommand guard now share a resolver that probes persistent uv/pipx tool environments, verifies imports, rejects uv archive-cache interpreters and skips PE launchers before any shebang read. The guard also repairs stale sidecars; installation or import failure stops before persisting a new path.
Interpreter invocations are quoted throughout POSIX skill fragments so Windows paths containing spaces remain usable. Generated hosts and snapshots are regenerated, PowerShell translation remains validated, and monolith allowances enumerate only the old/new resolver and quoting lines. Sixteen regressions execute the snippets for POSIX and Windows tool layouts, native Windows paths, missing imports, hostile shebangs and stale sidecars.
Validation: Ruff, pre-commit, all five skillgen validators, fresh wheel installation, installed-artifact smoke checks, CLI help/install and the AST graph refresh pass. Regression tests reproduce the defects on unchanged upstream and pass with these changes.
Full CI passes on Python 3.10 (6234 passed, 13 repository-defined skips) and 3.12/3.13/3.14 (6233 passed, 14 repository-defined skips each). Frozen dependencies include all extras; CLI help/install passes on every version. No test filters or new skips were added.
The full Windows Python 3.12 suite finished with 6152 passed, 37 failed and 58 repository-defined skips. All 37 failing test IDs were rerun on unchanged upstream with the same Git Bash environment and reproduced there; none are introduced by this PR. All 16 new interpreter regressions pass under Git Bash. An isolated real uv tool installation with spaces in its path persisted the durable tool interpreter, imported Graphify after the uv cache was removed, and passed the subcommand guard.
Pyright reports the same 586 baseline diagnostics with no introduced diagnostics. Bandit has the same 12 filtered baseline findings (4 high, 8 medium); pip-audit has the same 41 vulnerability records across 8 packages, matched by advisory aliases to upstream. Security CI is non-blocking in this repository, so its green status is not represented as a clean security audit. Dependencies and lockfile are unchanged.