Followup x2: Fixup tests, add ruff lint fixes

[a-johnston]

Followup for #453 with mostly test changes as well as some ruff lint fixes.

Not included here but I wanted to mention: I had some unrelated issues with the plugin attempting to reuse an existing python process and failing and I don't really understand why some of the handling around that exists. First it seemed like there was a bug with the "proof" mechanism with a relatively simple fix:

--- a/plugin/addons/godot_ai/plugin.gd
+++ b/plugin/addons/godot_ai/plugin.gd
@@ -1103,9 +1103,11 @@ func _legacy_pidfile_kill_targets(_port: int, listener_pids: Array[int]) -> Arra
        var pidfile_branded := _pid_cmdline_is_godot_ai_for_proof(pidfile_pid)
        if pidfile_pid <= 1 or pidfile_pid == OS.get_process_id():
                return targets
-       if not listener_pids.has(pidfile_pid) or not pidfile_alive:
-               return targets
-       if not pidfile_branded:
+       ## Do not require the pid-file PID to itself be a port listener:
+       ## with uvicorn --reload the launcher writes the pid-file but the
+       ## worker binds the port, so listener_pids never contains the launcher.
+       ## Alive + branded is sufficient proof this is our server.
+       if not pidfile_alive or not pidfile_branded:
                return targets

But then the next time I experienced a crash/time out/orphaned pid and had an opportunity to test it, I realized the dev mode bypassed that and treated the existing pid as compatible even if it was not and just left the plugin in a broken state. Why not clean up the python process more aggressively? Is there a benefit to allowing the python process to stay in a mismatched state from the gdscript? Initially I was only even looking at it because the thin "proof" wrappers were just kinda bizarre to me.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[dsarno]

Thanks for the nudge, you were definitely right that the dev mode was pointlessly letting "externally spawned" severs stay alive for now good reason. "User mode" is smarter, it just kills any mismatched server, so it was good to bring dev mode in line and now it does same. So both things you flagged were good issues we fixed in #460.

For the pidfile: you were right, the listener_pids check breaks for the --reload shape — the reloader writes the pidfile but its worker binds the port, so the logic just bails. Your diff is basically what we used. One addition: we also kill the reloader PID, otherwise the parent just respawns a new worker the moment we kill the old one.

And the proof wrappers are just there so unit tests can fake the OS calls.