Skip to content

fix(windows): make worker pipe typing portable - #1682

Open
Doar999 wants to merge 1 commit into
oceanbase:masterfrom
Doar999:codex/fix-windows-worker-pipe-typing
Open

Doar999 wants to merge 1 commit into
oceanbase:masterfrom
Doar999:codex/fix-windows-worker-pipe-typing

Conversation

@Doar999

@Doar999 Doar999 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Closes #1658.

Rationale for this change

Windows spawn pipes are represented as PipeConnection, while the worker lifecycle annotated them as POSIX Connection objects. The lifecycle tests also depended on POSIX-only process APIs and spawn internals, causing seven Windows-target type-checking errors and preventing the cleanup behavior from being exercised on Windows.

Related work and acknowledgement

Thanks to @knqiufan for sharing the overlapping draft implementation in #1676 and closing it in favor of the existing community contribution.

I reviewed that draft as an optional reference. This PR uses capability-based pipe protocols and platform-specific spawn seams so the existing cleanup tests can run on Windows instead of being skipped. It also incorporates the focused Windows CI structure and retains coverage for cancellation after a child has been published.

What changes are included in this PR?

  • Model the worker pipe endpoints with separate sender and receiver protocols exposing only the operations each endpoint uses.
  • Select the appropriate spawn interception seam on each platform: Popen._launch on POSIX and Popen.__init__ on Windows.
  • Add platform-specific child exit and reaping assertions.
  • Treat only ERROR_INVALID_PARAMETER as evidence that a Windows PID no longer exists; propagate other OpenProcess failures.
  • Run the existing worker startup, timeout, cancellation, pipe cleanup, and pre-publication failure tests on Windows.
  • Add regression coverage for cancellation after the child process has been published but before the start operation returns.
  • Add focused Windows-target type checking and worker lifecycle testing to CI.

Are there any user-facing changes?

No public API, schema, dependency, configuration, or persisted-format changes.

The change improves Windows type safety and validates worker cleanup behavior without changing the worker lifecycle contract.

How was this change tested?

  • Windows-target type checking:
    uv run --locked --no-sync ty check --python-platform win32 src/powercontext/builtin/runtime/artifact_processing.py tests/builtin/runtime/test_artifact_processing.py
  • Linux-target type checking:
    uv run --locked --no-sync ty check --python-platform linux src/powercontext/builtin/runtime/artifact_processing.py tests/builtin/runtime/test_artifact_processing.py
  • Actual Windows worker lifecycle suite: 7 passed
  • Ruff lint and format checks: passed
  • Workflow YAML validation: passed
  • GitHub Actions reference validation: 79 references checked

The repository-wide Windows type check also reports three pre-existing url2pathname deprecation warnings that #1658 explicitly identifies as out of scope. The focused Windows check added by this PR passes without ignores or warning suppression.

AI usage statement

OpenAI Codex (GPT-5) was used to inspect the issue and review feedback, implement the fixes, design regression tests, and review the final change.

Model spawn pipe endpoints by the operations workers use, isolate POSIX-only lifecycle assertions, and add Windows type-check and worker cleanup coverage.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(windows): resolve worker pipe typing and POSIX-only test assumptions

2 participants