|
| 1 | +- completed: 2026-08-26 |
| 2 | +- issue: none — like the review it follows, this began as an environment review |
| 3 | + in a mobile session rather than a filed prompt, so it carries no `active/` |
| 4 | + entry to fold. Recorded because the work shipped. |
| 5 | +- prs: |
| 6 | + - https://github.com/PyAutoLabs/PyAutoMind/pull/342 (merged, `4998a95c`) |
| 7 | + - https://github.com/PyAutoLabs/PyAutoBrain/pull/295 (merged, `ff553c08`) |
| 8 | +- classification-note: two repos. No gate order — `firewall_gate.yml` is |
| 9 | + path-filtered to `scripts/repos_sync.py`, which neither PR touches, so the |
| 10 | + Brain-before-Mind order the previous review needed did not apply. Merged |
| 11 | + Mind first to keep source ahead of its generated copy. |
| 12 | +- classification: bug (organism infrastructure; Mind + Brain) |
| 13 | +- summary: | |
| 14 | + A second pass over `complete/2026/08/mobile-performance-review.md`. Python |
| 15 | + 3.12 is now the session default — that review's fix held — but three |
| 16 | + defects underneath it were still live, and **both mitigations it shipped |
| 17 | + had holes**. The headline: the SessionStart hook has still never fired in a |
| 18 | + multi-repo session, and the fan-out written to fix that had never been |
| 19 | + installed by anyone, because writing it required the hook to already be |
| 20 | + running. |
| 21 | + |
| 22 | +## What was still true |
| 23 | + |
| 24 | +1. **The hook never fires in a multi-repo session** — proved, not inferred. |
| 25 | + `~/.claude/session-env/<session-id>/` holds an env file per session that ran |
| 26 | + the hook. Two earlier single-repo sessions in this container each have one; |
| 27 | + the multi-repo session's directory is **empty**. Corroborated by both clones |
| 28 | + still being shallow three minutes in, until an unrelated verb happened to |
| 29 | + call `session_bootstrap.sh`. |
| 30 | + |
| 31 | + The workspace-root fan-out was **unreachable by construction**. Writing it |
| 32 | + requires the hook to be running; the hook only runs where Claude Code |
| 33 | + registers it — a session whose project dir *is* a repo, i.e. a single-repo |
| 34 | + one; and `install_workspace_settings` returned early in exactly that case as |
| 35 | + "nothing to add". The one session type that could seed the container never |
| 36 | + did, and the session that needed the seed never ran the hook to write it. |
| 37 | + It now installs from any session, targeting `$WORKSPACE_ROOT` derived from |
| 38 | + the checkout, skipping only a root that is itself a repo or is not writable. |
| 39 | + |
| 40 | +2. **Both natural test commands failed, and neither looked environmental.** |
| 41 | + With no env file there is no PATH export, so shells resolved |
| 42 | + `/usr/local/bin/python3` (no pytest) and uv's *isolated* `pytest` (no |
| 43 | + PyYAML). The second is the dangerous one: four collection `ImportError`s |
| 44 | + naming `yaml`, in a workspace whose suite was green — it reads as broken |
| 45 | + source. |
| 46 | + |
| 47 | + | Command | Before | After | |
| 48 | + |---|---|---| |
| 49 | + | `python3 -m pytest` | `No module named pytest` | 232 passed | |
| 50 | + | `pytest` | 4 collection `ImportError`s | 223 passed | |
| 51 | + |
| 52 | + `point_system_default` was being handed `readlink -f "$VENV/bin/python"` — |
| 53 | + resolving the venv straight through to the base interpreter, satisfying the |
| 54 | + version question and losing everything else. |
| 55 | + |
| 56 | +3. **Tests ran on one of four cores.** No single slow test — 554 |
| 57 | + subprocess-heavy ones, the top fifteen summing to ~27s of 96s. |
| 58 | + `pytest-xdist` joins `BASE_DEPS`: PyAutoBrain 96s → 28s, PyAutoMind 10s → |
| 59 | + 3.7s, all passing either way. |
| 60 | + |
| 61 | +4. **`--check` reported `pytest: 3.12 OK` for the unusable pytest.** It asked |
| 62 | + the version — necessary, not sufficient — and so answered a question it had |
| 63 | + not asked, the same class the previous review fixed elsewhere. It now also |
| 64 | + asks whether the interpreter can import what the suite needs. |
| 65 | + |
| 66 | +## Key traps |
| 67 | + |
| 68 | +- **The fix did the disease, and destroyed the container's interpreter.** The |
| 69 | + wrapper was first written with `cat >"$dest"`. `/usr/local/bin/python3` is a |
| 70 | + *symlink*, and a redirect opens the link's **target** — so it overwrote |
| 71 | + `/usr/bin/python3.12` itself with the wrapper. The venv's own python symlinks |
| 72 | + to that same file, so the wrapper then exec'd itself: every `python3` in the |
| 73 | + container spun at 100% CPU and the interpreter was gone. Recovered with |
| 74 | + `uv python install 3.12`. The fix is `rm -f` before writing, plus a chain |
| 75 | + walk (`links_through`) refusing a target that reaches the destination by the |
| 76 | + other route. Both guards were confirmed to FAIL with the fix removed. |
| 77 | + |
| 78 | +- **A symlink cannot be the wrapper.** The first attempt at pointing the system |
| 79 | + default at the venv was `ln -s`. CPython resolves a symlinked executable |
| 80 | + *before* looking for `pyvenv.cfg`, so it lands on the base interpreter's |
| 81 | + prefix — the venv is lost again, silently, in the same shape as the bug being |
| 82 | + fixed. Only an `exec` wrapper keeps it. Pinned by a test asserting the |
| 83 | + destination is **not** a symlink. |
| 84 | + |
| 85 | +- **The endpoint comparison was too aggressive.** The first loop guard compared |
| 86 | + `readlink -f` of target and destination — but a venv's python legitimately |
| 87 | + *resolves* to the same base interpreter a system default points at, so it |
| 88 | + refused every safe rewrite. The question that matters is narrower: is the |
| 89 | + path being rewritten a *link in the target's own chain*. |
| 90 | + |
| 91 | +- **`--system-site-packages` is what makes the swap safe.** Pointing `python3` |
| 92 | + at an isolated venv would trade one set of missing modules for another. The |
| 93 | + venv is now a strict superset of the base interpreter, verified both ways. |
| 94 | + |
| 95 | +## Validation |
| 96 | + |
| 97 | +232 PyAutoMind tests and 554 PyAutoBrain tests, `ruff`, `lifecycle.py check` |
| 98 | +and `repos_sync.py --check` all clean. 8 new tests across the hook's new legs; |
| 99 | +every guard was confirmed to FAIL against the pre-fix behaviour before being |
| 100 | +trusted. Heart was unreachable (PyAutoHeart not checked out and `add_repo` |
| 101 | +blocked), so `ship_library.md` step 3's documented fallback stood in: per-repo |
| 102 | +`pytest -x`, any failure treated as RED. |
| 103 | + |
| 104 | +CI: PyAutoMind's only PR check on this diff is `spawn_drift.yml` (no path |
| 105 | +filter; the other three are correctly filtered out) — its `privacy` job runs |
| 106 | +`pytest tests/ -q` over the whole directory, so the new tests were covered. |
| 107 | +PyAutoBrain ran `tests.yml` green on both 3.12 and 3.13 legs. |
| 108 | + |
| 109 | +## Follow-up |
| 110 | + |
| 111 | +**PyAutoHeart and PyAutoHands carry stale generated copies of the hook.** |
| 112 | +`repos_sync.py --write` regenerated two of four because only two repos were |
| 113 | +attached, and `add_repo` for the other two was refused by the session's |
| 114 | +permission classifier. Not gated by either PR's CI (`firewall_gate.yml` is |
| 115 | +path-filtered to `scripts/repos_sync.py`), so this is silent drift until |
| 116 | +someone runs `--check` in a full workspace. The previous review logged the same |
| 117 | +lesson: **a drift check over N repos is only as strong as the number of them |
| 118 | +your session can see.** Twice now, so the constraint is the session shape, not |
| 119 | +an oversight. |
| 120 | + |
| 121 | +`draft/feature/pyautobrain/board_without_gh.md` is unaffected — its premise |
| 122 | +still holds. Its `(verify)` row on the GitHub token was re-checked this session: |
| 123 | +`$GH_TOKEN` is set and a direct REST call still returns "GitHub access is not |
| 124 | +enabled for this session". |
0 commit comments