Fix/windows pip mode - #94
Merged
Merged
Conversation
… not see A Ctrl+C in pip mode on Windows left 2 of 23 processes running -- both Streamlit, holding port 8501. Two separate reasons, and fixing either alone changes nothing. The sweep could not see them. On Windows it matched the image name `testgen.exe`, but every child is `python`. On POSIX it matched `testgen.*run-app`, but `run_ui` spawns the UI as `python -m streamlit run .../testgen/ui/app.py`, which carries no `run-app` -- and `start_new_session` puts it outside the group `killpg` targets. Match on the uv tool environment path instead: it appears in argv[0] of every child and in postgres' own command line, so it identifies the install without depending on what any process is called. Separators are normalised, since postgres writes its command line with forward slashes and the python children with back. The Windows clause is generated from the same patterns so the platforms cannot drift. The sweep was also never reached. `stop_app_tree` returned early on `proc.poll() is not None`, and on Windows the parent is usually gone by then: a console Ctrl+C reaches every process in the console's group, and the app is spawned without CREATE_NEW_PROCESS_GROUP (`start_new_session` is POSIX-only), so it dies with us. The UI does not -- `run_ui` gives it a group of its own. Run the force-kill path on Windows regardless of whether the parent is alive; a dead parent there says nothing about its children. POSIX keeps the early return, since there the parent forwards the signal before exiting. Windows enumerates and kills each match by PID rather than `taskkill /T`, which walks the tree as it stands -- killing a parent re-parents its children mid-walk and they escape. A failed sweep now logs: PowerShell can be absent or blocked by policy, and silence there left no trace of why the next start found the port taken. Verified on both platforms with real process trees. Linux, via a container: 16 orphans after a SIGKILLed installer, 0 after. Windows, on a real console: 2 survivors before, 0 after. Test fixtures use the command lines captured from both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On Windows `tg delete` printed "TestGen uninstalled." while leaving the uv tool
environment and the `testgen` entry point on disk. A leaked UI process held
`<tool env>/Scripts/python.exe` open, `uv tool uninstall` could not remove it, and
the resulting error was caught and softened to "it may already be uninstalled" --
which is exactly backwards. It had not been uninstalled at all.
Check the disk instead of trusting the exit code, and name what survived:
TestGen was only partly uninstalled. These were left behind:
~/.local/share/uv/tools/dataops-testgen
~/.local/bin/testgen
This usually means a TestGen process was still running and held a file open.
Close any running TestGen, then run this command again.
That retry has to be able to do something, so an incomplete uninstall now keeps the
install marker -- dropping it makes the next `tg delete` report "nothing to delete"
-- and keeps the installer-local uv, which the retry needs to uninstall with.
The leftover check runs before the rest of the cleanup, so it tolerates any failure
rather than only `CommandFailed`: uv can fail to spawn outright, and aborting there
left more behind than doing nothing would. The pre-existing uninstall call is
widened for the same reason.
uv's bin directory is shared, commonly ~/.local/bin, so a `testgen` shim there is
not necessarily ours. It is only reported alongside a surviving tool environment;
on its own it would call a clean uninstall a failure and then strand the user
behind the marker we now keep.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A pip install on Windows failed inside `standalone-setup` with:
initdb: error: program "postgres" is needed by initdb but was not found in
the same directory as ".../initdb.exe"
postgres.exe was present the whole time. initdb runs `postgres -V`, that launch
failed, and initdb reported the failed launch as a missing file -- naming the one
thing that was not wrong. (The cause was a pgserver build shipped without a library
it links against, but the class is wider than that one bug.)
Run `postgres -V` ourselves first. If the bundled binary will not run, the install
cannot succeed, so stop and point at `--docker`. The exit code goes to the session
log, where support can use it; the console gets the way forward instead, since
neither an NT status code nor a library name helps the person installing. Under
`upgrade` it says to delete first, because `tg install` refuses while an install
marker exists and would otherwise walk the user into a refusal.
Checking before `standalone-setup` matters for a second reason: a partial setup
leaves `config.env` behind, and every retry then stops on an interactive
"Overwrite?" prompt the installer has no terminal to answer. One failed install
otherwise wedges every attempt after it.
The check runs after the tool environment is built -- it has to, since the binary
ships inside it -- so it saves seconds, not minutes. Its value is a legible failure
and a clean retry, not speed. Guidance is emitted from `on_action_fail` rather than
`execute`, which runs inside a partial console line. Skips rather than fails when
the binary cannot be located: not finding it says nothing about whether it works.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aarthy-dk
approved these changes
Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Testing pip (standalone) mode on a clean Windows box turned up three defects. They compound: the first leaves a process behind, the second then reports a failed uninstall as a success, and the third makes an unrelated failure unreadable.
Ctrl+C left the UI running, and it broke everything after it. A stop left 2 of 23 processes alive — both Streamlit, still holding port 8501 — after printing TestGen stopped. That one orphan then made the next tg start bind nothing (while wait_for_tcp_port connected to the orphan and reported success), and made tg delete fail, since it held /Scripts/python.exe open.
Two independent causes, and fixing either alone changes nothing — the sweep could not see them (image-name match on testgen.exe, but every child is python; the POSIX pattern missed Streamlit for the same reason), and the sweep was never reached (stop_app_tree returned early on a dead parent, which on Windows is the normal case since a console Ctrl+C kills it along with us).
tg delete reported a success it had not verified — printing TestGen uninstalled. while leaving the tool environment behind, with the underlying error softened to "it may already be uninstalled". It now checks the disk and keeps the install marker, so the retry it recommends can actually do something.
A failed embedded database was unreadable — initdb misreports a failed launch as a missing file. We now run postgres -V first and point at --docker.
Then a Verified table (Linux 16 → 0, Windows 2 → 0), and a Not covered section noting Docker mode is untouched and that the database failure's actual cause lives outside this repo.