diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d586ed4..07867b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,10 +98,14 @@ jobs: $failed = $false foreach ($f in $files) { $script = Get-Content -Raw $f.FullName - $noComments = ($script -split "`n" | ForEach-Object { $_ -replace ';.*$','' }) -join "`n" - $noStrings = $noComments -replace '"[^"\n]*"','' -replace "'[^'\n]*'", '' - $opens = ($noStrings.ToCharArray() | Where-Object { $_ -eq '{' }).Count - $closes = ($noStrings.ToCharArray() | Where-Object { $_ -eq '}' }).Count + # Strings FIRST, then comments. A ';' inside a string literal is not + # a comment -- `Loop Parse EnvGet("PATH"), ";" {` is valid AHK -- and + # stripping comments first truncates that line at the semicolon, + # taking the brace with it and failing correct code (SPEC.md B59). + $noStrings = $script -replace '"[^"\n]*"','' -replace "'[^'\n]*'", '' + $noComments = ($noStrings -split "`n" | ForEach-Object { $_ -replace ';.*$','' }) -join "`n" + $opens = ($noComments.ToCharArray() | Where-Object { $_ -eq '{' }).Count + $closes = ($noComments.ToCharArray() | Where-Object { $_ -eq '}' }).Count if ($opens -ne $closes) { Write-Host "FAIL $($f.Name): braces $opens / $closes"; $failed = $true } else { Write-Host "OK $($f.Name): braces $opens / $closes" } } @@ -125,7 +129,7 @@ jobs: run: | $ahk = Get-ChildItem -Path "C:\Program Files\AutoHotkey" -Recurse -Filter AutoHotkey64.exe -File -ErrorAction SilentlyContinue | Select-Object -First 1 if (-not $ahk) { Write-Host "AutoHotkey64.exe not found; skipping AHK unit tests"; exit 0 } - foreach ($test in @("tests/test_classify_clipboard.ahk", "tests/test_parse_mode.ahk")) { + foreach ($test in @("tests/test_classify_clipboard.ahk", "tests/test_parse_mode.ahk", "tests/test_pythonw_discovery.ahk")) { $errFile = Join-Path $env:RUNNER_TEMP "_ahk_test.err" $p = Start-Process -FilePath $ahk.FullName -ArgumentList @('/ErrorStdOut', $test) -RedirectStandardError $errFile -PassThru -Wait $e = Get-Content -Raw -LiteralPath $errFile -ErrorAction SilentlyContinue diff --git a/CHANGELOG.md b/CHANGELOG.md index c227bdf..43fdbde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ ## Unreleased +## 2.5.3 + +**Flowkey survives a Python upgrade.** Removing the interpreter Flowkey's virtual environment was built against broke every hotkey with a modal Windows dialog, and re-running the source installer repaired nothing. Found on a live machine. + +### Fixed + +- **"Python venv launcher is sorry to say ... did not find executable" on every hotkey.** A virtual environment's `Scripts\pythonw.exe` is not an interpreter — it is a ~250 KB stub that re-execs the interpreter recorded in `pyvenv.cfg`. Upgrading Python 3.13 to 3.14 uninstalls that interpreter but leaves the stub on disk, so the resolver's existence check still passed and every action spawned a dead launcher that hung on a modal dialog. The virtual environment is now accepted only while its base interpreter still exists, verified by reading `pyvenv.cfg` rather than by running the stub — running it to find out is precisely what raised the dialog. +- **Locating Python no longer assumes an install layout.** The rung below the virtual environment was the bare name `pyw.exe`, which the PSF Python Manager installer does not ship at all (it installs `pythonw.exe` under `%LOCALAPPDATA%\Python\bin`), so deleting the stale environment would only have moved the failure. Discovery now walks the PEP 514 registry entries every conformant Windows Python writes, newest 3.11+ first, and resolves `PATH` by hand so the zero-byte Microsoft Store alias stubs that shadow real installs are rejected rather than launched. +- **FastFlowLM reported as "not installed" on a machine where it was installed.** A process only ever sees the environment block built when its session started, and Flowkey normally launches at logon — so FastFlowLM installed (or repaired) afterwards appended itself to the machine `PATH` where Flowkey could never see it. `flm` answered fine from any new shell while `doctor` said `fastflowlm_cli: not found` and every hotkey failed. Provider CLIs are now located by an explicit resolver — `PATH`, then `PATH` as the registry currently holds it, then the known install directories — and `argv[0]` is passed as an absolute path. That last part matters on its own: Windows resolves a bare `argv[0]` against the *parent* process's `PATH`, so repairing the child's environment does not affect the lookup. Detection and execution now share one resolver, so `doctor` can no longer contradict the running app. +- **`flm validate` ran on the raw inherited environment.** It was the only `flm` call site that did not go through `flm_env()`, so it alone missed both the `PATH` repair above and the 2.5.2 `FLM_MODEL_PATH` repair. +- **`install.ps1` repairs an unhealthy virtual environment instead of reporting success.** It carried the same existence-is-health assumption, so re-running the installer on an affected machine printed "venv already present" and fixed nothing. It now checks that the environment's base interpreter exists, rebuilds it when it does not, and finds the interpreter to rebuild with using the same rung order as the app. + ## 2.5.2 **The local server starts after a reboot, and start-with-Windows works again.** Both were silent failures with no error anywhere; both were found on a live machine. diff --git a/README.md b/README.md index 019e22a..164ed13 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,14 @@ Flowkey is a Windows desktop assistant that adds local-LLM hotkeys for grammar f Everything runs locally through [FastFlowLM](https://fastflowlm.com) (AMD Ryzen AI NPU) or, on machines without the NPU, through [Ollama](https://ollama.com) (CPU/GPU) as a secondary provider. No cloud service, analytics, or telemetry is used by the app. -Current version: `2.5.2` +Current version: `2.5.3` + +## What's new in 2.5.3 + +- **Upgrading Python no longer breaks every hotkey.** Flowkey runs its Python helpers from a small virtual environment. When the Python that environment was built against is removed — exactly what a 3.13 to 3.14 upgrade does — the environment keeps a launcher stub that no longer works, and every hotkey stopped with a modal "Python venv launcher is sorry to say..." dialog that had to be dismissed by hand. Flowkey now notices the environment is dead and falls back to a working Python on the machine. +- **Flowkey finds Python however it was installed.** Its only fallback was one specific launcher that the newer Python installer does not ship at all, so a machine with a perfectly good Python could still fail. Flowkey now looks Python up the way Windows itself records it, and ignores the Microsoft Store placeholder that stands in for Python on `PATH` without being it. +- **Flowkey finds FastFlowLM even when Windows hasn't caught up.** If you install FastFlowLM while Flowkey is already running — or Flowkey starts at sign-in before Windows has published the change — FastFlowLM was invisible to Flowkey until you signed out and back in, showing as "not installed" on a machine where it plainly was. Flowkey now looks it up properly instead of trusting what it inherited at startup. +- **Re-running the source installer repairs a broken setup.** It previously reported "venv already present" and changed nothing, because it checked only whether the file was there — not whether it worked. ## What's new in 2.5.2 @@ -191,4 +198,5 @@ AutoHotkey tests are run by CI on Windows. Locally, run them with AutoHotkey v2: ```powershell & "C:\Program Files\AutoHotkey\v2\AutoHotkey64.exe" /ErrorStdOut tests\test_parse_mode.ahk & "C:\Program Files\AutoHotkey\v2\AutoHotkey64.exe" /ErrorStdOut tests\test_classify_clipboard.ahk +& "C:\Program Files\AutoHotkey\v2\AutoHotkey64.exe" /ErrorStdOut tests\test_pythonw_discovery.ahk ``` diff --git a/SPEC.md b/SPEC.md index fcd8b43..bfd7794 100644 --- a/SPEC.md +++ b/SPEC.md @@ -127,6 +127,8 @@ Caveman-encoded (compression, not amputation). Paths / ids / action names / numb - V64: cached update-check result past TTL ⊥ presented as current fact — flagged `stale` (incl. network-failure fallback to disk) ∧ UI labels it ∧ triggers one background forced refresh; ⊥ blocking the tab - V66: ∀ `flm` child spawned by Flowkey gets `flm_env()`: inherited `FLM_MODEL_PATH` pointing into `config\systemprofile` → replaced w/ `~/.flm`; unset → left unset (⊥ invent a path); sane value → untouched - V67: autostart Run value MUST name a resolvable exe (absolute-exists ∨ on PATH) — ⊥ bare name we haven't resolved; `get_autostart_state` reports `valid`; daemon startup REPAIRS an enabled-but-unlaunchable entry (repair-only, ⊥ create what the user never enabled) +- V68: python/pythonw discovery ⊥ assume install layout ∨ launcher name. probe order = `GRAMMARFIX_PYTHONW` → `scripts\.venv` (iff `pyvenv.cfg` base ∃) → PEP 514 registry (3.11+, newest, HKCU≻HKLM) → PATH `pyw.exe`/`pythonw.exe`. candidate valid ⟺ ∃ ∧ size>0 (⊥ 0-byte WindowsApps alias stub). venv health = STATIC `pyvenv.cfg` read, ⊥ spawn ∴ a dead venv can never raise its own modal dialog merely to be detected. `install.ps1` ∧ `daemon_client.ahk` share the rung order +- V69: provider CLI (`flm`/`ollama`) argv[0] = ABSOLUTE path via `resolve_cli` (PATH → registry machine+user PATH → known install dirs); ⊥ bare name ∵ Windows resolves argv[0] against the PARENT's PATH ∴ an `env=` PATH repair alone ⊥ suffice. detection (`provider_status`) ∧ execution share ONE resolver ∴ `doctor` ⊥ contradict the app. `flm_env()` repairs PATH ∀ flm child (∧ keeps the B54 `FLM_MODEL_PATH` repair) - V65: force re-pull of an installed model = provider-correct: FLM `pull` only fetches when ABSENT ∴ force ⇒ remove-then-pull (DESTRUCTIVE on download failure → error says the model is now uninstalled + retry); ollama `pull` already re-fetches on digest change ∴ ⊥ remove. UI confirms before sending force ## §T tasks @@ -236,4 +238,8 @@ B54|2026-08-31|THE actual root cause of the "exited early (exit 1)" saga, found B55|2026-08-31|self-inflicted: 2.5.1 implemented force re-pull as remove-then-pull ∵ I read `flm pull --help | head -20`, which TRUNCATED the option list. `flm --force` exists ("Force re-download even if model exists") ∧ is non-destructive ∴ I shipped a needlessly destructive path + a scary warning|V65; use `flm pull --force`; drop the remove + the "no longer installed" wording; retarget the 2 tests that pinned the destructive contract. Lesson: never conclude a CLI lacks a flag from truncated `--help` B56|2026-08-31|autostart silently stopped working: `_autostart_command_line()` fell back to the BARE string `"AutoHotkey64.exe"` whenever the installed layout (`APP_DIR\ahk`) was absent — i.e. ∀ dev/source trees, where AHK lives in `vendor\ahk` — ∧ AHK ⊥ on PATH ∴ Windows launched nothing at logon while the Run value still read as "enabled". ⊥ error anywhere|V67; probe `APP_DIR\ahk` → `APP_DIR\vendor\ahk` → `shutil.which`, else return "" (never an unresolved bare name); `get_autostart_state` gains `valid`; daemon repairs an enabled-but-unlaunchable entry at startup (repair-only). Live-verified: entry self-healed to the vendored path, `valid: True` B52|2026-08-27|dashboard advertised "FastFlowLM v0.9.45 → v0.9.46 available" from a 14-DAY-old cache (real latest 1.0.3): `cache_only` read serves an expired cache verbatim ∧ UI rendered it as current fact; network-failure fallback also served disk while reporting `cached:false` ∧ ⊥ `stale`|V64; mark `stale` on the network-failure fallback (+ carry `checked_at`, `cached:true`); UI labels stale reads "(cached — rechecking…)" ∧ fires ONE background forced refresh so the label self-corrects ⊥ blocking the tab +B57|2026-09-14|live: ∀ hotkey popped a modal `Python venv launcher is sorry to say ... did not find executable at '...\Programs\Python\Python313\pythonw.exe'` ∧ hung there (one stuck `pythonw.exe` per action). cause: a 3.13→3.14 upgrade (PSF Python Manager) uninstalled the base interpreter, leaving `Python313\` a husk (`Lib`/`Scripts`/`share`, ⊥ `python.exe`) — but `scripts\.venv\Scripts\pythonw.exe` survived ∵ it is only a ~250KB STUB that re-execs the path in `pyvenv.cfg`. `ResolvePythonwPath_Impl` accepted it on bare `FileExist` ∴ AHK spawned a dead launcher ∀ time. Worse, the rung below it was the bare name `pyw.exe`, which Python Manager ⊥ ship AT ALL (it installs `pythonw.exe` under `%LOCALAPPDATA%\Python\bin`) ∴ deleting the venv would have failed a 2nd time, differently. `install.ps1` carried the SAME existence-≠-health blind spot (`Test-Path $venvPythonw` → "venv already present") ∴ re-running the installer repaired nothing. Same family as B10 (stale `pytest.exe` shim outliving its interpreter)|V68; portable discovery in BOTH `daemon_client.ahk` ∧ `install.ps1`: static `pyvenv.cfg` base check, PEP 514 registry sweep, 0-byte alias-stub veto, PATH resolved by hand; `install.ps1` rebuilds an unhealthy venv instead of reporting success. New `tests/test_pythonw_discovery.ahk` (12 asserts) pins the dead-venv + alias-stub cases. Live-verified on the broken machine: dead venv rejected ⊥ spawning it, resolved → `pythoncore-3.14-64\pythonw.exe` via registry +B58|2026-09-14|live: FLM 1.0.5 installed, working, ∧ `C:\Program Files\flm` ∈ MACHINE PATH — but the app reported it absent: `doctor` → `fastflowlm_cli: not found`, `flm_validate: flm CLI not in PATH`, ∀ hotkey dead. ∵ a process only ever sees the env block built at SESSION START ∧ Flowkey launches at logon ∴ an FLM installed after that is invisible until sign-out; `shutil.which("flm")` ∧ bare-name argv[0] both read that stale PATH. Nothing was broken except the lookup (`flm version` answered fine from any new shell). Same family as B57 (Python) ∧ B10: trusting an ambient PATH. SELF-CORRECTION mid-fix: the first attempt repaired only `flm_env()["PATH"]` ∧ `provider_status` — `fastflowlm_cli` went green while `model_installed`/`flm_validate` STILL failed, ∵ **Windows resolves argv[0] against the PARENT's PATH ∴ `env=` ⊥ affect that lookup at all**. Caught by re-running live `doctor`, ⊥ by reasoning|V69; `subprocess_util.resolve_exe`/`resolve_cli`; ∀ 9 provider-CLI argv[0] sites absolutised (flm_server serve/list/version, benchmark, provider_runtime pull/remove, pull, first_run, grammar_fix validate, install); `provider_status` shares the resolver; `grammar_fix`'s `flm validate` gains `env=flm_env()` — it was the SOLE flm call site on the raw inherited env ∴ it also silently missed B54. New `tests/test_cli_discovery.py` (11). Live-verified under a stale PATH (`flm` ⊥ resolvable in the launching shell): doctor all-green (`model_installed: yes`, `flm_validate: ready=True`) + a real grammar fix end-to-end +B59|2026-09-14|PR #46 CI red: the AHK brace-balance gate reported `daemon_client.ahk: braces 54 / 55` on code AHK itself parses clean. The gate stripped COMMENTS BEFORE STRINGS (`;.*$` per line, then quoted spans) ∴ `Loop Parse EnvGet("PATH"), ";" {` — a semicolon inside a STRING — was truncated at that `;`, taking the line's `{` with it. A false positive on valid code, ∧ it would fire for any `;` in a literal. Same shape as B48: a brace/comment heuristic mis-reading syntax it doesn't actually parse|swap the order — strings stripped first, then comments. Verified over ∀ 10 `.ahk` files: every pre-existing count UNCHANGED (grammarFix 84/84, tray 34/34, ...) ∧ daemon_client now 55/55. Fixed the gate, ⊥ contorted the source to suit it +B60|2026-09-14|PR #46 automated review, both valid: (1) `ResolvePythonwPath_Impl` cached the resolved interpreter for the whole session w/ ⊥ revalidation ∴ a Python upgrade mid-session pinned the dead path until AHK itself restarted — reintroducing B57 for exactly the long-lived sessions Flowkey has (it autostarts at logon). (2) `install.py` `_has_cmd` was still `shutil.which` ∧ gates ∀ FLM path ∴ it rejected `flm` BEFORE `resolve_cli` could ever run — `postreboot` opened the FLM download page for an already-installed FLM. Detection ⊥ agreeing w/ execution is precisely what V69 forbids, violated in a file I had just edited: I converted the argv ∧ left the guard|V68/V69; cache revalidates on read — file stats ONLY, ⊥ spawn (∴ a dead venv still can't raise its own dialog) — ∧ walks up from `\Scripts\pythonw.exe` to re-check `pyvenv.cfg`, covering GRAMMARFIX_PYTHONW-supplied venvs too; `_has_cmd` → `resolve_exe`. SELF-CAUGHT while testing: my own AHK fixture wrote a 0-BYTE venv stub ∴ the alias-stub veto fired first ∧ the new dead-venv asserts would have passed for the wrong reason — exposed by the live-venv case failing; fixture now writes content ``` diff --git a/installer/install.ps1 b/installer/install.ps1 index 840f7b2..4a73c5d 100644 --- a/installer/install.ps1 +++ b/installer/install.ps1 @@ -86,10 +86,20 @@ function Test-Command([string]$Name) { return [bool](Get-Command $Name -ErrorAction SilentlyContinue) } -function Test-PythonOk { - if (-not (Test-Command "python")) { return $false } +# A path is a real program only if it exists AND has content. Windows "App +# Execution Alias" stubs under WindowsApps are 0-byte reparse points that open +# the Microsoft Store instead of running Python, and they sit on PATH ahead of +# real installs -- so `python` resolving is not proof that Python is installed. +function Test-RealExe([string]$Path) { + if (-not $Path) { return $false } + $item = Get-Item -LiteralPath $Path -Force -ErrorAction SilentlyContinue + return ($item -and -not $item.PSIsContainer -and $item.Length -gt 0) +} + +function Test-PythonVersionOk([string]$Exe) { + if (-not (Test-RealExe $Exe)) { return $false } try { - $v = & python --version 2>&1 + $v = & $Exe --version 2>&1 if ($v -match "Python (\d+)\.(\d+)") { return ([int]$matches[1] -eq 3 -and [int]$matches[2] -ge 11) } @@ -97,6 +107,69 @@ function Test-PythonOk { return $false } +# PEP 514: every conformant Windows Python registers +# \SOFTWARE\Python\\\InstallPath +# with (default) = install dir and, where supported, ExecutablePath. This is the +# only install-layout-independent way to find an interpreter -- python.org drops +# py.exe in C:\Windows, PSF Python Manager ships no py/pyw launcher at all, and +# the Store build hides behind alias stubs. Newest version first. +function Get-RegistryPythonExes { + $found = foreach ($root in 'HKCU:\SOFTWARE\Python', 'HKLM:\SOFTWARE\Python', 'HKLM:\SOFTWARE\WOW6432Node\Python') { + if (-not (Test-Path $root)) { continue } + foreach ($key in (Get-ChildItem -Path $root -Recurse -ErrorAction SilentlyContinue)) { + if ($key.PSChildName -ne 'InstallPath') { continue } + $tag = Split-Path -Leaf (Split-Path -Parent $key.Name) + if ($tag -notmatch '(\d+)\.(\d+)') { continue } + $ver = [int]$matches[1] * 100 + [int]$matches[2] + $exe = $key.GetValue('ExecutablePath') + if (-not $exe) { + $dir = $key.GetValue('') + if ($dir) { $exe = Join-Path $dir 'python.exe' } + } + if ($exe) { [pscustomobject]@{ Version = $ver; Exe = $exe } } + } + } + $found | Sort-Object Version -Descending | Select-Object -ExpandProperty Exe +} + +# Mirrors grammarFix.ahk's DiscoverPythonwPath_Impl() rung order so the +# installer and the running app never disagree about which Python is in play. +function Resolve-PythonExe { + $candidates = @() + $onPath = Get-Command python -ErrorAction SilentlyContinue + if ($onPath) { $candidates += $onPath.Source } + $pyLauncher = Get-Command py -ErrorAction SilentlyContinue + if ($pyLauncher) { + $viaPy = & $pyLauncher.Source -3 -c "import sys; print(sys.executable)" 2>$null + if ($viaPy) { $candidates += "$viaPy".Trim() } + } + $candidates += Get-RegistryPythonExes + foreach ($candidate in $candidates) { + if (Test-PythonVersionOk $candidate) { return $candidate } + } + return $null +} + +# A venv's Scripts\*.exe are ~250 KB stubs that re-exec the interpreter named in +# pyvenv.cfg. Uninstalling that interpreter (a 3.13 -> 3.14 upgrade does exactly +# that) leaves the stubs on disk, so a Test-Path check reports a dead venv as +# healthy -- and the app then pops a modal "Python venv launcher is sorry to +# say ... did not find executable" dialog on every hotkey. See SPEC.md B57. +function Test-VenvHealthy([string]$VenvDir) { + $cfg = Join-Path $VenvDir "pyvenv.cfg" + if (-not (Test-Path (Join-Path $VenvDir "Scripts\pythonw.exe"))) { return $false } + if (-not (Test-Path $cfg)) { return $false } + $baseHome = $null + $baseExe = $null + foreach ($line in (Get-Content -LiteralPath $cfg -ErrorAction SilentlyContinue)) { + if ($line -match '^\s*executable\s*=\s*(.+?)\s*$') { $baseExe = $matches[1] } + elseif ($line -match '^\s*home\s*=\s*(.+?)\s*$') { $baseHome = $matches[1] } + } + if ($baseExe) { return (Test-Path -LiteralPath $baseExe) } + if ($baseHome) { return (Test-Path -LiteralPath (Join-Path $baseHome 'python.exe')) } + return $false +} + function Update-SessionPath { # Pull the freshly-written machine + user PATH into this process so tools # installed seconds ago (python, flm) become visible without a new shell. @@ -148,9 +221,8 @@ if ($releaseRoot -like "$env:ProgramFiles*" -or $releaseRoot -like "${env:Progra # ---- 1. Python --------------------------------------------------------------- Info "Step 1/6: Python 3.11+" -if (Test-PythonOk) { - Ok "$(& python --version 2>&1)" -} else { +$pythonExe = Resolve-PythonExe +if (-not $pythonExe) { if (-not (Test-Command "winget")) { throw "Python 3.11+ not found and winget is unavailable. Install Python from " + "https://www.python.org/downloads/windows/ (tick 'Add to PATH'), then re-run." @@ -159,24 +231,32 @@ if (Test-PythonOk) { winget install --id Python.Python.3.13 --silent --scope user ` --accept-package-agreements --accept-source-agreements Update-SessionPath - if (-not (Test-PythonOk)) { - throw "Python installed but 'python' isn't on PATH yet. Close this window, open a " + - "new one, and re-run install.ps1." + $pythonExe = Resolve-PythonExe + if (-not $pythonExe) { + throw "Python installed but no 3.11+ interpreter could be found on PATH, via the " + + "py launcher, or in the registry. Close this window, open a new one, and " + + "re-run install.ps1." } - Ok "$(& python --version 2>&1)" } +Ok "$(& $pythonExe --version 2>&1) [$pythonExe]" # ---- 2. venv ----------------------------------------------------------------- -# scripts\.venv is what grammarFix.ahk's ResolvePythonwPath_Impl() probes for. +# scripts\.venv is what grammarFix.ahk's DiscoverPythonwPath_Impl() probes for. # No pip install: deps are stdlib-only and the daemon/wizard/chat all launch by # file path (pythonw ), so a bare venv interpreter is sufficient. +# Health, not mere presence: a venv whose base interpreter was uninstalled is +# worse than no venv at all, so rebuild it instead of reporting success. Info "Step 2/6: virtualenv at scripts\.venv" -if (Test-Path $venvPythonw) { +if (Test-VenvHealthy $venvDir) { Ok "venv already present." } else { - & python -m venv "$venvDir" - if ($LASTEXITCODE -ne 0 -or -not (Test-Path $venvPythonw)) { - throw "venv creation failed (expected $venvPythonw)." + if (Test-Path $venvDir) { + Info "Existing venv points at an interpreter that is gone -- rebuilding." + Remove-Item $venvDir -Recurse -Force + } + & $pythonExe -m venv "$venvDir" + if ($LASTEXITCODE -ne 0 -or -not (Test-VenvHealthy $venvDir)) { + throw "venv creation failed (expected a working $venvPythonw)." } Ok "Created venv -> AHK will auto-detect $venvPythonw" } diff --git a/installer/installer.iss b/installer/installer.iss index 8e04188..3db675a 100644 --- a/installer/installer.iss +++ b/installer/installer.iss @@ -41,7 +41,7 @@ #define AppURL "https://github.com/agr77one/Fastflow" #define AppExeName "Flowkey.exe" ; symbolic — actual launchers below ; Keep in lockstep with scripts\_version.py. -#define AppVersion "2.5.2" +#define AppVersion "2.5.3" [Setup] AppId={{8A4F1E6C-9B3D-4E62-9F7A-FASTFLOW140}} diff --git a/pyproject.toml b/pyproject.toml index cd582fd..2f79b5a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,7 @@ build-backend = "setuptools.build_meta" [project] name = "fastflowprompt" -version = "2.5.2" +version = "2.5.3" description = "Local-LLM-powered grammar fix, prompt rewrite, chat, and dashboard for Windows." readme = "README.md" requires-python = ">=3.11" diff --git a/scripts/_version.py b/scripts/_version.py index f43b199..3a87840 100644 --- a/scripts/_version.py +++ b/scripts/_version.py @@ -1,3 +1,3 @@ """Single source of truth for the app version. Read by grammar_fix.py.""" -__version__ = "2.5.2" +__version__ = "2.5.3" diff --git a/scripts/ffp_benchmark.py b/scripts/ffp_benchmark.py index 5a9659c..a453a17 100644 --- a/scripts/ffp_benchmark.py +++ b/scripts/ffp_benchmark.py @@ -24,7 +24,7 @@ from pathlib import Path import ffp_flm_server -from subprocess_util import run_hidden +from subprocess_util import resolve_cli, run_hidden log = logging.getLogger("ffp.benchmark") @@ -119,7 +119,7 @@ def cell(raw: list[str], i: int): def _default_runner(model: str, work: Path, no_window: int) -> str: """Run the real `flm bench ` in `work` so the CSV lands there.""" result = run_hidden( - ["flm", "bench", model], + [resolve_cli("flm"), "bench", model], env=ffp_flm_server.flm_env(), cwd=str(work), timeout=5400, # 90 min hard cap; large-context sweeps can be slow diff --git a/scripts/ffp_flm_server.py b/scripts/ffp_flm_server.py index 901b062..4b80227 100644 --- a/scripts/ffp_flm_server.py +++ b/scripts/ffp_flm_server.py @@ -14,7 +14,7 @@ from dataclasses import dataclass from pathlib import Path -from subprocess_util import popen_hidden, run_hidden +from subprocess_util import popen_hidden, resolve_cli, run_hidden, search_path log = logging.getLogger("ffp.flmserver") @@ -149,6 +149,13 @@ def flm_env() -> dict: than depending on the machine's environment being sane (B54). """ env = dict(os.environ) + # PATH repair, always -- FLM's installer appends its install directory to + # the MACHINE PATH, but a process only ever sees the environment block + # built when its session started. Flowkey normally launches at logon, so an + # FLM installed or repaired afterwards stays invisible until the user signs + # out: `flm` reads as "not installed" on a machine where it is installed + # and on the machine PATH (B58). + env["PATH"] = search_path() configured = (env.get("FLM_MODEL_PATH") or "").strip() if not configured: return env @@ -222,7 +229,7 @@ def start_flm_server( perf_mode = settings.performance_mode if settings.performance_mode in {"balanced", "max"} else "balanced" pmode = PERF_TO_PMODE.get(perf_mode, "turbo") args = [ - "flm", + resolve_cli("flm"), "serve", settings.model, "--pmode", @@ -349,7 +356,7 @@ def flm_list(filter_kind: str, model: str, no_window: int) -> dict: return {"error": f"bad filter: {filter_kind}", "models": [], "active": model} try: result = run_hidden( - ["flm", "list", "--json"], + [resolve_cli("flm"), "list", "--json"], env=flm_env(), timeout=15, creationflags=no_window, @@ -409,7 +416,7 @@ def flm_version(no_window: int) -> str: """ try: result = run_hidden( - ["flm", "version", "--json"], + [resolve_cli("flm"), "version", "--json"], env=flm_env(), timeout=10, creationflags=no_window, diff --git a/scripts/ffp_provider_runtime.py b/scripts/ffp_provider_runtime.py index ad3806b..9c69174 100644 --- a/scripts/ffp_provider_runtime.py +++ b/scripts/ffp_provider_runtime.py @@ -9,7 +9,7 @@ import ffp_flm_server import ffp_provider_status -from subprocess_util import run_hidden +from subprocess_util import resolve_cli, run_hidden log = logging.getLogger("ffp.provider") @@ -94,7 +94,7 @@ def pull_model(provider: str, model: str, no_window: int, *, timeout: int = 900, if not name: raise ValueError("model name is empty") cli = "ollama" if provider == "ollama" else "flm" - argv = [cli, "pull", name] + argv = [resolve_cli(cli), "pull", name] if force and provider != "ollama": # `flm pull` alone only downloads "if not present"; FLM's own # `--force` re-downloads without deleting first (B55). `ollama pull` @@ -115,7 +115,7 @@ def remove_model(provider: str, model: str, no_window: int, *, timeout: int = 60 raise ValueError("model name is empty") cli = "ollama" if provider == "ollama" else "flm" command = "rm" if provider == "ollama" else "remove" - result = run_hidden([cli, command, name], timeout=timeout, creationflags=no_window, + result = run_hidden([resolve_cli(cli), command, name], timeout=timeout, creationflags=no_window, env=ffp_flm_server.flm_env()) output = (result.stdout or "") + (result.stderr or "") if result.returncode != 0: diff --git a/scripts/ffp_provider_status.py b/scripts/ffp_provider_status.py index 2d1217d..798fa82 100644 --- a/scripts/ffp_provider_status.py +++ b/scripts/ffp_provider_status.py @@ -8,10 +8,11 @@ from __future__ import annotations -import shutil import socket from dataclasses import dataclass +from subprocess_util import resolve_exe + @dataclass(frozen=True) class ProviderSpec: @@ -81,7 +82,11 @@ def provider_status(provider: str, *, base_url: str = "") -> dict: key = str(provider or "").strip().lower() spec = PROVIDERS.get(key) or PROVIDERS["fastflowlm"] effective_url = str(base_url or spec.base_url).strip().rstrip("/") - cli_path = shutil.which(spec.cli) or "" + # resolve_exe, not shutil.which: `which` only sees the PATH this process + # inherited at session start, which is how an installed FLM reports as + # missing (B58). Detection has to agree with what flm_env() will actually + # run, or doctor and the wizard contradict the app. + cli_path = resolve_exe(spec.cli) installed = bool(cli_path) reachable = is_reachable(effective_url) return { diff --git a/scripts/ffp_pull.py b/scripts/ffp_pull.py index 521099f..118cb08 100644 --- a/scripts/ffp_pull.py +++ b/scripts/ffp_pull.py @@ -17,7 +17,7 @@ from collections.abc import Callable import ffp_flm_server -from subprocess_util import NO_WINDOW +from subprocess_util import NO_WINDOW, resolve_cli log = logging.getLogger("ffp.pull") @@ -54,7 +54,7 @@ def _default_runner( ) -> int: is_ollama = str(provider).strip().lower() == "ollama" cli = "ollama" if is_ollama else "flm" - argv = [cli, "pull", model] + argv = [resolve_cli(cli), "pull", model] if force and not is_ollama: # `flm pull` alone downloads only "if not present", so re-pulling an # installed model is a silent no-op. FLM exposes `--force` for exactly diff --git a/scripts/first_run.py b/scripts/first_run.py index 07e1663..752dc38 100644 --- a/scripts/first_run.py +++ b/scripts/first_run.py @@ -35,6 +35,7 @@ import ffp_provider_status import paths as _paths from loopback_http import daemon_headers, json_get, json_post +from subprocess_util import resolve_cli log = logging.getLogger("ffp.first_run") @@ -546,7 +547,7 @@ def on_pull_model(self) -> None: def worker() -> None: try: proc = subprocess.Popen( - [cli, "pull", model], + [resolve_cli(cli), "pull", model], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0), ) diff --git a/scripts/grammar_fix.py b/scripts/grammar_fix.py index a437d45..b9af9f3 100644 --- a/scripts/grammar_fix.py +++ b/scripts/grammar_fix.py @@ -30,7 +30,7 @@ import ffp_telemetry import ffp_updater import paths as _paths -from subprocess_util import NO_WINDOW +from subprocess_util import NO_WINDOW, resolve_cli try: from _version import __version__ as APP_VERSION @@ -711,7 +711,10 @@ def run_doctor() -> str: ahk_path = "" checks.append(("autohotkey", ahk_path or "not found in PATH")) try: - val = subprocess.run(["flm", "validate", "--json"], capture_output=True, text=True, timeout=15, check=False, creationflags=_NO_WINDOW) + # env=flm_env(): this was the only `flm` call site running on the raw + # inherited environment, so it alone missed both the PATH repair (B58) + # and the FLM_MODEL_PATH repair (B54). + val = subprocess.run([resolve_cli("flm"), "validate", "--json"], capture_output=True, text=True, timeout=15, check=False, creationflags=_NO_WINDOW, env=ffp_flm_server.flm_env()) if val.returncode == 0 and val.stdout.strip(): try: vdata = json.loads(val.stdout) diff --git a/scripts/install.py b/scripts/install.py index 5eef718..39c072a 100644 --- a/scripts/install.py +++ b/scripts/install.py @@ -30,6 +30,7 @@ import ffp_config import paths as _paths +from subprocess_util import resolve_cli, resolve_exe HERE = Path(__file__).resolve().parent @@ -57,7 +58,13 @@ def _step(msg: str) -> None: # ---------- Prereq detection ------------------------------------------------- def _has_cmd(name: str) -> bool: - return shutil.which(name) is not None + # resolve_exe, not shutil.which: `which` only sees the PATH this process + # inherited, so an FLM installed after the shell started reads as missing + # and postreboot() opens its download page for software already present. + # Detection has to agree with the resolver the call sites launch through, + # or the guard rejects the command before resolve_cli is ever reached + # (V69 / B58). + return bool(resolve_exe(name)) def _has_autohotkey() -> bool: @@ -128,7 +135,7 @@ def _model_installed(name: str) -> bool: return False try: result = subprocess.run( - ["flm", "list", "--quiet", "--filter", "installed"], + [resolve_cli("flm"), "list", "--quiet", "--filter", "installed"], capture_output=True, text=True, timeout=15, check=False, ) except Exception: @@ -142,7 +149,7 @@ def _pull_model(name: str) -> bool: return False _step(f"Pulling model {name} (first run may take several minutes)...") try: - result = subprocess.run(["flm", "pull", name], check=False) + result = subprocess.run([resolve_cli("flm"), "pull", name], check=False) except Exception as e: _step(f"flm pull failed: {e}") return False diff --git a/scripts/lib/daemon_client.ahk b/scripts/lib/daemon_client.ahk index b2aab78..823cf98 100644 --- a/scripts/lib/daemon_client.ahk +++ b/scripts/lib/daemon_client.ahk @@ -230,16 +230,169 @@ IsDaemonHealthy_Impl() { } } +; --- pythonw discovery ------------------------------------------------------ +; Dev/source runs launch the Python entrypoints with pythonw, so finding one +; has to work on ANY machine. Nothing below hardcodes an install path: +; 1. GRAMMARFIX_PYTHONW explicit override (escape hatch) +; 2. scripts\.venv ONLY while its base interpreter still exists +; 3. PEP 514 registry how every conformant Windows Python advertises +; itself (python.org, PSF Python Manager, Anaconda) +; 4. pyw.exe / pythonw.exe on PATH, skipping Microsoft Store alias stubs +; +; B57: a venv's Scripts\pythonw.exe is only a ~250 KB stub that re-execs the +; interpreter named in pyvenv.cfg. Uninstalling that interpreter (a 3.13 -> 3.14 +; upgrade does exactly that) leaves the stub on disk, so the old FileExist() +; check still passed and every hotkey popped a modal "Python venv launcher is +; sorry to say ... did not find executable" dialog. Existence != usable. The +; venv rung now validates pyvenv.cfg's base statically -- no spawn, so a dead +; venv can never raise that dialog merely to be detected -- and the old sole +; fallback ("pyw.exe") is no longer assumed: PSF Python Manager ships +; pythonw.exe with no py/pyw launcher at all. +global _pythonwPathCache := "" + ResolvePythonwPath_Impl() { - pythonwPath := EnvGet("GRAMMARFIX_PYTHONW") - if (pythonwPath = "") { - venvPythonw := A_ScriptDir "\\.venv\\Scripts\\pythonw.exe" - if FileExist(venvPythonw) - pythonwPath := venvPythonw + global _pythonwPathCache + ; Revalidate rather than blindly reuse. Flowkey runs for a whole login + ; session, so the interpreter can be upgraded or uninstalled underneath a + ; cached path -- B57 is that exact scenario -- and a cache that never + ; rechecks would pin the dead path until AHK itself is restarted, which is + ; the failure this release exists to remove. Validation is file stats only, + ; never a spawn, so the cache still saves the registry sweep on the hot path + ; while healing itself the moment the interpreter moves. + if (_pythonwPathCache != "" && CachedPythonwUsable_Impl(_pythonwPathCache)) + return _pythonwPathCache + return _pythonwPathCache := DiscoverPythonwPath_Impl() +} + +CachedPythonwUsable_Impl(path) { + ; The bare-name last resort can't be stat-checked, and re-discovering is how + ; a Python installed after we gave up gets picked up at all. + if (path = "" || path = "pyw.exe") + return false + if !UsablePythonwExe_Impl(path) + return false + ; Any venv stub is only good while its base interpreter survives -- whether + ; we chose it or GRAMMARFIX_PYTHONW pointed at it. Walk up from + ; \Scripts\pythonw.exe and re-check the cfg if this is one. + SplitPath(path, , &scriptsDir) + SplitPath(scriptsDir, , &maybeVenvDir) + if (maybeVenvDir != "" && FileExist(maybeVenvDir "\pyvenv.cfg")) + return VenvBaseInterpreterExists_Impl(maybeVenvDir) + return true +} + +DiscoverPythonwPath_Impl() { + override := EnvGet("GRAMMARFIX_PYTHONW") + if (override != "" && UsablePythonwExe_Impl(override)) + return override + + venvDir := A_ScriptDir "\.venv" + venvPythonw := venvDir "\Scripts\pythonw.exe" + if (UsablePythonwExe_Impl(venvPythonw) && VenvBaseInterpreterExists_Impl(venvDir)) + return venvPythonw + + fromRegistry := PythonwFromRegistry_Impl() + if (fromRegistry != "") + return fromRegistry + + for exeName in ["pyw.exe", "pythonw.exe"] { + fromPath := ExeOnPath_Impl(exeName) + if (fromPath != "") + return fromPath } - if (pythonwPath = "") - pythonwPath := "pyw.exe" - return pythonwPath + ; Nothing found. Return the launcher name so the failure surfaces as a + ; normal "can't start" rather than a silent no-op. + return "pyw.exe" +} + +; Exists AND has content. Windows "App Execution Alias" entries under +; WindowsApps are 0-byte reparse stubs that open the Microsoft Store instead of +; running Python, and they sit on PATH ahead of real installs. +UsablePythonwExe_Impl(path) { + if (path = "" || !FileExist(path)) + return false + try return FileGetSize(path) > 0 + catch + return false +} + +; Static health check for a venv: pyvenv.cfg names the base interpreter that +; the Scripts\ stubs re-exec. If that file is gone, the venv is dead weight. +VenvBaseInterpreterExists_Impl(venvDir) { + cfgPath := venvDir "\pyvenv.cfg" + if !FileExist(cfgPath) + return false + try cfg := FileRead(cfgPath) + catch + return false + home := "", executable := "" + Loop Parse cfg, "`n", "`r" { + if RegExMatch(A_LoopField, "i)^\s*executable\s*=\s*(.+?)\s*$", &m) + executable := m[1] + else if RegExMatch(A_LoopField, "i)^\s*home\s*=\s*(.+?)\s*$", &m) + home := m[1] + } + if (executable != "") + return FileExist(executable) != "" + if (home != "") + return FileExist(RTrim(home, "\") "\python.exe") != "" + return false +} + +; PEP 514: conformant installs register +; \SOFTWARE\Python\\\InstallPath +; with (default) = install dir and, where supported, WindowedExecutablePath. +; Highest 3.11+ minor wins; HKCU (per-user) is searched before HKLM so a user +; install shadows a machine one, matching what `python` on PATH would pick. +PythonwFromRegistry_Impl() { + best := "", bestVer := -1 + for root in ["HKCU\SOFTWARE\Python", "HKLM\SOFTWARE\Python", "HKLM\SOFTWARE\WOW6432Node\Python"] { + try { + Loop Reg root, "KR" { + if (A_LoopRegName != "InstallPath") + continue + ; v2 has no A_LoopRegSubKey: A_LoopRegKey is the FULL path of + ; the key being enumerated, so the version tag is its last + ; segment and InstallPath hangs directly off it. + tag := A_LoopRegKey + if (sep := InStr(tag, "\", , -1)) + tag := SubStr(tag, sep + 1) + if !RegExMatch(tag, "(\d+)\.(\d+)", &v) + continue + ver := v[1] * 100 + v[2] + if (ver < 311 || ver <= bestVer) + continue + keyPath := A_LoopRegKey "\" A_LoopRegName + exe := "" + try exe := RegRead(keyPath, "WindowedExecutablePath") + if (exe = "") { + dir := "" + try dir := RegRead(keyPath) ; (default) = install dir + if (dir != "") + exe := RTrim(dir, "\") "\pythonw.exe" + } + if UsablePythonwExe_Impl(exe) { + best := exe + bestVer := ver + } + } + } + } + return best +} + +; Resolve a bare exe name against PATH ourselves: FileExist() doesn't search +; PATH, and this lets UsablePythonwExe_Impl() veto the Store alias stubs. +ExeOnPath_Impl(exeName) { + Loop Parse EnvGet("PATH"), ";" { + dir := Trim(A_LoopField, " `t`"") + if (dir = "") + continue + candidate := RTrim(dir, "\") "\" exeName + if UsablePythonwExe_Impl(candidate) + return candidate + } + return "" } ; --- Entrypoint launching (frozen exe vs dev .py) --------------------------- diff --git a/scripts/subprocess_util.py b/scripts/subprocess_util.py index 9f72215..ed5c6ca 100644 --- a/scripts/subprocess_util.py +++ b/scripts/subprocess_util.py @@ -2,10 +2,104 @@ from __future__ import annotations +import os +import shutil import subprocess NO_WINDOW = getattr(subprocess, "CREATE_NO_WINDOW", 0) +# Where a provider CLI lands when its installer doesn't put it on PATH, or puts +# it there too late for us to see (see _registry_path). Probed in order, after +# PATH; expanded with os.path.expandvars at lookup time so these stay literal. +_CLI_INSTALL_DIRS: dict[str, tuple[str, ...]] = { + "flm": (r"%ProgramFiles%\flm", r"%ProgramFiles%\FastFlowLM"), + "ollama": (r"%LOCALAPPDATA%\Programs\Ollama", r"%ProgramFiles%\Ollama"), +} + + +def _registry_path() -> str: + """Machine + user PATH as the registry holds it *right now*. + + A process inherits the environment block built when its session started, so + an installer that appends to PATH is invisible to everything already + running. Flowkey normally launches at logon, which makes that the common + case rather than the exception: install FastFlowLM afterwards and `flm` is + "not found" until the user signs out, on a machine where it is plainly + installed and on the machine PATH (SPEC.md B58). + + Windows-only; returns "" elsewhere, and on any registry failure — a PATH + lookup must never be the thing that raises. + """ + if os.name != "nt": + return "" + try: + import winreg + except ImportError: + return "" + parts: list[str] = [] + for root, key in ( + (winreg.HKEY_LOCAL_MACHINE, + r"SYSTEM\CurrentControlSet\Control\Session Manager\Environment"), + (winreg.HKEY_CURRENT_USER, "Environment"), + ): + try: + with winreg.OpenKey(root, key) as handle: + value, _ = winreg.QueryValueEx(handle, "Path") + except OSError: + continue # key or value absent on this machine — not an error + expanded = os.path.expandvars(str(value or "")).strip() + if expanded: + parts.append(expanded) + return os.pathsep.join(parts) + + +def search_path() -> str: + """PATH to resolve executables against: what we inherited, plus what the + registry says now. Inherited entries stay first, so a PATH deliberately set + for this process still wins over the machine's.""" + inherited = os.environ.get("PATH", "") + return os.pathsep.join(p for p in (inherited, _registry_path()) if p) + + +def resolve_exe(name: str) -> str: + """Absolute path to an executable, or "" if it genuinely isn't installed. + + PATH first, then PATH as the registry currently holds it, then the known + install directories for that CLI. Never returns a bare name: a caller that + cannot find the program has to be able to tell, rather than handing Windows + a name that resolves to nothing — or to something else. + """ + if not str(name or "").strip(): + return "" + found = shutil.which(name) or shutil.which(name, path=search_path()) + if found: + return found + for raw_dir in _CLI_INSTALL_DIRS.get(name.strip().lower(), ()): + expanded = os.path.expandvars(raw_dir) + if "%" in expanded: + continue # a variable that doesn't exist on this machine + found = shutil.which(name, path=expanded) + if found: + return found + return "" + + +def resolve_cli(name: str) -> str: + """argv[0] for a provider CLI: an absolute path when one can be found, + otherwise the bare name. + + Windows resolves a bare argv[0] against the PARENT process's PATH — the + `env=` mapping handed to subprocess does NOT affect that lookup. Repairing + PATH for the child is therefore not enough on its own: argv[0] itself has + to be absolute, or a CLI that is installed but missing from our stale + session PATH stays unreachable however well the child's environment is + patched up (B58). + + Falls back to the bare name so a genuinely absent CLI still raises + FileNotFoundError at the same place it always did. + """ + return resolve_exe(name) or name + def run_hidden(argv: list[str], **kwargs) -> subprocess.CompletedProcess: kwargs.setdefault("creationflags", NO_WINDOW) diff --git a/tests/test_cli_discovery.py b/tests/test_cli_discovery.py new file mode 100644 index 0000000..c9ab24d --- /dev/null +++ b/tests/test_cli_discovery.py @@ -0,0 +1,147 @@ +"""Finding a provider CLI must not depend on the PATH this process inherited. + +Regression (SPEC.md B58): FastFlowLM was installed, working, and its directory +was on the *machine* PATH — but Flowkey launches at logon, and a process only +ever sees the environment block built when its session started. FLM's installer +appended to PATH after that, so `shutil.which("flm")` found nothing and the app +reported FastFlowLM as not installed on a machine where `flm version` answered +fine from any new shell. `doctor` said `fastflowlm_cli: not found`; every hotkey +failed. Nothing was broken except the lookup. + +Same family as B57 (Python discovery): the bug is trusting an ambient PATH. +""" + +from __future__ import annotations + +import os + +import ffp_flm_server +import ffp_provider_status +import pytest +import subprocess_util + + +def _fake_cli(directory, name: str = "flm"): + """A stand-in executable that shutil.which will accept on this platform.""" + exe = directory / (f"{name}.exe" if os.name == "nt" else name) + exe.write_text("", encoding="utf-8") + exe.chmod(0o755) + return exe + + +def test_resolve_exe_rejects_empty_and_missing(): + assert subprocess_util.resolve_exe("") == "" + assert subprocess_util.resolve_exe(" ") == "" + assert subprocess_util.resolve_exe("ffp-definitely-not-installed-xyz") == "" + + +def test_resolve_exe_finds_a_cli_on_path(tmp_path, monkeypatch): + exe = _fake_cli(tmp_path) + monkeypatch.setenv("PATH", str(tmp_path)) + resolved = subprocess_util.resolve_exe("flm") + assert resolved, "a CLI sitting on PATH was not found" + assert os.path.isabs(resolved), f"resolve_exe returned a bare name: {resolved!r}" + assert os.path.samefile(resolved, exe) + + +def test_resolve_exe_finds_a_cli_absent_from_path(tmp_path, monkeypatch): + """The B58 case: installed, but this process's PATH predates the install.""" + install_dir = tmp_path / "flm" + install_dir.mkdir() + exe = _fake_cli(install_dir) + + monkeypatch.setenv("PATH", str(tmp_path / "somewhere-else")) + monkeypatch.setattr(subprocess_util, "_registry_path", lambda: "") + # Empty the install-dir table first: this machine may have a real FLM, and + # the precondition is about PATH, not about what happens to be installed. + monkeypatch.setitem(subprocess_util._CLI_INSTALL_DIRS, "flm", ()) + assert subprocess_util.resolve_exe("flm") == "", "precondition: not on PATH" + + monkeypatch.setitem(subprocess_util._CLI_INSTALL_DIRS, "flm", (str(install_dir),)) + resolved = subprocess_util.resolve_exe("flm") + assert resolved and os.path.samefile(resolved, exe), ( + "an installed CLI that is not on the inherited PATH stayed invisible" + ) + + +def test_resolve_exe_consults_the_registry_path(tmp_path, monkeypatch): + exe = _fake_cli(tmp_path) + monkeypatch.setenv("PATH", str(tmp_path / "nothing-here")) + monkeypatch.setattr(subprocess_util, "_registry_path", lambda: str(tmp_path)) + resolved = subprocess_util.resolve_exe("flm") + assert resolved and os.path.samefile(resolved, exe) + + +def test_search_path_keeps_inherited_entries_first(tmp_path, monkeypatch): + """An explicitly-set PATH must still win over the machine's.""" + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setattr(subprocess_util, "_registry_path", lambda: r"C:\machine\only") + parts = subprocess_util.search_path().split(os.pathsep) + assert parts[0] == str(tmp_path) + assert r"C:\machine\only" in parts + + +def test_registry_path_never_raises(): + # Called on every flm spawn; a lookup must not be what takes the app down. + assert isinstance(subprocess_util._registry_path(), str) + + +def test_flm_env_repairs_path(tmp_path, monkeypatch): + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setattr(subprocess_util, "_registry_path", lambda: r"C:\machine\only") + env = ffp_flm_server.flm_env() + assert r"C:\machine\only" in env["PATH"].split(os.pathsep), ( + "flm children still inherit the stale session PATH" + ) + + +def test_flm_env_repairs_path_even_without_a_model_path(tmp_path, monkeypatch): + """flm_env() used to return early when FLM_MODEL_PATH was unset — which is + the common case, and exactly when the PATH repair is still needed.""" + monkeypatch.delenv("FLM_MODEL_PATH", raising=False) + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setattr(subprocess_util, "_registry_path", lambda: r"C:\machine\only") + env = ffp_flm_server.flm_env() + assert r"C:\machine\only" in env["PATH"].split(os.pathsep) + + +def test_flm_env_still_repairs_a_systemprofile_model_path(monkeypatch): + """B54 must survive the B58 change.""" + monkeypatch.setenv("FLM_MODEL_PATH", r"C:\Windows\system32\config\systemprofile\.flm") + env = ffp_flm_server.flm_env() + assert "config\\systemprofile" not in env["FLM_MODEL_PATH"].lower() + assert env["FLM_MODEL_PATH"].endswith(".flm") + + +@pytest.mark.parametrize("provider", ["fastflowlm", "ollama"]) +def test_provider_status_uses_resolve_exe(provider, monkeypatch): + """Detection has to agree with what flm_env() will actually run, or doctor + and the wizard contradict the app.""" + seen: list[str] = [] + + def fake_resolve(name: str) -> str: + seen.append(name) + return rf"C:\fake\{name}.exe" + + monkeypatch.setattr(ffp_provider_status, "resolve_exe", fake_resolve) + status = ffp_provider_status.provider_status(provider) + assert seen == [ffp_provider_status.PROVIDERS[provider].cli] + assert status["installed"] is True + assert status["cli_path"].endswith(".exe") + + +def test_install_guards_use_the_shared_resolver(monkeypatch): + """install.py's _has_cmd gated every FLM path before resolve_cli could run. + + With shutil.which behind the guard, `ffp-install` on a stale PATH reported + FLM missing and postreboot() opened its download page for software that was + already installed — detection disagreeing with execution, which is exactly + what V69 forbids. + """ + import install + + monkeypatch.setattr(install, "resolve_exe", lambda name: rf"C: ound\{name}.exe") + assert install._has_cmd("flm") is True + + monkeypatch.setattr(install, "resolve_exe", lambda _name: "") + assert install._has_cmd("flm") is False diff --git a/tests/test_ffp_provider_runtime.py b/tests/test_ffp_provider_runtime.py index 456424e..95e1e29 100644 --- a/tests/test_ffp_provider_runtime.py +++ b/tests/test_ffp_provider_runtime.py @@ -96,6 +96,9 @@ def fake_run(argv, **_kwargs): return SimpleNamespace(returncode=0, stdout="", stderr="") monkeypatch.setattr(ffp_provider_runtime, "run_hidden", fake_run) + # argv[0] is now resolved to an absolute path (B58); pin it to the bare + # name so this test asserts the CLI contract, not what is installed here. + monkeypatch.setattr(ffp_provider_runtime, "resolve_cli", lambda name: name) assert ffp_provider_runtime.pull_model("ollama", "llama3.2:3b", 0) == "pulled llama3.2:3b" assert ffp_provider_runtime.remove_model("ollama", "llama3.2:3b", 0) == "removed llama3.2:3b" diff --git a/tests/test_ffp_provider_status.py b/tests/test_ffp_provider_status.py index aeeb657..44e298b 100644 --- a/tests/test_ffp_provider_status.py +++ b/tests/test_ffp_provider_status.py @@ -4,7 +4,7 @@ def test_provider_status_reports_cli_and_reachability(monkeypatch): - monkeypatch.setattr(ffp_provider_status.shutil, "which", lambda name: f"C:/bin/{name}.exe") + monkeypatch.setattr(ffp_provider_status, "resolve_exe", lambda name: f"C:/bin/{name}.exe") monkeypatch.setattr(ffp_provider_status, "is_reachable", lambda base_url: base_url.endswith(":11434")) status = ffp_provider_status.provider_status("ollama", base_url="http://127.0.0.1:11434") @@ -19,9 +19,9 @@ def test_provider_status_reports_cli_and_reachability(monkeypatch): def test_providers_status_allows_ollama_and_flm_on_same_pc(monkeypatch): monkeypatch.setattr( - ffp_provider_status.shutil, - "which", - lambda name: f"C:/bin/{name}.exe" if name in {"ollama", "flm"} else None, + ffp_provider_status, + "resolve_exe", + lambda name: f"C:/bin/{name}.exe" if name in {"ollama", "flm"} else "", ) monkeypatch.setattr(ffp_provider_status, "is_reachable", lambda _base_url: True) @@ -34,7 +34,7 @@ def test_providers_status_allows_ollama_and_flm_on_same_pc(monkeypatch): def test_providers_status_prefers_configured_active_even_when_missing(monkeypatch): - monkeypatch.setattr(ffp_provider_status.shutil, "which", lambda _name: None) + monkeypatch.setattr(ffp_provider_status, "resolve_exe", lambda _name: "") monkeypatch.setattr(ffp_provider_status, "is_reachable", lambda _base_url: False) status = ffp_provider_status.providers_status("fastflowlm", "http://127.0.0.1:52625") diff --git a/tests/test_ffp_pull.py b/tests/test_ffp_pull.py index 2b514ef..2400fec 100644 --- a/tests/test_ffp_pull.py +++ b/tests/test_ffp_pull.py @@ -82,6 +82,9 @@ def wait(self): ffp_pull.subprocess, "Popen", lambda argv, **kw: popened.append(argv) or _Proc(), ) + # argv[0] is now resolved to an absolute path (B58); pin it to the bare + # name so this test asserts the CLI contract, not what is installed here. + monkeypatch.setattr(ffp_pull, "resolve_cli", lambda name: name) rc = ffp_pull._default_runner("ollama", "llama3.2:3b", 0, lambda _l: None, force=True) @@ -109,6 +112,9 @@ def wait(self): ffp_pull.subprocess, "Popen", lambda argv, **kw: calls.append(("popen", argv)) or _Proc(), ) + # argv[0] is now resolved to an absolute path (B58); pin it to the bare + # name so this test asserts the CLI contract, not what is installed here. + monkeypatch.setattr(ffp_pull, "resolve_cli", lambda name: name) rc = ffp_pull._default_runner("fastflowlm", "qwen3.5:9b", 0, lambda _l: None, force=True) diff --git a/tests/test_pythonw_discovery.ahk b/tests/test_pythonw_discovery.ahk new file mode 100644 index 0000000..e3a434f --- /dev/null +++ b/tests/test_pythonw_discovery.ahk @@ -0,0 +1,125 @@ +#Requires AutoHotkey v2.0 +; Regression tests for portable pythonw discovery (B57). +; +; The bug: scripts\.venv\Scripts\pythonw.exe is a stub that re-execs the +; interpreter named in pyvenv.cfg. A 3.13 -> 3.14 upgrade uninstalls that +; interpreter but leaves the stub, so a bare FileExist() check still passed and +; every hotkey popped a modal "Python venv launcher is sorry to say ..." dialog. +; These tests pin the two checks that make discovery machine-independent: +; existence != usable, and a venv is only usable while its base still exists. +; +; AutoHotkey64.exe /ErrorStdOut test_pythonw_discovery.ahk + +; daemon_client.ahk reads these from its host script (grammarFix.ahk). This +; test only exercises the pure discovery helpers, but the globals have to +; exist or AHK's load-time #Warn fires while parsing the include. +global scriptPath := "" +global daemonScriptPath := "" +global daemonBaseUrl := "http://127.0.0.1:52650" + +; ShutdownFlowkeyChildren_Impl() calls the host script's RunAction() +; wrapper. Nothing here invokes it, but it has to exist at load time. +RunAction(action, body := "{}") => "" + +#Include "..\scripts\lib\daemon_client.ahk" + +failures := 0 +total := 0 + +Check(name, actual, expected) { + global failures, total + total++ + if (actual != expected) { + failures++ + FileAppend(Format("FAIL [{}]: got {} want {}`n", name, actual, expected), "**") + } +} + +sandbox := A_Temp "\ffp_pythonw_discovery_" A_TickCount +DirCreate(sandbox) + +; --- UsablePythonwExe_Impl --------------------------------------------------- +emptyFile := sandbox "\alias_stub.exe" ; 0 bytes, like a WindowsApps alias +FileAppend("", emptyFile) +realFile := sandbox "\real.exe" +FileAppend("MZ not really an exe, just non-empty", realFile) + +Check("empty path", UsablePythonwExe_Impl(""), false) +Check("missing file", UsablePythonwExe_Impl(sandbox "\nope.exe"), false) +; A Microsoft Store "App Execution Alias" is a 0-byte reparse point that opens +; the Store instead of running Python, and it sits on PATH ahead of real installs. +Check("zero-byte store alias", UsablePythonwExe_Impl(emptyFile), false) +Check("real file", UsablePythonwExe_Impl(realFile), true) + +; --- VenvBaseInterpreterExists_Impl ----------------------------------------- +MakeVenv(name, cfgText) { + global sandbox + dir := sandbox "\" name + DirCreate(dir "\Scripts") + ; Non-empty on purpose: a 0-byte stub is rejected as an alias stub before + ; the pyvenv.cfg check runs, which would let the dead-venv cases below pass + ; for entirely the wrong reason. + FileAppend("venv launcher stub", dir "\Scripts\pythonw.exe") + if (cfgText != "") + FileAppend(cfgText, dir "\pyvenv.cfg") + return dir +} + +baseDir := sandbox "\base313" +DirCreate(baseDir) +FileAppend("interpreter", baseDir "\python.exe") + +aliveExec := MakeVenv("alive_exec", + "home = " baseDir "`nversion = 3.13.7`nexecutable = " baseDir "\python.exe`n") +Check("venv with live executable=", VenvBaseInterpreterExists_Impl(aliveExec), true) + +; The B57 case verbatim: the stub is still on disk, pyvenv.cfg still names its +; base, but the base interpreter was uninstalled out from under it. +deadExec := MakeVenv("dead_exec", + "home = " sandbox "\gone`nversion = 3.13.7`nexecutable = " sandbox "\gone\python.exe`n") +Check("venv with uninstalled base", VenvBaseInterpreterExists_Impl(deadExec), false) + +; Older venvs omit executable= and only carry home=. +aliveHome := MakeVenv("alive_home", "home = " baseDir "`nversion = 3.13.7`n") +Check("venv with live home= only", VenvBaseInterpreterExists_Impl(aliveHome), true) + +deadHome := MakeVenv("dead_home", "home = " sandbox "\gone`nversion = 3.13.7`n") +Check("venv with dead home= only", VenvBaseInterpreterExists_Impl(deadHome), false) + +Check("venv with no pyvenv.cfg", VenvBaseInterpreterExists_Impl(MakeVenv("no_cfg", "")), false) +Check("venv dir absent", VenvBaseInterpreterExists_Impl(sandbox "\not_a_venv"), false) + +; --- Cached path revalidation ----------------------------------------------- +; Flowkey runs for a whole login session, so a cached interpreter can be +; upgraded or uninstalled underneath it. A cache that never rechecks pins the +; dead path until AHK restarts -- reintroducing the very failure B57 is about. +Check("cached empty", CachedPythonwUsable_Impl(""), false) +; The bare-name last resort must always re-discover: a Python installed after +; we gave up is only picked up if we look again. +Check("cached bare pyw.exe", CachedPythonwUsable_Impl("pyw.exe"), false) +Check("cached path now missing", CachedPythonwUsable_Impl(sandbox "\gone\pythonw.exe"), false) +Check("cached plain interpreter", CachedPythonwUsable_Impl(realFile), true) +Check("cached live venv stub", CachedPythonwUsable_Impl(aliveExec "\Scripts\pythonw.exe"), true) +; The regression: the venv was fine when we cached it, then its base went away. +Check("cached venv whose base vanished", CachedPythonwUsable_Impl(deadExec "\Scripts\pythonw.exe"), false) + +; --- Discovery on THIS machine ---------------------------------------------- +; The portability contract: wherever a conformant Python 3.11+ is installed, +; discovery must hand back a real file, never the bare "pyw.exe" guess. PSF +; Python Manager ships no py/pyw launcher at all, so that guess alone is not a +; fallback anyone can rely on. +fromRegistry := PythonwFromRegistry_Impl() +if (fromRegistry != "") { + Check("registry hit is usable", UsablePythonwExe_Impl(fromRegistry), true) + Check("discovery returns a real file", UsablePythonwExe_Impl(DiscoverPythonwPath_Impl()), true) +} + +DirDelete(sandbox, true) + +if (failures > 0) { + FileAppend(Format("test_pythonw_discovery: {}/{} FAILED`n", failures, total), "**") + ExitApp(1) +} +; Success: exit 0 only -- FileAppend("*") needs a console and errors when run +; from Explorer/IDE. +ExitApp(0) diff --git a/tests/test_pythonw_discovery.py b/tests/test_pythonw_discovery.py new file mode 100644 index 0000000..02c76a8 --- /dev/null +++ b/tests/test_pythonw_discovery.py @@ -0,0 +1,106 @@ +"""Drift guard: finding a Python interpreter must never assume an install +layout, and "the file is there" must never be mistaken for "the file works". + +Regression (SPEC.md B57): a 3.13 -> 3.14 upgrade uninstalled the venv's base +interpreter. ``scripts\\.venv\\Scripts\\pythonw.exe`` survived -- it is only a +~250 KB stub that re-execs the path recorded in ``pyvenv.cfg`` -- so the AHK +resolver's bare ``FileExist()`` check still passed and every hotkey spawned a +dead launcher that hung on a modal "Python venv launcher is sorry to say ..." +dialog. The rung below it was the bare name ``pyw.exe``, which PSF Python +Manager does not ship at all, so deleting the venv would only have moved the +failure. ``install.ps1`` had the same existence-is-health blind spot, so +re-running the installer reported "venv already present" and repaired nothing. + +The behavioural assertions live in ``tests/test_pythonw_discovery.ahk`` (run by +CI's AHK job, which can actually execute the resolver). These are the cheap +structural guards that keep the two implementations from drifting apart. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +AHK = (ROOT / "scripts" / "lib" / "daemon_client.ahk").read_text(encoding="utf-8") +PS1 = (ROOT / "installer" / "install.ps1").read_text(encoding="utf-8") + + +def test_ahk_venv_rung_validates_the_base_interpreter(): + """Accepting the venv on file existence alone is the B57 bug itself.""" + m = re.search( + r"venvPythonw\s*:=.*?\n(.*?)\n\s*return venvPythonw", AHK, re.DOTALL + ) + assert m, "daemon_client.ahk no longer has a recognisable venv rung" + guard = m.group(1) + assert "VenvBaseInterpreterExists_Impl" in guard, ( + "the venv rung accepts scripts\\.venv without checking that pyvenv.cfg's " + "base interpreter still exists — a stale stub will be spawned again" + ) + + +def test_ahk_venv_health_check_is_static(): + """Detecting a dead venv must not run it: that is what pops the dialog.""" + m = re.search( + r"VenvBaseInterpreterExists_Impl\(venvDir\)\s*\{(.*?)\n\}", AHK, re.DOTALL + ) + assert m, "VenvBaseInterpreterExists_Impl is gone" + body = m.group(1) + assert "pyvenv.cfg" in body, "the health check no longer reads pyvenv.cfg" + for spawner in ("Run(", "RunWait(", "ComObject(\"WScript.Shell\")"): + assert spawner not in body, ( + f"the venv health check spawns a process ({spawner}) — a dead venv " + "would raise its own modal dialog just to be detected" + ) + + +def test_ahk_discovery_does_not_rely_on_the_py_launcher_alone(): + """PSF Python Manager installs pythonw.exe and no py/pyw launcher at all.""" + assert "PythonwFromRegistry_Impl" in AHK, ( + "no PEP 514 registry rung — discovery is back to guessing launcher names" + ) + assert "SOFTWARE\\Python" in AHK, "the registry rung stopped reading PEP 514 keys" + assert 'ExeOnPath_Impl("pythonw.exe")' in AHK or '"pythonw.exe"' in AHK, ( + "pythonw.exe is no longer probed on PATH" + ) + + +def test_both_implementations_veto_zero_byte_alias_stubs(): + """WindowsApps App Execution Aliases are 0-byte reparse points on PATH.""" + m = re.search(r"UsablePythonwExe_Impl\(path\)\s*\{(.*?)\n\}", AHK, re.DOTALL) + assert m, "UsablePythonwExe_Impl is gone" + assert "FileGetSize" in m.group(1), ( + "AHK no longer rejects 0-byte files, so a Microsoft Store alias stub can " + "be picked as the interpreter" + ) + assert re.search(r"function Test-RealExe.*?\.Length -gt 0", PS1, re.DOTALL), ( + "install.ps1 no longer rejects 0-byte files (Store alias stubs)" + ) + + +def test_install_ps1_gates_the_venv_on_health_not_presence(): + m = re.search(r'Info "Step 2/6.*?\n(.*?)\n# ---- 3\.', PS1, re.DOTALL) + assert m, "install.ps1 step 2 (venv) is no longer recognisable" + step2 = m.group(1) + assert "Test-VenvHealthy" in step2, ( + "step 2 gates on presence again — a venv whose base interpreter was " + "uninstalled will be reported as 'already present' and never repaired" + ) + assert re.search(r"if \(Test-Path \$venvDir\)[\s\S]{0,200}Remove-Item", step2), ( + "step 2 no longer rebuilds an unhealthy venv" + ) + assert "& $pythonExe -m venv" in step2, ( + "the venv is created with a bare `python` again rather than the " + "interpreter discovery actually resolved" + ) + + +def test_install_ps1_resolves_python_without_assuming_path(): + assert "function Resolve-PythonExe" in PS1, "install.ps1 lost its Python discovery" + assert "Get-RegistryPythonExes" in PS1, ( + "install.ps1 no longer falls back to the PEP 514 registry, so a Python " + "that isn't on PATH triggers a redundant winget install" + ) + assert "Test-PythonOk" not in PS1, ( + "the old PATH-only Test-PythonOk is back" + ) diff --git a/tests/test_version_sync.py b/tests/test_version_sync.py index 6325293..d306130 100644 --- a/tests/test_version_sync.py +++ b/tests/test_version_sync.py @@ -32,4 +32,4 @@ def test_v18_release_version_is_synchronized(): ), } - assert set(versions.values()) == {"2.5.2"}, versions + assert set(versions.values()) == {"2.5.3"}, versions