Skip to content

Widen the pattern-kill validator to cover ui.app, worker, and bridge processes #3316

Description

@tomcounsell

What happened

During lane 3's REVIEW (#3215, PR #3315) the reviewer agent ran pkill -f "python -m ui.app" to stop the throwaway dashboard it had started on port 8517. The pattern also matched the production dashboard on 8500 (pid 10854, launched by scripts/valor-service.sh) and killed it. The agent noticed and restarted it (now pid 88620, /health 200), so the outage was short, but nothing prevented it.

Why the guard let it through

.claude/hooks/validators/validate_no_broad_process_kill.py blocks pattern kills only when the pattern names a test runner (_TEST_RUNNER_PATTERN = r"(?:py\.?test|xdist|pytest-clean)", by design per #2562). CLAUDE.md states the broader rule ("never clear processes by pattern") but the hook enforces the narrow one. Every long-lived service this machine runs is reachable by the same shape:

  • python -m ui.app (dashboard, 8500)
  • python -m worker (session execution engine)
  • bridge/telegram_bridge.py
  • reflection_worker, worker-watchdog

Fix shape

Widen _BLOCK_PATTERNS so pkill -f / killall / kill $(pgrep -f ...) / pgrep ... | xargs kill are blocked when the pattern names any of those services, with a per-service reason that points at the sanctioned stop path (scripts/valor-service.sh stop, worker-stop, kill by PID for a throwaway instance). Keep the test-runner block as is. Add a test row per service pattern plus a negative row for a PID kill, following the existing test file for this validator.

Refs #3177 (surfaced during lane 3 review, #3215).


Triage 2026-09-15

Status: confirmed by code read on main 205344717.

Single-line hotfix: Technically yes — widening _TEST_RUNNER_PATTERN (.claude/hooks/validators/validate_no_broad_process_kill.py:23) to add ui\.app|worker|telegram_bridge|reflection_worker is a one-line regex edit and would have caught this exact incident. But it's a false economy: the enumeration is provably incomplete the moment a new long-lived service ships, and the generic _REASON text already references pytest-specific remediation, so a faithful fix also needs per-service reason text — more than one line in practice.

Reduce-complexity option: Yes, strongly. Enumerating "dangerous" process patterns is an unwinnable arms race — every new service is a silent gap until someone gets burned by it, as just happened. Inverting the guard (block pkill -f / killall / pgrep|kill broadly, with a narrow allowlist for the sanctioned reapers reap-xdist.sh/pytest-clean.sh's own cleanup) deletes the whole class: the default becomes "kill by PID, not by pattern," which is also the rule CLAUDE.md already states in prose. This is the fix to plan for; the pattern-widen is only a stopgap.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingskillsRelated to skills, tools, and SDLC pipeline

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions