Skip to content

fix: suppress guarded process registry Windows footgun#23816

Closed
jackaraus wants to merge 1 commit into
NousResearch:mainfrom
jackaraus:fix/windows-footgun-process-registry
Closed

fix: suppress guarded process registry Windows footgun#23816
jackaraus wants to merge 1 commit into
NousResearch:mainfrom
jackaraus:fix/windows-footgun-process-registry

Conversation

@jackaraus

@jackaraus jackaraus commented May 11, 2026

Copy link
Copy Markdown

Summary

  • Fixes the failing Lint (ruff + ty) / Windows footguns blocking job on main.
  • Keeps the POSIX process-group kill guarded behind not _IS_WINDOWS.
  • Replaces bare signal.SIGKILL with getattr fallback and adds the checker-approved inline suppression for os.killpg.

Evidence

Notes

  • No production runtime change.
  • Targeted process_registry pytest currently has unrelated environment/dependency failures in this worktree: psutil missing and live-system guard blocking subprocess cleanup/PID probes.

@daimon-nous daimon-nous Bot added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets labels May 11, 2026
@briandevans briandevans mentioned this pull request May 12, 2026
Closed
4 tasks
@M3NT8L-One M3NT8L-One mentioned this pull request May 12, 2026
Open
12 tasks
@briandevans briandevans mentioned this pull request May 12, 2026
Closed
4 tasks
@teknium1

teknium1 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Automated hermes-sweeper review: this PR's process_registry Windows-footgun suppression is already implemented on current main.

Evidence:

  • tools/process_registry.py:637 now uses getattr(signal, "SIGKILL", signal.SIGTERM) for the POSIX-only kill signal.
  • tools/process_registry.py:638 keeps the os.killpg(...) call behind if not _IS_WINDOWS and includes the inline windows-footgun: ok suppression marker.
  • Commit 51c68d4ab1a9e3c62fb1048fccb84144c409f0e7 added the equivalent fix on main.
  • python scripts/check-windows-footguns.py --all now passes with ✓ No Windows footguns found (617 file(s) scanned).

Thanks for the targeted fix; main has since picked up the same behavior.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jackaraus @teknium1 @Jackarujo