From 8081f5444528e72057290f27eb54d48cfaa1f31d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 3 Jul 2025 20:21:27 +0000 Subject: [PATCH 1/4] Add comprehensive PID file mechanism analysis and recommendations Co-authored-by: bas --- pid_file_analysis.md | 135 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 pid_file_analysis.md diff --git a/pid_file_analysis.md b/pid_file_analysis.md new file mode 100644 index 00000000..cc652526 --- /dev/null +++ b/pid_file_analysis.md @@ -0,0 +1,135 @@ +# PID File Mechanism Analysis + +## Current Implementation Overview + +The agent-cli codebase uses a PID file mechanism for process management with the following components: + +### 1. Process Manager (`agent_cli/process_manager.py`) +- **Purpose**: Manages background process lifecycle using PID files +- **Location**: PID files stored in `~/.cache/agent-cli/{process_name}.pid` +- **Key Functions**: + - `pid_file_context()`: Context manager that creates/cleans up PID files + - `is_process_running()`: Checks if process is active using PID file + `os.kill(pid, 0)` + - `kill_process()`: Terminates process and removes PID file + - `get_running_pid()`: Returns PID if process running, cleans up stale files + +### 2. CLI Process Control +The `--stop`, `--status`, and `--toggle` flags are implemented via `stop_or_status_or_toggle()`: +- **`--stop`**: Kills the background process and removes PID file +- **`--status`**: Checks if process is running and shows PID +- **`--toggle`**: Stops if running, shows warning if not running + +### 3. Current Usage in Agents +All agents (voice-assistant, interactive, transcribe, speak) use the same pattern: +```python +with process_manager.pid_file_context(process_name), suppress(KeyboardInterrupt): + # Main agent logic here +``` + +## The "Recording" Context + +Based on the code analysis, "recording" refers to: +1. **Audio input capture** via PyAudio stream (`send_audio()` in `asr.py`) +2. **Continuous listening** for voice commands +3. **Background process lifecycle** where agents run until explicitly stopped + +The `--stop` command doesn't just "stop recording and start processing" - it **terminates the entire background process**. + +## Analysis: Is PID File Mechanism Needed? + +### ❌ **Arguments Against PID Files** + +#### 1. **Misleading Mental Model** +- The user's description suggests `--stop` should pause recording → process → resume +- Reality: `--stop` kills the entire process, requiring manual restart +- This disconnect suggests the PID mechanism doesn't match the intended workflow + +#### 2. **Signal Handling Already Exists** +- The codebase has sophisticated signal handling via `signal_handling_context()`: + - First Ctrl+C: Graceful shutdown (stops recording, processes transcription) + - Second Ctrl+C: Force exit + - SIGTERM: Immediate graceful shutdown +- **Recording control is already handled via signals without PID files** + +#### 3. **Race Conditions & Cleanup Issues** +- Stale PID files require cleanup logic when processes crash +- Multiple instances of same agent cannot run simultaneously +- File system state can become inconsistent + +#### 4. **Workflow Mismatch** +For the described workflow ("stop recording → process → resume"), the current approach requires: +```bash +agent-cli voice-assistant --stop # Kill entire process +agent-cli voice-assistant & # Start new process +``` +Instead of a simple pause/resume mechanism. + +### ✅ **Arguments For PID Files** + +#### 1. **Cross-Process Communication** +- Enables external tools (Keyboard Maestro, scripts) to control background processes +- `--status` provides process discovery for automation +- Useful for system integration where processes start via different mechanisms + +#### 2. **Process Lifecycle Management** +- Prevents multiple instances of same agent +- Provides clean shutdown mechanism via `--stop` +- Enables "toggle" functionality for hotkey integration + +#### 3. **Background Process Tracking** +- Users can check if agents are running without guessing +- System administrators can monitor active agent processes + +## Recommendations + +### Option 1: **Remove PID Files** (If workflow is pause/resume) +If the intended behavior is truly "stop recording → process → resume recording": + +```python +# Replace current approach with in-process control +class VoiceAssistant: + def __init__(self): + self.recording = False + self.processing = False + + async def handle_stop_signal(self): + """Stop recording, process, then resume""" + self.recording = False + self.processing = True + # Process transcription + await self.process_command() + self.processing = False + self.recording = True +``` + +**Pros**: Simpler, matches described workflow, no file system dependencies +**Cons**: Loses cross-process control capabilities + +### Option 2: **Keep PID Files** (If background process control is needed) +If external process control is valuable: + +1. **Clarify documentation** - `--stop` kills the process, doesn't pause +2. **Add pause/resume functionality** via additional signals: + ```bash + kill -USR1 $(cat ~/.cache/agent-cli/voice-assistant.pid) # Pause + kill -USR2 $(cat ~/.cache/agent-cli/voice-assistant.pid) # Resume + ``` +3. **Consider adding `--pause`/`--resume` CLI flags** + +### Option 3: **Hybrid Approach** +- Keep PID files for process management (`--stop`, `--status`, `--toggle`) +- Add in-process pause/resume via signals (USR1/USR2) +- Document the distinction clearly + +## Conclusion + +**The PID file mechanism appears to be over-engineered for the described use case.** + +If the primary goal is "stop recording → process → resume recording," then: +- **Signal handling already provides this functionality** (Ctrl+C stops recording, processes transcription) +- **PID files add complexity without solving the core problem** +- **The current `--stop` behavior (process termination) doesn't match the described workflow** + +**Recommendation**: Remove PID files and rely on signal-based control for recording lifecycle. This would simplify the codebase while providing the exact workflow described. + +If cross-process control is truly needed for automation, keep PID files but clarify that `--stop` means "terminate process," not "pause recording." \ No newline at end of file From b49ad47c247ea3516ff60d0f08b67f590b84cbfb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 3 Jul 2025 20:31:17 +0000 Subject: [PATCH 2/4] Enhance PID file analysis with Keyboard Maestro integration details Co-authored-by: bas --- pid_file_analysis.md | 184 +++++++++++++++++++++++++++---------------- 1 file changed, 114 insertions(+), 70 deletions(-) diff --git a/pid_file_analysis.md b/pid_file_analysis.md index cc652526..d35ea732 100644 --- a/pid_file_analysis.md +++ b/pid_file_analysis.md @@ -26,6 +26,36 @@ with process_manager.pid_file_context(process_name), suppress(KeyboardInterrupt) # Main agent logic here ``` +## The Keyboard Maestro Integration + +The **voice-assistant** has detailed instructions for Keyboard Maestro integration: + +``` +KEYBOARD MAESTRO INTEGRATION: +To create a hotkey toggle for this script, set up a Keyboard Maestro macro with: + +1. Trigger: Hot Key (e.g., Cmd+Shift+A for "Assistant") + +2. If/Then/Else Action: + - Condition: Shell script returns success + - Script: voice-assistant --status >/dev/null 2>&1 + +3. Then Actions (if process is running): + - Display Text Briefly: "🗣️ Processing command..." + - Execute Shell Script: voice-assistant --stop --quiet + - (The script will show its own "Done" notification) + +4. Else Actions (if process is not running): + - Display Text Briefly: "📋 Listening for command..." + - Execute Shell Script: voice-assistant --input-device-index 1 --quiet & + - Select "Display results in a notification" +``` + +**This workflow relies entirely on the PID file mechanism for:** +1. **Status checking** (`--status`) to determine if the process is running +2. **Process termination** (`--stop`) to stop the background listener +3. **Single hotkey toggle behavior** - same key starts/stops the agent + ## The "Recording" Context Based on the code analysis, "recording" refers to: @@ -33,103 +63,117 @@ Based on the code analysis, "recording" refers to: 2. **Continuous listening** for voice commands 3. **Background process lifecycle** where agents run until explicitly stopped -The `--stop` command doesn't just "stop recording and start processing" - it **terminates the entire background process**. +The `--stop` command doesn't just "stop recording and start processing" - it **terminates the entire background process**. But this is **intentional for the Keyboard Maestro workflow**: + +1. Copy text to clipboard +2. Press hotkey → starts `voice-assistant &` (background listening) +3. Speak command +4. Press same hotkey → runs `voice-assistant --stop` (stops listening, processes command, exits) +5. Result is in clipboard ## Analysis: Is PID File Mechanism Needed? -### ❌ **Arguments Against PID Files** +### ✅ **Strong Arguments FOR PID Files** -#### 1. **Misleading Mental Model** -- The user's description suggests `--stop` should pause recording → process → resume -- Reality: `--stop` kills the entire process, requiring manual restart -- This disconnect suggests the PID mechanism doesn't match the intended workflow +#### 1. **Essential for Keyboard Maestro Integration** +- **Status checking**: `--status` enables conditional logic in KM macros +- **Toggle behavior**: Single hotkey can start/stop based on current state +- **Background process control**: Enables external automation to manage long-running processes +- **Cross-session persistence**: KM can detect if agent is running even after system restarts -#### 2. **Signal Handling Already Exists** -- The codebase has sophisticated signal handling via `signal_handling_context()`: - - First Ctrl+C: Graceful shutdown (stops recording, processes transcription) - - Second Ctrl+C: Force exit - - SIGTERM: Immediate graceful shutdown -- **Recording control is already handled via signals without PID files** +#### 2. **Prevents Multiple Instance Issues** +- Only one voice-assistant can run at a time (prevents audio conflicts) +- Clean process lifecycle management +- Prevents resource leaks from abandoned processes -#### 3. **Race Conditions & Cleanup Issues** -- Stale PID files require cleanup logic when processes crash -- Multiple instances of same agent cannot run simultaneously -- File system state can become inconsistent +#### 3. **System Integration** +- Standard Unix process management pattern +- Works with any external automation tool (not just KM) +- Enables scripting and monitoring by system administrators -#### 4. **Workflow Mismatch** -For the described workflow ("stop recording → process → resume"), the current approach requires: -```bash -agent-cli voice-assistant --stop # Kill entire process -agent-cli voice-assistant & # Start new process -``` -Instead of a simple pause/resume mechanism. +#### 4. **Clean Architecture** +- Separates process control from application logic +- Enables background operation with foreground control +- Provides reliable cleanup on exit -### ✅ **Arguments For PID Files** +### ❌ **Minor Arguments Against PID Files** -#### 1. **Cross-Process Communication** -- Enables external tools (Keyboard Maestro, scripts) to control background processes -- `--status` provides process discovery for automation -- Useful for system integration where processes start via different mechanisms +#### 1. **Terminology Confusion** +- "Stop recording" suggests pause/resume, but `--stop` terminates the process +- This is actually **correct behavior** for the KM workflow, just confusing terminology -#### 2. **Process Lifecycle Management** -- Prevents multiple instances of same agent -- Provides clean shutdown mechanism via `--stop` -- Enables "toggle" functionality for hotkey integration +#### 2. **File System Dependencies** +- PID files can become stale if processes crash +- But the code handles cleanup well (`get_running_pid()` cleans stale files) -#### 3. **Background Process Tracking** -- Users can check if agents are running without guessing -- System administrators can monitor active agent processes +## Alternative Architectures (NOT Recommended) -## Recommendations +### Option 1: Signal-Based Control Only +```bash +# Instead of PID files, use signals +kill -USR1 $(pgrep voice-assistant) # Stop recording, process +kill -USR2 $(pgrep voice-assistant) # Resume recording +``` -### Option 1: **Remove PID Files** (If workflow is pause/resume) -If the intended behavior is truly "stop recording → process → resume recording": +**Problems**: +- **Breaks KM integration**: No way to check if process is running reliably +- **Multiple instances**: No prevention of duplicate processes +- **Complex state management**: Application needs internal state machine +- **Poor user experience**: No simple status checking +### Option 2: In-Process Pause/Resume ```python -# Replace current approach with in-process control class VoiceAssistant: - def __init__(self): + def handle_pause_signal(self): self.recording = False - self.processing = False - - async def handle_stop_signal(self): - """Stop recording, process, then resume""" - self.recording = False - self.processing = True - # Process transcription - await self.process_command() - self.processing = False - self.recording = True + # Process and exit ``` -**Pros**: Simpler, matches described workflow, no file system dependencies -**Cons**: Loses cross-process control capabilities +**Problems**: +- **Wrong mental model**: KM workflow expects process termination, not pause +- **Breaks the clipboard workflow**: Process needs to exit to complete the task +- **Session management**: Would need persistent process for clipboard operations + +## Recommendations + +### ✅ **Keep PID Files** (Strongly Recommended) +The PID file mechanism is **perfectly designed** for your Keyboard Maestro workflow: + +1. **Maintain current architecture** - it's working as intended +2. **Clarify documentation** to explain the KM integration context +3. **Update terminology** - `--stop` means "complete the task and exit" not "pause recording" -### Option 2: **Keep PID Files** (If background process control is needed) -If external process control is valuable: +### Possible Improvements -1. **Clarify documentation** - `--stop` kills the process, doesn't pause -2. **Add pause/resume functionality** via additional signals: +1. **Better naming**: ```bash - kill -USR1 $(cat ~/.cache/agent-cli/voice-assistant.pid) # Pause - kill -USR2 $(cat ~/.cache/agent-cli/voice-assistant.pid) # Resume + --complete # Instead of --stop (complete task and exit) + --terminate # More explicit about process termination ``` -3. **Consider adding `--pause`/`--resume` CLI flags** -### Option 3: **Hybrid Approach** -- Keep PID files for process management (`--stop`, `--status`, `--toggle`) -- Add in-process pause/resume via signals (USR1/USR2) -- Document the distinction clearly +2. **Enhanced status messages**: + ```bash + agent-cli voice-assistant --status + # Output: "✅ Voice assistant is listening for commands (PID: 1234)" + ``` + +3. **Add `--toggle` examples to KM docs**: + ```bash + voice-assistant --toggle # Simpler than if/then/else logic + ``` ## Conclusion -**The PID file mechanism appears to be over-engineered for the described use case.** +**The PID file mechanism is ESSENTIAL and well-designed for your use case.** + +Your Keyboard Maestro workflow depends on: +- ✅ **Status checking** to determine current state +- ✅ **Background process management** for the listening phase +- ✅ **Reliable termination** to complete the clipboard workflow +- ✅ **Single hotkey toggle** behavior -If the primary goal is "stop recording → process → resume recording," then: -- **Signal handling already provides this functionality** (Ctrl+C stops recording, processes transcription) -- **PID files add complexity without solving the core problem** -- **The current `--stop` behavior (process termination) doesn't match the described workflow** +**DO NOT remove PID files** - they are the foundation that makes your automation workflow possible. -**Recommendation**: Remove PID files and rely on signal-based control for recording lifecycle. This would simplify the codebase while providing the exact workflow described. +The only "issue" is terminological: `--stop` doesn't mean "pause recording" in your context - it means "complete the voice command task and exit." This is actually the correct behavior for a clipboard-based workflow where each hotkey press represents a complete interaction cycle. -If cross-process control is truly needed for automation, keep PID files but clarify that `--stop` means "terminate process," not "pause recording." \ No newline at end of file +**Recommendation**: Keep the PID mechanism exactly as it is. It's a well-architected solution for your automation needs. \ No newline at end of file From 3b1f9fc73d37d612a23254e5bd112acc2ae431f8 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 4 Jul 2025 15:11:09 +0000 Subject: [PATCH 3/4] Improve interactive agent stop behavior with SIGINT and double-stop logic Co-authored-by: bas --- agent_cli/utils.py | 49 +++++++++++++++++++++++++++++++++++++------- pid_file_analysis.md | 34 +++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/agent_cli/utils.py b/agent_cli/utils.py index 4adb2180..1cf052b3 100644 --- a/agent_cli/utils.py +++ b/agent_cli/utils.py @@ -207,11 +207,31 @@ def stop_or_status_or_toggle( ) -> bool: """Handle process control for a given process name.""" if stop: - if process_manager.kill_process(process_name): - if not quiet: - print_with_style(f"✅ {which.capitalize()} stopped.") - elif not quiet: - print_with_style(f"⚠️ No {which} is running.", style="yellow") + # For interactive agent, send SIGINT (like Ctrl+C) instead of SIGTERM + # This allows it to handle stop signals gracefully with double-stop logic + if process_name == "interactive": + pid = process_manager.get_running_pid(process_name) + if pid: + try: + import os + import signal + os.kill(pid, signal.SIGINT) + if not quiet: + print_with_style(f"✅ Sent stop signal to {which}.") + except (ProcessLookupError, PermissionError): + # Process already dead or no permission, clean up PID file + process_manager.kill_process(process_name) + if not quiet: + print_with_style(f"✅ {which.capitalize()} stopped.") + elif not quiet: + print_with_style(f"⚠️ No {which} is running.", style="yellow") + else: + # For other agents, kill immediately as before + if process_manager.kill_process(process_name): + if not quiet: + print_with_style(f"✅ {which.capitalize()} stopped.") + elif not quiet: + print_with_style(f"⚠️ No {which} is running.", style="yellow") return True if status: @@ -225,8 +245,23 @@ def stop_or_status_or_toggle( if toggle: if process_manager.is_process_running(process_name): - if process_manager.kill_process(process_name) and not quiet: - print_with_style(f"✅ {which.capitalize()} stopped.") + # Use the same logic as stop for consistency + if process_name == "interactive": + pid = process_manager.get_running_pid(process_name) + if pid: + try: + import os + import signal + os.kill(pid, signal.SIGINT) + if not quiet: + print_with_style(f"✅ Sent stop signal to {which}.") + except (ProcessLookupError, PermissionError): + process_manager.kill_process(process_name) + if not quiet: + print_with_style(f"✅ {which.capitalize()} stopped.") + else: + if process_manager.kill_process(process_name) and not quiet: + print_with_style(f"✅ {which.capitalize()} stopped.") return True if not quiet: print_with_style(f"⚠️ {which.capitalize()} is not running.", style="yellow") diff --git a/pid_file_analysis.md b/pid_file_analysis.md index d35ea732..bf12fdea 100644 --- a/pid_file_analysis.md +++ b/pid_file_analysis.md @@ -56,6 +56,32 @@ To create a hotkey toggle for this script, set up a Keyboard Maestro macro with: 2. **Process termination** (`--stop`) to stop the background listener 3. **Single hotkey toggle behavior** - same key starts/stops the agent +## Interactive Agent Special Case + +The **interactive** agent has different `--stop` behavior than other agents: + +### Problem +- **Other agents**: `--stop` should kill the entire process (one-shot tasks) +- **Interactive agent**: `--stop` should behave like Ctrl+C with double-stop logic: + - First `--stop`: Stop current recording, continue conversation loop + - Second `--stop`: Exit the entire program + +### Solution +Modified `stop_or_status_or_toggle()` to handle interactive agent differently: + +```python +if process_name == "interactive": + # Send SIGINT (like Ctrl+C) instead of SIGTERM + os.kill(pid, signal.SIGINT) +else: + # Kill immediately as before + process_manager.kill_process(process_name) +``` + +This allows the interactive agent to use its existing signal handling logic: +- First SIGINT: Set stop event, process current turn, continue loop +- Second SIGINT: Force exit with code 130 + ## The "Recording" Context Based on the code analysis, "recording" refers to: @@ -96,6 +122,11 @@ The `--stop` command doesn't just "stop recording and start processing" - it **t - Enables background operation with foreground control - Provides reliable cleanup on exit +#### 5. **Interactive Agent Support** +- Allows external control of long-running conversations +- Double-stop logic matches double Ctrl+C behavior +- Preserves conversation state between stop signals + ### ❌ **Minor Arguments Against PID Files** #### 1. **Terminology Confusion** @@ -137,7 +168,7 @@ class VoiceAssistant: ## Recommendations ### ✅ **Keep PID Files** (Strongly Recommended) -The PID file mechanism is **perfectly designed** for your Keyboard Maestro workflow: +The PID file mechanism is **perfectly designed** for your workflow: 1. **Maintain current architecture** - it's working as intended 2. **Clarify documentation** to explain the KM integration context @@ -171,6 +202,7 @@ Your Keyboard Maestro workflow depends on: - ✅ **Background process management** for the listening phase - ✅ **Reliable termination** to complete the clipboard workflow - ✅ **Single hotkey toggle** behavior +- ✅ **Interactive agent double-stop support** (like double Ctrl+C) **DO NOT remove PID files** - they are the foundation that makes your automation workflow possible. From cb2a5501282223bdd81dd252193547d889913b04 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 4 Jul 2025 15:13:40 +0000 Subject: [PATCH 4/4] Refactor process stop handling with configurable stop signals Co-authored-by: bas --- agent_cli/agents/interactive.py | 2 + agent_cli/utils.py | 71 +++++++++++++++++++-------------- pid_file_analysis.md | 39 ++++++++++++++---- 3 files changed, 73 insertions(+), 39 deletions(-) diff --git a/agent_cli/agents/interactive.py b/agent_cli/agents/interactive.py index f6c363e1..eed0e71e 100644 --- a/agent_cli/agents/interactive.py +++ b/agent_cli/agents/interactive.py @@ -16,6 +16,7 @@ import json import logging import os +import signal import time from contextlib import suppress from datetime import UTC, datetime @@ -477,6 +478,7 @@ def interactive( status, toggle, quiet=general_cfg.quiet, + stop_signal=signal.SIGINT, # Use SIGINT for graceful double-stop behavior ): return diff --git a/agent_cli/utils.py b/agent_cli/utils.py index 1cf052b3..a0174866 100644 --- a/agent_cli/utils.py +++ b/agent_cli/utils.py @@ -204,34 +204,44 @@ def stop_or_status_or_toggle( toggle: bool, *, quiet: bool = False, + stop_signal: int = signal.SIGTERM, ) -> bool: - """Handle process control for a given process name.""" + """Handle process control for a given process name. + + Args: + process_name: Name of the process to control + which: Human-readable description of the process + stop: Whether to stop the process + status: Whether to check process status + toggle: Whether to toggle the process + quiet: Whether to suppress output + stop_signal: Signal to send when stopping (SIGTERM for kill, SIGINT for graceful) + + Returns: + True if any action was taken, False otherwise + """ if stop: - # For interactive agent, send SIGINT (like Ctrl+C) instead of SIGTERM - # This allows it to handle stop signals gracefully with double-stop logic - if process_name == "interactive": - pid = process_manager.get_running_pid(process_name) - if pid: - try: - import os - import signal - os.kill(pid, signal.SIGINT) + pid = process_manager.get_running_pid(process_name) + if pid: + try: + import os + os.kill(pid, stop_signal) + if stop_signal == signal.SIGINT: + # For graceful stops (SIGINT), don't remove PID file - let process handle it if not quiet: print_with_style(f"✅ Sent stop signal to {which}.") - except (ProcessLookupError, PermissionError): - # Process already dead or no permission, clean up PID file + else: + # For force kills (SIGTERM), clean up PID file immediately process_manager.kill_process(process_name) if not quiet: print_with_style(f"✅ {which.capitalize()} stopped.") - elif not quiet: - print_with_style(f"⚠️ No {which} is running.", style="yellow") - else: - # For other agents, kill immediately as before - if process_manager.kill_process(process_name): + except (ProcessLookupError, PermissionError): + # Process already dead or no permission, clean up PID file + process_manager.kill_process(process_name) if not quiet: print_with_style(f"✅ {which.capitalize()} stopped.") - elif not quiet: - print_with_style(f"⚠️ No {which} is running.", style="yellow") + elif not quiet: + print_with_style(f"⚠️ No {which} is running.", style="yellow") return True if status: @@ -245,23 +255,22 @@ def stop_or_status_or_toggle( if toggle: if process_manager.is_process_running(process_name): - # Use the same logic as stop for consistency - if process_name == "interactive": - pid = process_manager.get_running_pid(process_name) - if pid: - try: - import os - import signal - os.kill(pid, signal.SIGINT) + pid = process_manager.get_running_pid(process_name) + if pid: + try: + import os + os.kill(pid, stop_signal) + if stop_signal == signal.SIGINT: if not quiet: print_with_style(f"✅ Sent stop signal to {which}.") - except (ProcessLookupError, PermissionError): + else: process_manager.kill_process(process_name) if not quiet: print_with_style(f"✅ {which.capitalize()} stopped.") - else: - if process_manager.kill_process(process_name) and not quiet: - print_with_style(f"✅ {which.capitalize()} stopped.") + except (ProcessLookupError, PermissionError): + process_manager.kill_process(process_name) + if not quiet: + print_with_style(f"✅ {which.capitalize()} stopped.") return True if not quiet: print_with_style(f"⚠️ {which.capitalize()} is not running.", style="yellow") diff --git a/pid_file_analysis.md b/pid_file_analysis.md index bf12fdea..4b581f3b 100644 --- a/pid_file_analysis.md +++ b/pid_file_analysis.md @@ -67,15 +67,31 @@ The **interactive** agent has different `--stop` behavior than other agents: - Second `--stop`: Exit the entire program ### Solution -Modified `stop_or_status_or_toggle()` to handle interactive agent differently: +Added a `stop_signal` parameter to `stop_or_status_or_toggle()` for configurable behavior: ```python -if process_name == "interactive": - # Send SIGINT (like Ctrl+C) instead of SIGTERM - os.kill(pid, signal.SIGINT) -else: - # Kill immediately as before - process_manager.kill_process(process_name) +def stop_or_status_or_toggle( + process_name: str, + which: str, + stop: bool, + status: bool, + toggle: bool, + *, + quiet: bool = False, + stop_signal: int = signal.SIGTERM, # Default to force kill +) -> bool: +``` + +Interactive agent now calls it with `stop_signal=signal.SIGINT`: + +```python +stop_or_status_or_toggle( + "interactive", + "interactive agent", + stop, status, toggle, + quiet=quiet, + stop_signal=signal.SIGINT, # Graceful double-stop behavior +) ``` This allows the interactive agent to use its existing signal handling logic: @@ -174,7 +190,14 @@ The PID file mechanism is **perfectly designed** for your workflow: 2. **Clarify documentation** to explain the KM integration context 3. **Update terminology** - `--stop` means "complete the task and exit" not "pause recording" -### Possible Improvements +### Implemented Improvements + +1. **Clean parameterized signal handling**: + - No special casing based on process names + - Configurable stop behavior via `stop_signal` parameter + - Maintains single responsibility principle + +### Possible Future Improvements 1. **Better naming**: ```bash