Skip to content

fix(gateway): add Windows support for gateway process lifecycle#193

Open
Escapingbug wants to merge 1 commit intoHKUDS:mainfrom
Escapingbug:fix/gateway-windows-support
Open

fix(gateway): add Windows support for gateway process lifecycle#193
Escapingbug wants to merge 1 commit intoHKUDS:mainfrom
Escapingbug:fix/gateway-windows-support

Conversation

@Escapingbug
Copy link
Copy Markdown
Contributor

The ohmo gateway process management (start, find, stop) only worked on Unix-like systems. On Windows, all three operations fail:

  • start_gateway_process: uses start_new_session=True (no equivalent on Windows; also lacks stdin=DEVNULL which causes the subprocess to inherit the parent's console input)
  • _pid_is_running: uses os.kill(pid, 0) which raises PermissionError for processes the caller doesn't own on Windows
  • _iter_workspace_gateway_pids: shells out to ps, which doesn't exist on Windows
  • stop_gateway_process: uses os.kill(pid, SIGTERM), which is not supported on Windows

Replace each with a Windows-compatible path guarded by sys.platform:

  • Start: CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, stdin=DEVNULL
  • PID check: OpenProcess/GetExitCodeProcess via ctypes (STILL_ACTIVE)
  • PID listing: wmic process where commandline like ... get processid
  • Stop: taskkill /F /T /PID

The Unix paths remain unchanged.

Summary

  • What problem does this PR solve?
    The ohmo gateway process lifecycle management (start_gateway_process, _pid_is_running, _iter_workspace_gateway_pids, stop_gateway_process) only works on Unix-like systems. On Windows, every operation fails:

    • start_gateway_process uses start_new_session=True which has no Windows equivalent, and the subprocess inherits the parent's console input (no stdin=DEVNULL)
    • _pid_is_running uses os.kill(pid, 0) which raises PermissionError on Windows for processes the caller doesn't own
    • _iter_workspace_gateway_pids shells out to ps, which doesn't exist on Windows
    • stop_gateway_process uses os.kill(pid, SIGTERM), which is not supported on Windows
  • What changed?
    Added Windows-compatible paths for each operation, guarded by sys.platform == "win32":

    • Start: CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS + stdin=DEVNULL
    • PID check: OpenProcess/GetExitCodeProcess via ctypes (checks STILL_ACTIVE = 259)
    • PID listing: wmic process where commandline like ... get processid
    • Stop: taskkill /F /T /PID

    The Unix paths remain completely unchanged.

Validation

  • uv run ruff check src tests scripts
  • uv run pytest -q (pre-existing failures on Windows unrelated to this change; ohmo/ directory has no test suite)
  • cd frontend/terminal && npx tsc --noEmit (frontend not touched)

Notes

  • Related issue: N/A
  • Follow-up work: N/A

The ohmo gateway process management (start, find, stop) only worked on
Unix-like systems. On Windows, all three operations fail:

- start_gateway_process: uses start_new_session=True (no equivalent on
  Windows; also lacks stdin=DEVNULL which causes the subprocess to
  inherit the parent's console input)
- _pid_is_running: uses os.kill(pid, 0) which raises PermissionError
  for processes the caller doesn't own on Windows
- _iter_workspace_gateway_pids: shells out to `ps`, which doesn't exist
  on Windows
- stop_gateway_process: uses os.kill(pid, SIGTERM), which is not
  supported on Windows

Replace each with a Windows-compatible path guarded by sys.platform:

- Start: CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS, stdin=DEVNULL
- PID check: OpenProcess/GetExitCodeProcess via ctypes (STILL_ACTIVE)
- PID listing: wmic process where commandline like ... get processid
- Stop: taskkill /F /T /PID

The Unix paths remain unchanged.
Copy link
Copy Markdown
Collaborator

@tjb-tech tjb-tech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not merge this as-is. The main blocker is the new Windows PID discovery path depending on wmic:

  • Microsoft documents wmic as deprecated, and their current Windows client docs say it will be removed in a future Windows release. That makes it a poor new hard dependency for gateway lifecycle management.
  • Even before removal, wmic process where commandline like ... is brittle for paths with quoting / escaping differences, especially around --workspace values on Windows.

The Unix-side intent here makes sense, but I think the Windows implementation should switch to a non-wmic process query path before merge. For example: PowerShell/CIM (Get-CimInstance Win32_Process) or another API-level enumeration path, plus tests around PID discovery / stop behavior.

@Escapingbug
Copy link
Copy Markdown
Contributor Author

BTW, what is our attitude towards third-party dependencies? Maybe using psutil is a better option considering cross-platform compatibility?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants