Skip to content

fix(windows): support background task cancellation - #111

Open
pj991207 wants to merge 1 commit into
sendbird:mainfrom
pj991207:fix/windows-process-identity
Open

pj991207 wants to merge 1 commit into
sendbird:mainfrom
pj991207:fix/windows-process-identity

Conversation

@pj991207

@pj991207 pj991207 commented Sep 14, 2026 •

Copy link
Copy Markdown

Closes #110

Summary

On native Windows, background jobs cannot be cancelled because their PID identity is missing; once an identity is available, cancellation still takes the Unix process-group path. This change fixes both steps:

  • Store the Windows process start time as the stable PID identity, preserving the existing PID-reuse guard.
  • Route Windows cancellation through the existing taskkill /T /F process-tree helper.
  • Make the process tests portable and include them in the Windows CI suite.

Verification

Revalidated on 2026-10-14 with native Windows and Node.js 24.17.0 at 9a60624a82d43749a03536ef9f83d0324bbc6eb3. The branch already contains current main (19e565151f35b328a5b9433df351bd8f3818fdc7), so no merge or rebase was needed.

  • npm ci completed.
  • npm run test:cross-platform: 299 passed, 0 failed.
  • npm run lint, npm run typecheck, npm run check:version-sync, and npm run check:changelog passed as the initial stages of npm run check.
  • A fresh live Windows smoke check confirmed stable process identity, preservation of a running parent/child tree when the stored identity mismatches, termination of both processes with the matching identity, and safe repeated cancellation.
  • The original live companion-job check also confirmed cancellation removed the claude.exe, Bash, and Node process tree.

The full npm run check is not reported as passing on this Windows host. Its broader unit suite encountered Windows failures and was stopped before integration/e2e. The repository runs this full suite on Linux/macOS and the curated suite above on Windows; Linux/macOS CI still needs maintainer approval. The additional failures were:

  • codex-config: retries writable-root updates without losing a concurrent config change fails in afterEach temporary-directory cleanup with EPERM. The same failure was reproduced against an untouched archive of current main.
  • git: gracefully handles untracked directory contents and symlinks in working-tree review context fails creating a symlink with EPERM. The two omits very large ... diffs tests and degrades gracefully when working-tree diff output exceeds the process buffer fail in temporary-directory cleanup with EPERM. These test cases and scripts/lib/git.mjs are unchanged from main.

@pj991207

Copy link
Copy Markdown
Author

Hi maintainers! I rechecked this against current main; the branch is already up to date. The curated Windows suite (npm run test:cross-platform) passes all 299 tests locally, and lint, typecheck, version sync, and changelog checks pass. A fresh live Windows check also confirmed that cancellation removes the parent and child processes while a mismatched PID identity leaves them running.

I've refreshed the PR description with the results and the separate native-Windows full-suite limitations. The CI workflow is still awaiting approval. Could someone please approve the workflow and review the fix when convenient? Thanks!

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.

Windows: background job cancellation fails without a process identity

1 participant