Skip to content

fix(runtime): probe Windows Effect process without signals - #3423

Merged
huangruiteng merged 1 commit into
mainfrom
codex/fix-windows-effect-runtime-lifecycle
Aug 21, 2026
Merged

fix(runtime): probe Windows Effect process without signals#3423
huangruiteng merged 1 commit into
mainfrom
codex/fix-windows-effect-runtime-lifecycle

Conversation

@huangruiteng

@huangruiteng huangruiteng commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace the POSIX-only kill(pid, 0) liveness probe on Windows with a read-only process-handle wait
  • prevent the managed Node runtime from inheriting caller and installer pipe handles
  • preserve the existing POSIX probe and the managed runtime RPC/startup/shutdown contracts
  • add regression contracts for non-signaling liveness and closed descriptor inheritance

Root cause

PR #3416 added a shared managed TypeScript Effect runtime and read its persisted PID before every request. On POSIX, os.kill(pid, 0) is a side-effect-free existence probe. On Windows, Python documents os.kill as a signal/termination API; signal value 0 maps to the console Ctrl-C event rather than POSIX probe semantics. The metadata read therefore interrupted the test runner while it was blocked in socket.recv, producing 19 passing tests followed by KeyboardInterrupt.

The Windows branch now opens the process with SYNCHRONIZE, checks it using WaitForSingleObject(..., 0), and closes the handle. WAIT_TIMEOUT means the process remains live. Access-denied processes are treated as live because existence was established but query authority was unavailable.

The first native rerun then exposed the adjacent lifecycle bug: 37 tests passed, but test_windows_installer_promotes_release_and_runs_doctor timed out after 180 seconds. The installed doctor --deep process started the runtime with close_fds=False, so the long-lived Node child inherited the installer's captured stdout/stderr pipe handles and subprocess.communicate() could not observe EOF. Runtime launch now always closes unrelated descriptors; its own standard streams remain explicitly connected to DEVNULL.

Validation

  • exact Windows workflow selection on macOS: 35 passed, 6 skipped
  • Effect runtime integration: 11 passed
  • Ruff on changed paths: passed
  • repository MyPy contract: passed
  • npm run typecheck:control-plane: passed
  • npm run test:control-plane: 18 passed
  • git diff --check: passed
  • first native Windows diagnostic rerun: 37 passed before the installer pipe-inheritance timeout identified above
  • LoopX change-quality receipt: cqr_8de9da1b2b723f2a082d, exact-scope valid on the latest main
  • LoopX premerge canary: direct diff checks, Python compile, and selected control-plane canaries passed locally; native Windows behavior remains held for the windows-latest PR check

Risk boundary

This changes Windows process liveness observation and prevents unrelated handle inheritance at runtime launch. It does not change Effect Program interpretation, effect identity, retry safety, journal persistence, adapter behavior, installation semantics, or runtime shutdown authority.

@huangruiteng
huangruiteng force-pushed the codex/fix-windows-effect-runtime-lifecycle branch from 01f0b1d to 1f3252b Compare August 21, 2026 14:05
Signed-off-by: huangruiteng <huangrt01@163.com>
@huangruiteng
huangruiteng force-pushed the codex/fix-windows-effect-runtime-lifecycle branch from 1f3252b to 8a51c51 Compare August 21, 2026 14:10
@huangruiteng

Copy link
Copy Markdown
Owner Author

Maintainer self-review: ready to merge once the final full-suite job is green.

Product and architecture judgment

  • Changed surfaces: only the managed Effect runtime's Windows PID liveness probe and child-process descriptor inheritance, plus focused lifecycle regressions.
  • Why this is the right owner: both failures originate at the existing effect_runtime.py process boundary. The fix stays inside that owner; it does not add another supervisor, adapter, server, persisted schema, or compatibility layer.
  • Operator impact: Windows calls no longer emit a Ctrl-C event while reading runtime metadata, and installer/doctor callers can observe EOF after their short-lived command exits even though the managed Node runtime remains alive.
  • Unaffected contracts: Effect Program interpretation, effect identity, retry/idempotency, journal persistence, adapters, install commands, and explicit runtime shutdown authority are unchanged.
  • Residual risk: the new liveness path uses a small Win32 ctypes boundary. It is exercised by the native windows-latest lifecycle job; POSIX retains the existing kill(pid, 0) path.

Validation evidence

  • Native Windows lifecycle job on the latest base: 41 passed in 64.65s (job completed in 2m36s).
  • Earlier native diagnostic run: eliminated the original KeyboardInterrupt, then exposed a separate installer timeout after 37 passing tests; the final run covers both fixes. This diagnostic failure is not being hidden as merge evidence.
  • Exact Windows workflow selection locally: 35 passed, 6 platform skips.
  • Effect runtime integration: 11 passed.
  • TypeScript Effect core: typecheck passed; 18 native tests passed.
  • Ruff, repository MyPy, git diff --check, and Python compile: passed.
  • LoopX premerge canary: passed, 8/8 selected risk profiles, no manual holds.
  • Exact-scope change-quality receipt: cqr_8de9da1b2b723f2a082d (valid on the current base/head fingerprint).

This coverage is sufficient because the semantic boundary is narrow, the two failure modes have dedicated regression assertions, and the affected OS path has passed the repository's native Windows end-to-end lifecycle workflow. I found no remaining blocker in the diff.

@huangruiteng
huangruiteng merged commit 1285c72 into main Aug 21, 2026
7 checks passed
@huangruiteng
huangruiteng deleted the codex/fix-windows-effect-runtime-lifecycle branch August 21, 2026 14:21
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.

1 participant