diff --git a/.gitignore b/.gitignore index 128512b77..b6f47bf61 100644 --- a/.gitignore +++ b/.gitignore @@ -23,5 +23,6 @@ _testmain.go *.test *.prof .idea/ +.claude/ che-machine-exec diff --git a/devfile.yaml b/devfile.yaml index dd1dbb810..8d881521c 100644 --- a/devfile.yaml +++ b/devfile.yaml @@ -32,7 +32,7 @@ components: type: terminal-dev protocol: ws secure: false - image: registry.access.redhat.com/ubi9/go-toolset:1.25.5-1770596585 + image: registry.access.redhat.com/ubi9/go-toolset:1.25.9-1778675823 args: - tail - '-f' diff --git a/timeout/.noidle.example b/timeout/.noidle.example new file mode 100644 index 000000000..86442a1d1 --- /dev/null +++ b/timeout/.noidle.example @@ -0,0 +1,74 @@ +# Example .noidle configuration for CLI Watcher +# +# This file shows all available configuration options. +# For minimal config, see .noidle.minimal (just 'enabled: true' is enough!) +# +# Place this file in your project directory, or in $HOME/.noidle +# You can also set CLI_WATCHER_CONFIG environment variable to specify a custom path. +# +# NOTE: Comments (lines starting with #) are part of YAML syntax and work fine. +# Copy any section below directly into your .noidle file. + +enabled: true +checkPeriod: 30 # How often to check for active processes (accepts: 30, 30s, etc.) +activityWindow: 25m # How long to wait for activity from interactive processes +gracePeriod: 5m # All processes prevent idling when this young +maxProcessAge: 6h # Safety limit to prevent indefinite idling prevention + +watchedCommands: + # Simple string format (backward compatible) + # These commands will be auto-detected (interactive vs work process) + - claude # AI assistant, will be auto-detected as interactive (checks for user input activities) + - gemini # AI assistant, will be auto-detected as interactive (checks for user input activities) + - kubectl # Kubernetes deployment, will be auto-detected as non-interactive (always prevents idling) + + # Object format with explicit interactive mode control + - name: vim + interactive: auto # Auto-detect based on foreground + TTY read behavior + + # Force interactive mode (always check for user input activity) + - name: claude + interactive: true # Force interactive even if auto-detection thinks otherwise + + # Long-running build/deploy commands - force non-interactive (always prevent idling) + - name: npm + interactive: false # Force non-interactive (always prevent idling) + + - name: docker + interactive: no # Same as 'false' + + - name: gradle + interactive: false # Force non-interactive (always prevent idling during builds) + + # Override always-ignored commands (USE WITH CAUTION) + - name: watch + interactive: false + forceWatch: true # Override always-ignored list to watch 'watch' command + # Normally 'watch', 'top', 'htop', 'tail' are always ignored + + - name: top + forceWatch: yes # Also accepts: true, yes, false, no (default: false) + +# Interactive mode options: +# - auto: Auto-detect (foreground + has read from TTY → interactive, otherwise → work process) +# - true/yes: Force interactive (always check for user input activity) +# - false/no: Force non-interactive work process (always prevent idling, default) +# +# ForceWatch option (optional, defaults to false): +# - true/yes: Override always-ignored list (tail, watch, top, htop) to monitor this command +# - false/no: Respect always-ignored list (default behavior) +# WARNING: Use forceWatch with caution. Always-ignored commands are typically passive +# monitoring tools that shouldn't prevent workspace idling. +# +# Global settings: +# - activityWindowSeconds: How long to wait for input from interactive processes (default: 1500 = 25 min) +# - gracePeriodSeconds: All processes prevent idling when younger than this (default: 300 = 5 min) +# - checkPeriodSeconds: How often to scan for processes (default: 60 = 1 min) +# +# Unconfigured commands: +# - Auto-detected as interactive or work process during grace period +# - After grace period: interactive processes checked for activity, work processes always prevent idling +# +# Always-ignored commands (never prevent idling): +# - tail, watch, top, htop (passive monitoring tools) +# These are always ignored even if explicitly listed in watchedCommands. diff --git a/timeout/.noidle.minimal b/timeout/.noidle.minimal new file mode 100644 index 000000000..7fe60145c --- /dev/null +++ b/timeout/.noidle.minimal @@ -0,0 +1,32 @@ +# Minimal .noidle configuration +# +# This is the absolute minimum needed to enable CLI watcher. +# All user processes will be watched with smart defaults. +# +# NOTE: Comments (lines starting with #) are supported in YAML. +# You can copy these examples directly into your .noidle file. + +enabled: true + +# That's it! With just this, the CLI watcher will: +# +# ✅ Watch ALL user-initiated processes (processes with TTY from user terminals) +# ✅ Auto-detect interactive processes (vim, python REPL, etc.) vs work processes (builds, deploys) +# ✅ Interactive processes: check for user input activity within 25 minutes +# ✅ Work processes: always prevent idling while running +# ✅ All processes: prevent idling for first 5 minutes (grace period) +# ✅ Safety limit: stop preventing idling after 6 hours (catches hung/forgotten processes) +# ✅ Ignore passive monitoring tools: tail, watch, top, htop +# +# You can override any defaults: +# +# checkPeriod: 45 # Check every 45 seconds instead of default 60 +# activityWindow: 30m # 30 minutes instead of default 25m +# gracePeriod: 10m # 10 minutes instead of default 5m +# maxProcessAge: 8h # 8 hours instead of default 6h +# +# Explicitly configure specific commands only if you need to override auto-detection: +# +# watchedCommands: +# - name: myTool +# interactive: false # Force non-interactive (always prevent idling) diff --git a/timeout/CLI-WATCHER.md b/timeout/CLI-WATCHER.md new file mode 100644 index 000000000..7f4fd5c14 --- /dev/null +++ b/timeout/CLI-WATCHER.md @@ -0,0 +1,833 @@ +# CLI Watcher - Prevent Workspace Idling During Long-Running Commands + +The CLI Watcher monitors running CLI processes and prevents workspace idling during active development. This is particularly useful in containerized development environments like Eclipse Che where long-running deployments, builds, or interactive sessions shouldn't trigger automatic workspace shutdown. + +## How It Works + +The watcher periodically scans `/proc` to detect **all user-initiated CLI processes** (processes with TTY from user terminals). When an active process is found, it triggers a callback that resets the workspace idle timeout. + +**Key behavior**: +- **ALL user processes are watched** automatically (no explicit configuration needed) +- **Configured commands** (`watchedCommands`) allow you to override auto-detection behavior +- **Unconfigured commands** are intelligently classified as interactive or work processes after grace period + +## Upgrading from Previous Versions + +**⚠️ BREAKING BEHAVIORAL CHANGE:** The CLI Watcher now watches **ALL user-initiated terminal processes** by default, not just those explicitly listed in `watchedCommands`. + +### What Changed + +**Before (old behavior):** +- Only commands listed in `watchedCommands` were monitored +- Other processes were completely ignored +- Only `tail` was globally excluded + +**After (new behavior):** +- **ALL user processes with TTY are monitored automatically** +- `watchedCommands` now **overrides auto-detection** for specific commands (not required to enable watching) +- `tail`, `watch`, `top`, `htop` are now **always ignored** (expanded exclusion list) + +### Impact on Your Workspace + +1. **Workspaces may stay active longer** - processes that were previously ignored (shells, scripts, REPLs) now prevent idling +2. **Commands in `watchedCommands` may behave differently**: + - If you configured `watch`, `top`, or `htop` → now ignored with a warning + - If you only listed specific commands → other user processes are now also monitored +3. **Auto-detection may differ from your expectations** - interactive processes (vim, python REPL) only prevent idling when actively used + +### Migration Steps + +**If you have an existing `.noidle` configuration:** + +1. **Review your current `watchedCommands` list** + ```yaml + # Old config - only these were watched + watchedCommands: + - helm + - kubectl + - watch # ⚠️ Now globally ignored! + ``` + +2. **Understand the new behavior**: + - All user terminal processes are now watched (helm, kubectl, vim, bash scripts, etc.) + - `watchedCommands` is now for **overriding** auto-detection, not enabling watching + - Remove `watch`, `top`, `htop` from your config (they're always ignored) + +3. **Option A: Embrace auto-detection** (recommended for most users) + ```yaml + # Minimal config - let auto-detection handle everything + enabled: true + ``` + The watcher will automatically distinguish interactive (vim, REPLs) from work processes (builds, deploys). + +4. **Option B: Restrict to specific commands only** + ```yaml + enabled: true + + # Override auto-detection for specific commands + watchedCommands: + - helm + - kubectl + + # Add all other commands you DON'T want watched + ignoredCommands: + - bash + - sh + - python3 + - node + # ... any other commands you want to ignore + ``` + +5. **Test in a non-production workspace first** - verify idle timeout behavior matches your expectations + +### Examples + +**Example 1: Old config watching only deployments** +```yaml +# Before +watchedCommands: + - helm + - kubectl + - odo +``` + +**After migration (Option A - auto-detect):** +```yaml +# After - auto-detection handles everything +enabled: true + +# Optional: Force these to always prevent idling (skip auto-detection) +watchedCommands: + - helm + - kubectl + - odo +``` + +**Example 2: Old config with globally-excluded commands** +```yaml +# Before +watchedCommands: + - watch # Monitoring logs + - kubectl +``` + +**After migration:** +```yaml +# After - remove 'watch' (now globally ignored) +enabled: true + +watchedCommands: + - kubectl # Keep kubectl if you want to override auto-detection + +# WARNING will be logged: +# "You configured [watch] in watchedCommands, but these are globally excluded" +``` + +### Verification + +After updating your configuration: + +1. Check logs for warnings about globally-excluded commands +2. Monitor workspace idle timeout behavior +3. Use `LOG_LEVEL=debug` to see which processes are detected and classified +4. Refer to the [Testing](#testing) section for validation scenarios + +### Rollback + +If the new behavior doesn't suit your workflow: + +1. Use `ignoredCommands` to exclude unwanted processes +2. Set explicit `interactive` modes in `watchedCommands` to override auto-detection +3. Contact your platform administrator if workspace idle policies need adjustment + +## Configuration + +### Configuration File Requirement + +**IMPORTANT**: The CLI Watcher requires a configuration file to enable watching. Without a config file, NO processes will be watched and workspaces will idle normally. + +**Minimum Required Configuration**: +```yaml +enabled: true +``` + +That's it! With just this one line: +- ✅ ALL user processes are automatically watched +- ✅ Interactive vs. work processes are auto-detected +- ✅ All settings use smart defaults (grace period: 5min, activity window: 25min, max age: 6h) +- ✅ Passive monitoring tools (tail, watch, top, htop) are automatically ignored + +### Configuration File Locations + +The CLI Watcher looks for a `.noidle` configuration file in the following order: + +1. **Explicit override**: Set via `CLI_WATCHER_CONFIG` environment variable +2. **Project directory**: Search upward from `$PROJECT_SOURCE` to `$PROJECTS_ROOT` for `.noidle` +3. **Home directory**: Fallback to `$HOME/.noidle` + +If no config file is found, the watcher runs but does NOT prevent idling (waits for config to appear). + +### Basic Configuration (Backward Compatible) + +```yaml +enabled: true +checkPeriod: 30 +watchedCommands: + - helm + - odo + - kubectl +``` + +**Note**: The `watchedCommands` list is **optional** - it's used to **override** auto-detection, not to enable watching. Without this list, ALL user processes are still watched with smart defaults. + +This simple string format forces listed commands to be **non-interactive** - they always prevent idling when running, regardless of whether they're actively doing work. + +### Advanced Configuration - Override Auto-Detection (Optional) + +**⚠️ You probably don't need this section!** The CLI Watcher auto-detects process types correctly in most cases. + +**Only override auto-detection when:** +- Auto-detection misclassifies a specific command +- You need to completely ignore a command that shouldn't be watched +- You have special requirements or are debugging + +**Two escape hatches available:** + +#### 1. **`watchedCommands`** - Fix misclassification (process still watched, mode corrected) +```yaml +watchedCommands: + - name: myBuildTool + interactive: false # Auto-detected as interactive, but it's actually a build → force non-interactive + + - name: myREPL + interactive: true # Auto-detected as work process, but it's interactive → force interactive +``` + +#### 2. **`ignoredCommands`** - Stop watching entirely (process never prevents idling) +```yaml +ignoredCommands: + - weirdSystemDaemon # Has TTY but shouldn't be watched at all + - debugTool # Picked up by auto-detection but irrelevant +``` + +**Warning:** Misconfiguring can break workspace idling: +- Setting `sleep` as `interactive: true` → Long-running tasks interrupted ❌ +- Setting `vim` as `interactive: false` → Idle editor prevents idling forever ❌ +- Over-using `ignoredCommands` → Important work not tracked ❌ + +#### Full example with time settings: + +```yaml +enabled: true +checkPeriod: 30 # How often to check for active processes (default: 60 seconds) +activityWindow: 25m # How long to wait for activity from interactive processes (default: 25m) +gracePeriod: 5m # All processes prevent idling when this young (default: 5m) + +# Optional: Override auto-detection for specific commands +watchedCommands: + # Force long-running commands to always prevent idling (skip auto-detection) + - helm + - kubectl + + # Force interactive CLIs to always check for user input activity + - name: claude + interactive: true # Force interactive (always check for user input) + + # Let auto-detection decide (foreground + TTY read → interactive) + - name: vim + interactive: auto # Auto-detect (same as unconfigured, but explicit) + + # Force non-interactive mode (always prevent idling) + - name: npm + interactive: false # Force non-interactive (always prevent idling) + +# Optional: Completely ignore certain commands +ignoredCommands: + - systemDaemon + - debugHelper +``` + +**Remember**: +- **Unconfigured commands**: Auto-detected with `interactive: auto` behavior after grace period +- **`watchedCommands` entries**: Use your explicit `interactive` setting instead of auto-detection +- **`ignoredCommands` entries**: Never watched, never prevent idling (like `tail`, `watch`, `top`, `htop`) + +## Interactive Mode Options + +The `interactive` field controls how the watcher determines if a process should prevent idling: + +| Mode | Values | Behavior | +|------|--------|----------| +| **Non-interactive** (default) | `false`, `no`, or omit field | Always prevent idling when the process is running. Best for build tools, deployment commands, etc. | +| **Interactive** | `true`, `yes` | Force activity checking. Only prevent idling if process has recent user input (TTY access time). Best for interactive CLIs like editors, REPLs, or AI assistants. | +| **Auto-detect** | `auto` | Detect interactivity by checking if process is foreground AND has read from TTY. If yes → check activity; if no → always prevent idling. | + +## ForceWatch Override Option + +**⚠️ USE WITH EXTREME CAUTION ⚠️** + +The `forceWatch` field allows you to override the always-ignored commands list for specific commands. This should **rarely be needed** as always-ignored commands (`tail`, `watch`, `top`, `htop`) are passive monitoring tools that don't indicate active work. + +| Mode | Values | Behavior | +|------|--------|----------| +| **Respect always-ignored** (default) | `false`, `no`, or omit field | Commands in the always-ignored list will never prevent idling, even if explicitly configured | +| **Override always-ignored** | `true`, `yes` | Force this specific command to be watched, even if it's normally always-ignored | + +### When to Use ForceWatch + +**Valid use cases** (rare): +- Custom scripts named `watch`, `top`, etc. that actually perform work +- Debugging workspace idle behavior with monitoring tools +- Specialized monitoring tools that indicate active development + +**Invalid use cases** (common mistakes): +- Making `tail -f logfile` prevent idling → Logs aren't active work +- Making `top` prevent idling → Process monitoring isn't active work +- Making `watch kubectl get pods` prevent idling → Passive monitoring isn't active work + +### Configuration Example + +```yaml +watchedCommands: + # WRONG: Don't do this for actual monitoring tools + - name: watch + forceWatch: true # ❌ Bad - passive monitoring shouldn't prevent idling + + # VALID: Custom work script that happens to be named 'watch' + - name: watch + interactive: false + forceWatch: true # ✅ OK - custom script that actually does work +``` + +### Accepted Values + +- `true`, `yes` → Override always-ignored list (monitor this command) +- `false`, `no` → Respect always-ignored list (default behavior) +- Omit field → Same as `false` (respect always-ignored list) + +### Warning Messages + +When you configure always-ignored commands without `forceWatch: true`, you'll see: + +``` +WARNING: You configured [watch, top] in watchedCommands, but these are globally excluded (always ignored) +``` + +This usually means you should **remove those commands from your config**, not add `forceWatch: true`. + +### Default Values + +The CLI Watcher uses **smart defaults** that adapt to your workspace idle timeout when possible: + +#### Fixed Defaults (always the same): +- `interactive`: `no` (backward compatible - always prevent idling) +- `maxProcessAge`: `6h` (safety limit to prevent indefinite idling prevention) +- `checkPeriod`: `60` seconds + +#### Adaptive Defaults (calculated from workspace idle timeout): + +When **workspace idle timeout is available** (e.g., 30 minutes): +- `gracePeriod`: Smaller of `5m` or `15%` of idle timeout +- `activityWindow`: `idle timeout - gracePeriod - buffer` + - Buffer is smaller of `5m` or `20%` of idle timeout + - Example: 30m idle → 5m grace → 20m activity window + +When **workspace idle timeout is unavailable or disabled** (`-1`): +- `gracePeriod`: `5m` +- `activityWindow`: `25m` + +#### Minimum Values (enforced even for very short idle timeouts): +- `gracePeriod`: At least `1m` +- `activityWindow`: At least `2m` +- `checkPeriod`: At least `10` seconds + +**User-specified values always take priority** - smart defaults only apply to unspecified fields. + +### How It Works + +1. **User Process Detection**: Only watches processes with TTY that are children of user terminals (filters out system processes automatically) +2. **Always-Ignored Check**: Skips passive monitoring tools (`tail`, `watch`, `top`, `htop`) +3. **Safety Limit**: Processes older than `maxProcessAge` (default 6h) don't prevent idling - protects against hung/forgotten/misconfigured processes +4. **Grace Period**: All user processes < 5 minutes old prevent idling (gives builds time to start) +5. **Interactive Detection** (after grace period): + - **Configured commands**: Use their `interactive` setting + - **Unconfigured commands**: Auto-detect (foreground + has read from TTY → interactive, otherwise → work process) +6. **Activity Checking**: Interactive processes only prevent idling if user input detected within `activityWindow` + +**Note on time formats**: All time settings (`checkPeriod`, `activityWindow`, `gracePeriod`, `maxProcessAge`) accept: +- Duration strings: `6h`, `30m`, `21600s`, `6h30m` +- Plain integers: `21600` (treated as seconds) +- Invalid values log a warning and use the calculated or fixed default + +### Configuration Validation + +The CLI Watcher validates your configuration and warns about potential issues **without changing your specified values**: + +**Warnings you might see**: +``` +WARN: activityWindow (35m) exceeds workspace idle timeout (30m), may not work as expected +WARN: gracePeriod (25m) is very close to workspace idle timeout (30m) +WARN: activityWindow (3m) is less than gracePeriod (5m), interactive processes may not be detected correctly +WARN: checkPeriod (10m) may be too long for activityWindow (15m), activity might not be detected in time +WARN: Workspace idle timeout (8m) is very short, using minimum activity window (2m) +WARN: Both 'checkPeriod' (30s) and deprecated 'checkPeriodSeconds' (45) are set - using 'checkPeriod' value +``` + +These warnings help you identify misconfigurations but **your specified values are always respected**. + +## Activity Detection + +### Interactive Process Detection (`auto` mode) + +A process is considered interactive if: +1. It's in the **foreground process group** of its TTY, AND +2. Either: + - Currently waiting on `read` (from wchan), OR + - Has **ever read from its TTY** (TTY access time is after process start time) + +This detects: +- ✅ **Interactive**: `vim`, `python3` (REPL), `node` (REPL), `less` → Check for recent user input +- ✅ **Work**: `./compile.sh`, `go build`, `npm run build` → Always prevent idling + +### Activity Monitoring + +For interactive processes, recent activity is detected by monitoring **TTY Access Time (Atime)**: +- Atime updates when the TTY is **read from** (user types) +- Atime does NOT update from output (program writes) +- Process prevents idling if Atime is within the `activityWindow` + +**Examples**: +- `claude` actively used → Prevents idling ✅ +- `claude` idle for 30 minutes → Doesn't prevent idling ✅ +- `vim` with active typing → Prevents idling ✅ +- `vim` left open but untouched → Doesn't prevent idling after activity window ✅ +- `go build` running → Always prevents idling ✅ +- Background `node` (VS Code) → Skipped (system process) ✅ + +## Always-Ignored Commands + +The following commands are globally excluded and will NEVER prevent workspace idling, even if explicitly configured or detected as user processes: + +- `tail` - Log file monitoring +- `watch` - Repeated command execution monitoring +- `top` - Process monitoring +- `htop` - Enhanced process monitoring + +These are passive monitoring tools that don't indicate active work. + +## Use Cases + +### Long-Running Deployments (Override Auto-Detection) + +```yaml +# Optional: Force these to always prevent idling (skip auto-detection) +watchedCommands: + - helm + - kubectl + - odo +``` + +These always prevent idling during deployment operations, even if auto-detection would classify them differently. + +### Interactive Development with AI (Custom Activity Window) + +```yaml +activityWindow: 300 # Override global default (25min) to 5 minutes + +# Optional: Force claude to be interactive (it would likely auto-detect correctly anyway) +watchedCommands: + - name: claude + interactive: true +``` + +Workspace stays alive during active Claude Code sessions, but idles if left idle for 5+ minutes. + +**Note**: Without `watchedCommands`, claude would still be watched and likely auto-detected as interactive. This config just makes it explicit and adjusts the activity window. + +### Mixed Workload - Fine-Tuned Control + +```yaml +activityWindow: 1500 # Global default: 25 minutes +gracePeriod: 300 # All processes: 5 minute grace period + +# Override auto-detection for specific commands only when needed +watchedCommands: + # Force deployment tools to always prevent idling + - helm + - kubectl + + # Force claude interactive with custom activity window + - name: claude + interactive: true + + # Let vim auto-detect (would likely work the same without this entry) + - name: vim + interactive: auto + + # Force npm to always prevent idling (in case auto-detection misclassifies) + - name: npm + interactive: false +``` + +**Remember**: All user processes are watched. This config just overrides auto-detection for specific commands. + +## Environment Variables + +- `CLI_WATCHER_CONFIG`: Override config file path +- `PROJECT_SOURCE`: Starting point for upward `.noidle` search +- `PROJECTS_ROOT`: Stop point for upward `.noidle` search (defaults to `/`) + +## Logging + +The watcher logs its activity at INFO level: + +``` +CLI Watcher: Started +CLI Watcher: Config reloaded from /home/user/.noidle +CLI Watcher: Watching ALL user processes with 3 explicit override(s): +CLI Watcher: - helm (mode: non-interactive (always active)) +CLI Watcher: - claude (mode: interactive (activity check)) +CLI Watcher: - vim (mode: auto-detect TTY) +CLI Watcher: Detection period: 30 seconds +CLI Watcher: Activity window: 1500 seconds +CLI Watcher: Grace period: 300 seconds +CLI Watcher: Detected CLI command: helm — reporting activity tick +``` + +**Note**: The log shows configured overrides, but ALL user processes are monitored. + +Use DEBUG level for detailed process scanning: + +``` +CLI Watcher: Process claude (PID 12345) has recent activity +``` + +## Migration Guide + +### From Simple String List + +**Before:** +```yaml +watchedCommands: + - helm + - claude +``` + +**After (to enable activity checking for claude):** +```yaml +activityWindow: 300 # Set global activity window + +watchedCommands: + - helm # Still simple string - always active (or auto-detected after grace period) + - name: claude + interactive: true # Force interactive mode for explicit control +``` + +**Note**: With the new implementation, even unconfigured commands are automatically watched and intelligently classified as interactive or work processes after the grace period. Explicit configuration is only needed to override the auto-detection. + +## Deployment Requirements + +### Filesystem Access Time (atime) Dependency + +**CRITICAL**: Interactive process detection depends on filesystem access time (atime) updates for TTY devices. + +**Problem**: If containers or systems run on filesystems mounted with `noatime` or `relatime`: +- TTY access times won't update when users interact with terminals +- Interactive processes will appear "idle" even when actively used +- Workspaces may shutdown unexpectedly during active terminal sessions + +**Verification**: Check if `/dev/pts` is mounted with atime support: +```bash +# Check mount options for devpts filesystem +mount | grep devpts + +# Should NOT show 'noatime' - example of GOOD output: +devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000) + +# Example of PROBLEMATIC output: +devpts on /dev/pts type devpts (rw,nosuid,noexec,noatime,gid=5,mode=620,ptmxmode=000) +``` + +**Fix for Problematic Systems**: +- **Container environments**: Ensure devpts is mounted without `noatime` +- **Kubernetes**: Use appropriate volume mounts or security policies +- **Manual fix**: Remount devpts with atime support: + ```bash + sudo mount -o remount,relatime /dev/pts + ``` + +**Robust Fallback Detection**: When atime is unavailable or unreliable, the CLI Watcher automatically uses sophisticated alternative detection methods: + +1. **Process State Analysis** - Analyzes if process is sleeping (waiting for input) +2. **Enhanced Wait Channel Analysis** - Detects specific input-waiting syscalls: + - `poll_schedule_timeout` - polling with timeout (interactive pattern) + - `pipe_wait` - waiting on pipe input + - `unix_stream_read_generic` - reading from socket + - `select`, `ep_poll` - event-driven input waiting +3. **File Descriptor Activity** - Monitors recent TTY file descriptor usage + +**Scoring System**: Multiple detection signals are combined with a scoring threshold to reliably identify interactive processes, even without atime support. + +**Automatic Fallback**: No configuration needed - the system automatically detects atime issues and switches to alternative methods with debug logging. + +**Symptoms Indicating Fallback Mode**: +- Debug logs show: "TTY atime for PID X unavailable or unreliable, using fallback detection" +- Debug logs show: "PID X detected as interactive via fallback (score: N, wchan: Y)" + +**Result**: Interactive detection remains highly reliable even on `noatime` filesystems, though atime support is still preferred for optimal performance. + +## Testing + +### Monitoring Activity Ticks + +To watch CLI watcher activity ticks in real-time in a DevWorkspace environment, open a terminal and run: + +```bash +tail -f /checode/entrypoint-logs.txt +``` + +This will show continuous log output including: +- CLI Watcher startup messages +- Config reload events +- Detected CLI commands and activity ticks +- Process scanning debug messages (if `LOG_LEVEL=debug`) + +Example output: +``` +CLI Watcher: Started +CLI Watcher: Config reloaded from /projects/.noidle +CLI Watcher: Watching 3 command(s): +CLI Watcher: - sleep (mode: non-interactive (always active)) +CLI Watcher: - vi (mode: auto-detect TTY, activity window: 120s) +CLI Watcher: Detection period is 15 seconds +CLI Watcher: Detected CLI command: sleep — reporting activity tick +``` + +### Test Scenarios + +#### Available Commands in UBI9 Go-Toolset + +First, verify what commands are available in your dev container: + +```bash +# Check for interactive tools +which vi vim nano less more top python python3 bash sh 2>&1 | grep -v "not found" + +# Check for background/non-interactive tools +which sleep ping curl wget nc watch yes 2>&1 | grep -v "not found" +``` + +Typically available: +- **Interactive**: `vi`, `less`, `more`, `bash`, `sh` +- **Non-interactive**: `sleep`, `ping`, `curl`, `wget`, `yes` + +#### Scenario 1: Non-Interactive Long-Running Commands + +**Test Config** (`/tmp/.noidle.test`): +```yaml +enabled: true +checkPeriod: 15 + +watchedCommands: + - sleep + - ping +``` + +**Test Steps**: +```bash +export CLI_WATCHER_CONFIG=/tmp/.noidle.test + +# Start a long-running background process (no TTY) +sleep 1800 & + +# Watch logs in another terminal +tail -f /checode/entrypoint-logs.txt + +# Expected: "Detected CLI command: sleep — reporting activity tick" every 15s +``` + +**Cleanup**: `pkill sleep` + +#### Scenario 2: Interactive Command with Activity Tracking + +**Test Config** (`/tmp/.noidle.interactive`): +```yaml +enabled: true +checkPeriod: 15 +activityWindow: 120 # 2 minutes for easy testing + +watchedCommands: + - name: vi + interactive: auto +``` + +**Test Steps**: +```bash +export CLI_WATCHER_CONFIG=/tmp/.noidle.interactive + +# Terminal 1: Watch logs +tail -f /checode/entrypoint-logs.txt + +# Terminal 2: Open vi interactively +vi /tmp/testfile.txt + +# Type occasionally and watch for activity ticks +# Stop typing for 3+ minutes - activity ticks should stop +``` + +#### Scenario 3: Auto-Detection of Interactive vs Work Processes + +**Purpose**: Verify that the watcher correctly distinguishes between interactive CLIs (vim, REPLs) and work processes (builds, scripts) without explicit configuration. + +**Test Config** (`/tmp/.noidle.autodetect`): +```yaml +enabled: true +checkPeriod: 15 +activityWindow: 120 # 2 minutes for easy testing +gracePeriod: 1m # Short grace period for faster testing + +# No watchedCommands - everything is auto-detected! +``` + +**Test Steps**: +```bash +export CLI_WATCHER_CONFIG=/tmp/.noidle.autodetect + +# Terminal 1: Watch logs +tail -f /checode/entrypoint-logs.txt + +# Terminal 2: Test interactive process (should require activity) +vi /tmp/test.txt +# Type something, watch for activity tick +# Stop typing for 3+ minutes - ticks should stop + +# Terminal 3: Test work process (should always prevent idling) +sleep 300 +# Should see activity ticks every 15 seconds even without user interaction +``` + +**Expected behavior**: +- `vi` detected as **interactive** (foreground + reads from TTY) → Only ticks when typing +- `sleep` detected as **work process** (not interactive) → Always ticks while running +- Grace period (1min): Both prevent idling immediately when started + +**Debugging**: Set `LOG_LEVEL=debug` to see detailed detection: +``` +CLI Watcher: Process vi (PID 12345) auto-detected as interactive +CLI Watcher: Process vi (PID 12345) has recent activity +CLI Watcher: Process sleep (PID 12346) auto-detected as work process +``` + +#### Scenario 4: Excluded Commands (Negative Test) + +**Test Config** (`/tmp/.noidle.exclusion`): +```yaml +enabled: true +checkPeriod: 10 + +watchedCommands: + - tail # Globally excluded + - sleep +``` + +**Expected**: Logs show `tail` is skipped: +``` +CLI Watcher: WARNING: You configured [tail] in watchedCommands, but these are globally excluded (always ignored) +CLI Watcher: Watching ALL user processes with 1 explicit override(s): +CLI Watcher: - sleep (mode: non-interactive (always active)) +``` + +### Debugging + +Enable debug logging for detailed process scanning: + +```bash +export LOG_LEVEL=debug +``` + +This shows: +``` +CLI Watcher: Process vi (PID 12345) has recent activity +CLI Watcher: Process vi (PID 12345) found but no recent activity +``` + +### Quick Test Setup + +Create a test configuration file: + +```yaml +# /tmp/.noidle.quicktest +enabled: true +checkPeriod: 10 +activityWindow: 120 # 2 minutes for easy testing +watchedCommands: + - sleep + - name: vi + interactive: auto +``` + +**Testing Steps**: + +1. **Stop existing server** (use devfile command: `stop-exec-server`) + +2. **Start server with test config**: + ```bash + export CLI_WATCHER_CONFIG=/tmp/.noidle.quicktest + ``` + Then run devfile command: `start-exec-server` + +3. **Monitor activity ticks**: + ```bash + tail -f /checode/entrypoint-logs.txt + ``` + +4. **Start test processes**: + ```bash + # Terminal 1: Non-interactive (always active) + sleep 600 & + + # Terminal 2: Interactive (activity tracked) + vi /tmp/test.txt + ``` + +5. **Watch logs** - you should see: + ``` + CLI Watcher: Config reloaded from /tmp/.noidle.quicktest + CLI Watcher: Detected CLI command: sleep — reporting activity tick + ``` + +6. **Cleanup**: Stop the server using `stop-exec-server` command + +### Expected Test Results + +| Command | Mode | Has TTY? | Active I/O? | Prevents Idling? | +|---------|------|----------|-------------|------------------| +| `sleep 3600 &` | default (no) | No | N/A | ✅ Always | +| `vi file.txt` (typing) | auto | Yes | Yes | ✅ Yes | +| `vi file.txt` (idle) | auto | Yes | No | ❌ No (after window) | +| `tail -f file` | any | any | any | ❌ Never (excluded) | + +### Developer Testing + +For contributors working on the CLI Watcher code: + +**Run unit tests:** +```bash +go test ./timeout -v +``` + +**Check test coverage:** +```bash +go test ./timeout -cover +``` + +**Note on test coverage:** +- **Unit tests cover pure functions** (parsing, configuration, validation, defaults, YAML unmarshaling) +- **Core detection logic is untested** (process tree walking, TTY analysis, interactive process detection, `isWatchedProcessRunning`, `isUserInitiatedProcess`) + +**Why core detection logic requires manual testing:** +- Requires real `/proc` filesystem (not available in standard Go test environment) +- Needs multiple process scenarios (shells, interactive CLIs, work processes, TTY states) +- Depends on actual system process behavior and file descriptor states + +**For detection logic verification**: Use the manual test scenarios described above with real processes in a containerized development environment. diff --git a/timeout/cli-watcher.go b/timeout/cli-watcher.go index ca8222a20..e40034f38 100644 --- a/timeout/cli-watcher.go +++ b/timeout/cli-watcher.go @@ -1,5 +1,5 @@ // -// Copyright (c) 2025 Red Hat, Inc. +// Copyright (c) 2025-2026 Red Hat, Inc. // This program and the accompanying materials are made // available under the terms of the Eclipse Public License 2.0 // which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -13,63 +13,289 @@ package timeout import ( + "encoding/binary" "fmt" + "io" "os" "path/filepath" + "runtime" "slices" + "strconv" "strings" + "sync" + "syscall" "time" "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" ) +type InteractiveMode string + +const ( + InteractiveModeAuto InteractiveMode = "auto" + InteractiveModeTrue InteractiveMode = "true" + InteractiveModeFalse InteractiveMode = "false" + InteractiveModeYes InteractiveMode = "yes" + InteractiveModeNo InteractiveMode = "no" +) + +type ForceWatchMode string + +const ( + ForceWatchModeTrue ForceWatchMode = "true" + ForceWatchModeFalse ForceWatchMode = "false" + ForceWatchModeYes ForceWatchMode = "yes" + ForceWatchModeNo ForceWatchMode = "no" +) + +// isForceWatchEnabled checks if ForceWatchMode is enabled (true/yes) +func (f ForceWatchMode) isEnabled() bool { + return f == ForceWatchModeTrue || f == ForceWatchModeYes +} + +const ( + DefaultInteractiveMode InteractiveMode = InteractiveModeNo // Backward compatible: always prevent idling + DefaultCheckPeriod = 60 // Default check period: 60 seconds + DefaultActivityWindow = 25 * time.Minute // Default activity window: 25 minutes (fallback when idle timeout unavailable) + DefaultGracePeriod = 5 * time.Minute // Default grace period: 5 minutes + DefaultMaxProcessAge = 6 * time.Hour // Default max process age: 6 hours - safety limit + + MinActivityWindow = 2 * time.Minute // Minimum activity window for very short idle timeouts + MinGracePeriod = 1 * time.Minute // Minimum grace period + MinCheckPeriod = 10 // Minimum check period in seconds + SafetyBufferDuration = 5 * time.Minute // Safety buffer between activity window and idle timeout + SafetyBufferPercent = 0.2 // Or 20% of idle timeout, whichever is smaller +) + +// ttyCache holds cached TTY device information to reduce redundant filesystem operations +type ttyCache struct { + path string // TTY device path (e.g., "/dev/pts/1") + atime time.Time // Last access time + cachedAt time.Time // When this was cached + valid bool // Whether the TTY path resolution was successful +} + +// TTY cache with short TTL to avoid stale data across scan cycles +var ( + ttyPathCache = make(map[string]*ttyCache) + ttyPathCacheMutex sync.RWMutex +) +const ( + ttyCacheDuration = 2 * time.Second + ttyCacheMaxSize = 1000 // Maximum entries to prevent unbounded growth + ttyCacheCleanupAt = 800 // Trigger cleanup when reaching this size +) + +// cleanupTTYCache removes expired and dead PID entries from the cache +// MUST be called with ttyPathCacheMutex write lock held +func cleanupTTYCache() { + now := time.Now() + for pid, entry := range ttyPathCache { + // Remove if expired + if now.Sub(entry.cachedAt) >= ttyCacheDuration { + delete(ttyPathCache, pid) + continue + } + + // Remove if PID no longer exists (quick check without filesystem calls) + if _, err := os.Stat(filepath.Join("/proc", pid)); os.IsNotExist(err) { + delete(ttyPathCache, pid) + } + } +} + +type WatchedCommand struct { + Name string `yaml:"name"` + Interactive InteractiveMode `yaml:"interactive"` + ForceWatch ForceWatchMode `yaml:"forceWatch"` +} + +// UnmarshalYAML allows WatchedCommand to be unmarshaled from either a string or an object +func (w *WatchedCommand) UnmarshalYAML(unmarshal func(any) error) error { + // Try to unmarshal as a string first (backward compatible) + var str string + if err := unmarshal(&str); err == nil { + w.Name = str + w.Interactive = DefaultInteractiveMode + return nil + } + + // Otherwise, unmarshal as a struct + type rawWatchedCommand WatchedCommand + var raw rawWatchedCommand + if err := unmarshal(&raw); err != nil { + return err + } + + *w = WatchedCommand(raw) + return nil +} + type cliWatcherConfig struct { - WatchedCommands []string `yaml:"watchedCommands"` - IgnoredCommands []string `json:"-"` - CheckPeriodSeconds int `yaml:"checkPeriodSeconds"` - Enabled bool `yaml:"enabled"` - _lastModTime time.Time `json:"-"` + WatchedCommands []WatchedCommand `yaml:"watchedCommands"` + IgnoredCommands []string `yaml:"ignoredCommands" json:"-"` + CheckPeriodSeconds int `yaml:"checkPeriodSeconds"` // Deprecated: use CheckPeriod instead (kept for backward compatibility) + CheckPeriod string `yaml:"checkPeriod"` + ActivityWindow string `yaml:"activityWindow"` + GracePeriod string `yaml:"gracePeriod"` + MaxProcessAge string `yaml:"maxProcessAge"` + Enabled bool `yaml:"enabled"` + _lastModTime time.Time `json:"-"` + _checkPeriodParsed time.Duration `json:"-"` + _activityWindowParsed time.Duration `json:"-"` + _gracePeriodParsed time.Duration `json:"-"` + _maxProcessAgeParsed time.Duration `json:"-"` } // Watcher monitors CLI processes and invokes a tick callback when active ones are found type cliWatcher struct { + mu sync.Mutex // Protects config, warnedMissingConfig, started config *cliWatcherConfig warnedMissingConfig bool stopChan chan struct{} + stopOnce sync.Once // Ensures stopChan is only closed once started bool - tickFunc func() + tickFunc func() // Immutable after construction (safe to read without lock) + myPID string // Immutable after construction (safe to read without lock) + idleTimeout time.Duration // Immutable after construction (safe to read without lock) +} + +// Commands that should NEVER prevent workspace idling (passive monitoring tools) +var alwaysIgnoredCommands = []string{"tail", "watch", "top", "htop"} + +// systemClockTicks is the number of clock ticks per second (sysconf(_SC_CLK_TCK)) +// Detected lazily on first use from /proc/self/auxv with platform-dependent fallback +var ( + systemClockTicks int64 + systemBootTime time.Time + systemInitOnce sync.Once +) + +// ensureSystemInfoInitialized lazily initializes system clock ticks and boot time +// Uses sync.Once to ensure initialization happens exactly once, thread-safe +// Only called when needed (avoids /proc reads on non-Linux systems or when CLI watcher unused) +func ensureSystemInfoInitialized() { + systemInitOnce.Do(func() { + systemClockTicks = detectClockTicks() + if systemClockTicks <= 0 { + logrus.Warnf("CLI Watcher: Failed to detect system clock ticks, using platform default") + systemClockTicks = getPlatformDefaultClockTicks() + } + logrus.Debugf("CLI Watcher: System clock ticks: %d", systemClockTicks) + + systemBootTime = detectSystemBootTime() + if systemBootTime.IsZero() { + logrus.Warnf("CLI Watcher: Failed to detect system boot time") + } else { + logrus.Debugf("CLI Watcher: System boot time: %s", systemBootTime.Format(time.RFC3339)) + } + }) } -// CLIs that should never prevent idling -var excludedCommands = []string{"tail"} +// detectClockTicks reads AT_CLKTCK from /proc/self/auxv +func detectClockTicks() int64 { + const AT_CLKTCK = 17 // Auxiliary vector entry for clock ticks + + auxv, err := os.ReadFile("/proc/self/auxv") + if err != nil { + return 0 + } + + // auxv is a series of (type, value) pairs as uintptr (native word size) + // On 64-bit: 8 bytes per value, on 32-bit: 4 bytes per value + // Use NativeEndian to support both little-endian (x86, ARM) and big-endian (s390x) platforms + wordSize := strconv.IntSize / 8 // IntSize is 32 or 64 bits, convert to bytes + + for i := 0; i+wordSize*2 <= len(auxv); i += wordSize * 2 { + var auxType, auxVal uint64 + + if wordSize == 8 { + // 64-bit: need 16 bytes total (8 + 8) + if i+16 > len(auxv) { + break + } + auxType = binary.NativeEndian.Uint64(auxv[i : i+8]) + auxVal = binary.NativeEndian.Uint64(auxv[i+8 : i+16]) + } else { + // 32-bit: need 8 bytes total (4 + 4) + if i+8 > len(auxv) { + break + } + auxType = uint64(binary.NativeEndian.Uint32(auxv[i : i+4])) + auxVal = uint64(binary.NativeEndian.Uint32(auxv[i+4 : i+8])) + } + + if auxType == AT_CLKTCK { + // Sanity check: clock ticks should be in reasonable range + // Typical values: 100 (x86), 250 (ARM), 1000 (rare) + // Reject values outside [1, 10000] as corrupted data + if auxVal >= 1 && auxVal <= 10000 { + return int64(auxVal) + } + // Invalid value detected, return 0 to trigger platform default + logrus.Warnf("CLI Watcher: Invalid AT_CLKTCK value %d from auxv (expected 1-10000), using platform default", auxVal) + return 0 + } + } + + return 0 +} + +// getPlatformDefaultClockTicks returns platform-specific default clock ticks +// This is a FALLBACK used only if /proc/self/auxv detection fails (very rare) +// Most Linux systems use 100 ticks/sec (x86, RISC-V, PowerPC, MIPS, s390x) +// ARM is the main exception with 250 ticks/sec +func getPlatformDefaultClockTicks() int64 { + switch runtime.GOARCH { + case "arm", "arm64": + return 250 // ARM systems typically use 250 + case "amd64", "386": + return 100 // x86/x86_64 systems typically use 100 + default: + // RISC-V, PowerPC, MIPS, s390x, and most others also use 100 + return 100 + } +} // New creates a new Watcher with the given config and tick callback -func NewCliWatcher(tickFunc func()) *cliWatcher { +func NewCliWatcher(tickFunc func(), idleTimeout time.Duration) *cliWatcher { + if tickFunc == nil { + logrus.Warnf("CLI Watcher: Created with nil tick callback - activity will not be reported") + } return &cliWatcher{ - stopChan: make(chan struct{}), - tickFunc: tickFunc, + stopChan: make(chan struct{}), + tickFunc: tickFunc, + myPID: fmt.Sprintf("%d", os.Getpid()), + idleTimeout: idleTimeout, } } // Start begins the watcher loop func (w *cliWatcher) Start() { + w.mu.Lock() if w.started { + w.mu.Unlock() return } w.started = true + w.mu.Unlock() go func() { var err error + w.mu.Lock() w.config, err = w.loadConfig(getConfigPath(), w.config) + w.mu.Unlock() if err != nil { logrus.Errorf("CLI Watcher: Failed to reload config: %v", err) } - chkPeriod := 60 - if w.config != nil { - chkPeriod = w.config.CheckPeriodSeconds + w.mu.Lock() + chkPeriod := DefaultCheckPeriod + if w.config != nil && w.config._checkPeriodParsed > 0 { + chkPeriod = int(w.config._checkPeriodParsed.Seconds()) } + w.mu.Unlock() ticker := time.NewTicker(time.Duration(chkPeriod) * time.Second) defer ticker.Stop() @@ -83,32 +309,38 @@ func (w *cliWatcher) Start() { case <-ticker.C: oldPeriod := chkPeriod - // Reload config + // Reload config (protected) + w.mu.Lock() w.config, err = w.loadConfig(getConfigPath(), w.config) + configSnapshot := w.config // Take snapshot for use outside lock + w.mu.Unlock() if err != nil { logrus.Errorf("CLI Watcher: Failed to reload config: %v", err) } - if w.config == nil || !w.config.Enabled { - if chkPeriod != 60 { - logrus.Infof("CLI Watcher: Config was removed or disabled — resetting check period to default (60s)") - chkPeriod = 60 + if configSnapshot == nil || !configSnapshot.Enabled { + if chkPeriod != DefaultCheckPeriod { + logrus.Infof("CLI Watcher: Config was removed or disabled — resetting check period to default (%ds)", DefaultCheckPeriod) + chkPeriod = DefaultCheckPeriod ticker.Stop() + // Recreate ticker with new period ticker = time.NewTicker(time.Duration(chkPeriod) * time.Second) } continue } - if w.config.CheckPeriodSeconds > 0 && w.config.CheckPeriodSeconds != oldPeriod { - logrus.Infof("CLI Watcher: Detected new check period: %d seconds (was %d), restarting ticker", w.config.CheckPeriodSeconds, oldPeriod) - chkPeriod = w.config.CheckPeriodSeconds + newPeriod := int(configSnapshot._checkPeriodParsed.Seconds()) + if newPeriod > 0 && newPeriod != oldPeriod { + logrus.Infof("CLI Watcher: Detected new check period: %d seconds (was %d), restarting ticker", newPeriod, oldPeriod) + chkPeriod = newPeriod ticker.Stop() + // Recreate ticker with new period ticker = time.NewTicker(time.Duration(chkPeriod) * time.Second) } - found, name := isWatchedProcessRunning(w.config.WatchedCommands) + found, name := isWatchedProcessRunning(configSnapshot, w.myPID) if found { - logrus.Infof("CLI Watcher: Detected CLI command: %s — reporting activity tick", name) + logrus.Debugf("CLI Watcher: Detected CLI command: %s — reporting activity tick", name) if w.tickFunc != nil { w.tickFunc() } @@ -122,15 +354,30 @@ func (w *cliWatcher) Start() { // Stop terminates the watcher loop func (w *cliWatcher) Stop() { - if !w.started { + w.mu.Lock() + wasStarted := w.started + if wasStarted { + w.started = false + } + w.mu.Unlock() + + if !wasStarted { return } - close(w.stopChan) - w.started = false + + // Use sync.Once to ensure channel is only closed once, even if Stop() called concurrently + w.stopOnce.Do(func() { + close(w.stopChan) + }) } -// Scans /proc to check if any watched process is running -func isWatchedProcessRunning(watched []string) (bool, string) { +// Scans /proc to check if any watched process is running and active +func isWatchedProcessRunning(config *cliWatcherConfig, myPID string) (bool, string) { + // Handle nil config + if config == nil { + return false, "" + } + procEntries, err := os.ReadDir("/proc") if err != nil { logrus.Warnf("CLI Watcher: Cannot read /proc: %v", err) @@ -143,36 +390,102 @@ func isWatchedProcessRunning(watched []string) (bool, string) { } pid := entry.Name() - if pid == "1" { // Skip PID 1 (main container process) + if pid == "1" || pid == myPID { // Skip PID 1 and ourselves continue } - cmdlinePath := filepath.Join("/proc", pid, "cmdline") - data, err := os.ReadFile(cmdlinePath) - if err != nil || len(data) == 0 { + // FIRST CHECK: Only process user-initiated work (has TTY + main user process exists) + if !isUserInitiatedProcess(pid) { continue } - cmdParts := strings.Split(string(data), "\x00") - if len(cmdParts) == 0 { + // Get command name from /proc/[pid]/comm (shows invoked command name, not underlying binary) + // This handles multicall binaries like coreutils where cmdline shows the actual binary + // but comm shows the invoked command (e.g., "tail" not "coreutils") + commPath := filepath.Join("/proc", pid, "comm") + commData, err := os.ReadFile(commPath) + if err != nil { continue } - // Match against all command line parts, not just the first - for _, part := range cmdParts { - partName := filepath.Base(part) - for _, keyword := range watched { - if partName == keyword { - return true, keyword - } + cmdName := strings.TrimSpace(string(commData)) + if cmdName == "" { + continue + } + + // STEP 1: Check if command is in always-ignored list OR config ignored list + if slices.Contains(alwaysIgnoredCommands, cmdName) { + logrus.Debugf("CLI Watcher: Process %s (PID %s) is in always-ignored list, skipping", cmdName, pid) + continue + } + if slices.Contains(config.IgnoredCommands, cmdName) { + logrus.Debugf("CLI Watcher: Process %s (PID %s) is in config ignored list, skipping", cmdName, pid) + continue + } + + // STEP 2: Check if command is explicitly configured + var configuredCmd *WatchedCommand + for i := range config.WatchedCommands { + if config.WatchedCommands[i].Name == cmdName { + configuredCmd = &config.WatchedCommands[i] + break + } + } + + // STEP 3: Safety check - don't prevent idling for processes older than maxProcessAge + processAge := getProcessAge(pid) + maxAge := config._maxProcessAgeParsed + if maxAge <= 0 { + maxAge = DefaultMaxProcessAge + } + if processAge > 0 && processAge > maxAge { + logrus.Warnf("CLI Watcher: Process %s (PID %s) exceeds max age (%v, limit: %v), no longer preventing idling (safety limit)", cmdName, pid, processAge, maxAge) + continue + } + + // STEP 4: Grace period - all young processes prevent idling + gracePeriod := config._gracePeriodParsed + if gracePeriod <= 0 { + gracePeriod = DefaultGracePeriod + } + if processAge == 0 { + // Can't determine age (getProcessStartTime failed) - give benefit of doubt with grace period + logrus.Debugf("CLI Watcher: Process %s (PID %s) age unknown, applying grace period protection", cmdName, pid) + return true, cmdName + } + if processAge < gracePeriod { + logrus.Debugf("CLI Watcher: Process %s (PID %s) in grace period (age: %v), preventing idling", cmdName, pid, processAge) + return true, cmdName + } + + // STEP 5: Apply policy based on configuration or defaults + var mode InteractiveMode + var policySource string + if configuredCmd != nil { + mode = configuredCmd.Interactive + if mode == "" { + mode = DefaultInteractiveMode } + policySource = "configured" + } else { + mode = InteractiveModeAuto // Auto-detect for unconfigured commands + policySource = "default" } + + if !applyPolicy(pid, cmdName, mode, config._activityWindowParsed, policySource) { + continue + } + + return true, cmdName } return false, "" } func isNumeric(s string) bool { + if len(s) == 0 { + return false + } for _, c := range s { if c < '0' || c > '9' { return false @@ -181,6 +494,594 @@ func isNumeric(s string) bool { return true } +// procStat holds parsed fields from /proc/[pid]/stat +type procStat struct { + ppid string // Parent PID (field 4) + pgrp int // Process group ID (field 5) + tpgid int // Foreground process group of TTY (field 8) + startTicks int64 // Process start time in clock ticks (field 22) +} + +// parseProcStat reads and parses /proc/[pid]/stat once, returning all needed fields +// This avoids multiple reads of the same file for different fields +// +// Note: During detection, the same PID's stat file may be read 2-3 times via different +// callers (getProcessAge, isInForegroundProcessGroup, hasEverReadFromTTY). Caching would +// require threading *procStat through many function layers. Current design prioritizes +// code clarity over the small perf cost (2-3 file reads per detected process per scan). +func parseProcStat(pid string) (*procStat, error) { + statPath := filepath.Join("/proc", pid, "stat") + + // Add reasonable file size limit to prevent DoS via huge stat files + const maxStatFileSize = 4096 // 4KB should be more than enough for any real stat file + file, err := os.Open(statPath) + if err != nil { + return nil, err + } + defer file.Close() + + // Read with size limit + data := make([]byte, maxStatFileSize) + n, err := file.Read(data) + if err != nil && err != io.EOF { + return nil, err + } + data = data[:n] // Truncate to actual read size + + str := string(data) + // Parse /proc/[pid]/stat - format: pid (comm) state ppid pgrp session tty_nr tpgid ... + // Need to handle process names with spaces/parens + lastParen := strings.LastIndex(str, ")") + if lastParen == -1 { + return nil, fmt.Errorf("invalid stat format: no closing paren") + } + + // Fields after ')': state ppid pgrp session tty_nr tpgid flags ... starttime + fields := strings.Fields(str[lastParen+1:]) + + // Add reasonable field count limit (normal stat files have ~50 fields) + const maxStatFields = 100 + if len(fields) > maxStatFields { + return nil, fmt.Errorf("stat file has too many fields (%d > %d)", len(fields), maxStatFields) + } + + if len(fields) < 22 { + return nil, fmt.Errorf("insufficient fields in stat: %d", len(fields)) + } + + stat := &procStat{} + + // Field 4 (index 1): ppid + stat.ppid = fields[1] + + // Field 5 (index 2): pgrp + if n, err := fmt.Sscanf(fields[2], "%d", &stat.pgrp); err != nil || n != 1 { + return nil, fmt.Errorf("failed to parse pgrp") + } + + // Field 8 (index 5): tpgid (foreground process group) + if n, err := fmt.Sscanf(fields[5], "%d", &stat.tpgid); err != nil || n != 1 { + return nil, fmt.Errorf("failed to parse tpgid") + } + + // Field 22 (index 19): starttime (clock ticks since boot) + // Validate > 0: starttime=0 is invalid (would mean process started at boot time), + // and negative values indicate corrupted /proc data + if n, err := fmt.Sscanf(fields[19], "%d", &stat.startTicks); err != nil || n != 1 || stat.startTicks <= 0 { + return nil, fmt.Errorf("failed to parse starttime") + } + + return stat, nil +} + +// applyPolicy applies the interactive policy for a command +// Returns true if process should prevent idling, false otherwise +// Unified function handling both configured and default policies +func applyPolicy(pid, cmdName string, mode InteractiveMode, activityWindow time.Duration, policySource string) bool { + // Determine if process is interactive + var checkActivity bool + + switch mode { + case InteractiveModeAuto: + // Auto-detect: use foreground + TTY read analysis + checkActivity = isInteractiveProcess(pid) + if checkActivity { + logrus.Debugf("CLI Watcher: Process %s (PID %s) auto-detected as interactive (%s policy)", cmdName, pid, policySource) + } else { + logrus.Debugf("CLI Watcher: Process %s (PID %s) auto-detected as work process (%s policy)", cmdName, pid, policySource) + } + + case InteractiveModeTrue, InteractiveModeYes: + // Force interactive mode + checkActivity = true + logrus.Debugf("CLI Watcher: Process %s (PID %s) forced interactive (%s policy)", cmdName, pid, policySource) + + case InteractiveModeFalse, InteractiveModeNo: + // Force non-interactive (work) mode + checkActivity = false + logrus.Debugf("CLI Watcher: Process %s (PID %s) forced non-interactive (%s policy)", cmdName, pid, policySource) + } + + // If interactive, check for recent activity + if checkActivity { + if !hasRecentActivity(activityWindow, pid) { + logrus.Debugf("CLI Watcher: Process %s (PID %s) is interactive but no recent activity (%s policy)", cmdName, pid, policySource) + return false + } + logrus.Debugf("CLI Watcher: Process %s (PID %s) is interactive with recent activity (%s policy)", cmdName, pid, policySource) + } + + return true +} + +// getParentPID returns the parent PID of a given process +// Returns empty string if process no longer exists or /proc read fails +func getParentPID(pid string) string { + stat, err := parseProcStat(pid) + if err != nil { + // Normal: process may have exited between scan and read + return "" + } + return stat.ppid +} + +// getMainUserProcess walks up the process tree to find the first parent without TTY +// Returns the main user process PID and true if found, empty string and false otherwise +// Protected against infinite loops with max depth limit and cycle detection +func getMainUserProcess(pid string) (string, bool) { + // Maximum parent chain depth to prevent infinite loops + // Rationale: Typical process chains are 2-5 deep (terminal → shell → command) + // Even pathological cases (deeply nested tmux/screen/containers) rarely exceed 20 + // 64 provides ample headroom while preventing runaway traversal on corrupted /proc + const maxDepth = 64 + current := pid + visited := make(map[string]bool, maxDepth) // Pre-allocate for worst-case to avoid reallocations + + for depth := 0; depth < maxDepth; depth++ { + // Mark current as visited BEFORE processing to detect cycles early + if visited[current] { + logrus.Warnf("CLI Watcher: Detected cycle in process tree at PID %s", current) + return "", false + } + visited[current] = true + + parent := getParentPID(current) + + // Check for self-parent (corruption) + if parent == current { + logrus.Warnf("CLI Watcher: Process %s claims to be its own parent (corrupted /proc)", current) + return "", false + } + + // Reached top of process tree + if parent == "" || parent == "0" || parent == "1" { + return "", false // Reached top without finding main user process + } + + // Check if parent has NO TTY - that's our main user process + if !processHasTTY(parent) { + return parent, true + } + + current = parent + } + + // Max depth exceeded - highly unlikely to be a user terminal process + logrus.Warnf("CLI Watcher: Max depth (%d) exceeded walking process tree from PID %s", maxDepth, pid) + return "", false +} + +// isUserInitiatedProcess checks if a process is user-initiated by verifying: +// 1. It has a TTY +// 2. Its parent also has TTY (filters out shells themselves - bash/sh/zsh parent has no TTY) +// 3. Walking up the parent chain leads to a process without TTY (main user process) +func isUserInitiatedProcess(pid string) bool { + // Must have TTY + if !processHasTTY(pid) { + return false + } + + // Parent must exist and not be init process (PID 1) or kernel (PID 0) + parent := getParentPID(pid) + if parent == "" || parent == "0" || parent == "1" { + return false + } + + // Parent must also have TTY (filters out shells - shell has TTY but parent doesn't) + if !processHasTTY(parent) { + return false + } + + // Find main user process (first parent without TTY in the chain) + _, found := getMainUserProcess(pid) + return found +} + +// getProcessStartTime returns when the process started +// Returns zero time if process no longer exists or system info unavailable +func getProcessStartTime(pid string) time.Time { + // Ensure system info is initialized (lazy init on first call) + ensureSystemInfoInitialized() + + stat, err := parseProcStat(pid) + if err != nil { + // Normal: process may have exited between scan and read + return time.Time{} + } + + // Use cached system boot time (initialized lazily) + bootTime := systemBootTime + if bootTime.IsZero() { + // Rare: system boot time detection failed + return time.Time{} + } + + // Use detected clock ticks (from /proc/self/auxv or platform default) + clockTicks := systemClockTicks + if clockTicks <= 0 { + clockTicks = 100 // Ultimate fallback + } + + // Calculate process start time avoiding integer overflow + // Use floating point to prevent overflow: (startTicks * 1000) could overflow for long-running processes + // Formula: bootTime + (startTicks / clockTicks) seconds + startTimeMs := int64(float64(stat.startTicks) * 1000.0 / float64(clockTicks)) + startTime := bootTime.Add(time.Duration(startTimeMs) * time.Millisecond) + return startTime +} + +// detectSystemBootTime reads boot time from /proc/stat +// Called lazily via ensureSystemInfoInitialized(), cached in systemBootTime global +func detectSystemBootTime() time.Time { + data, err := os.ReadFile("/proc/stat") + if err != nil { + return time.Time{} + } + + for _, line := range strings.Split(string(data), "\n") { + if strings.HasPrefix(line, "btime ") { + var bootSec int64 + if n, err := fmt.Sscanf(line, "btime %d", &bootSec); err != nil || n != 1 || bootSec <= 0 { + return time.Time{} + } + return time.Unix(bootSec, 0) + } + } + return time.Time{} +} + +// getProcessAge returns how long the process has been running +// Returns 0 if process start time cannot be determined or if system clock skew results in negative age +func getProcessAge(pid string) time.Duration { + startTime := getProcessStartTime(pid) + if startTime.IsZero() { + return 0 + } + age := time.Since(startTime) + // Handle clock skew: if system clock was set backward after process started, + // treat as age 0 (just started) to ensure grace period protection + if age < 0 { + return 0 + } + return age +} + +// isInForegroundProcessGroup checks if process is in the foreground process group of its TTY +func isInForegroundProcessGroup(pid string) bool { + stat, err := parseProcStat(pid) + if err != nil { + return false + } + + // If tpgid == -1, no foreground process group + // If pgrp == tpgid, this process is in foreground + return stat.tpgid > 0 && stat.pgrp == stat.tpgid +} + +// getWaitChannel returns what the process is waiting on +func getWaitChannel(pid string) string { + wchanPath := filepath.Join("/proc", pid, "wchan") + data, err := os.ReadFile(wchanPath) + if err != nil { + return "" + } + return strings.TrimSpace(string(data)) +} + +// getCachedTTYInfo gets TTY device path and access time with caching to reduce filesystem operations +func getCachedTTYInfo(pid string) (string, time.Time, bool) { + // Check cache first (read lock) + ttyPathCacheMutex.RLock() + cached, exists := ttyPathCache[pid] + if exists && time.Since(cached.cachedAt) < ttyCacheDuration { + // Cache hit and still valid + if !cached.valid { + ttyPathCacheMutex.RUnlock() + return "", time.Time{}, false + } + // Use cached path to get fresh atime (filesystem call outside lock) + cachedPath := cached.path + ttyPathCacheMutex.RUnlock() + + // Update access time from cached path + var stat syscall.Stat_t + if err := syscall.Stat(cachedPath, &stat); err != nil { + return cachedPath, cached.atime, false // Use stale atime if stat fails + } + return cachedPath, time.Unix(stat.Atim.Sec, stat.Atim.Nsec), true + } + ttyPathCacheMutex.RUnlock() + + // Cache miss or expired - resolve TTY path (expensive operations outside locks) + fd0Path := filepath.Join("/proc", pid, "fd", "0") + target, err := os.Readlink(fd0Path) + if err != nil { + // Cache failure result (write lock) + ttyPathCacheMutex.Lock() + ttyPathCache[pid] = &ttyCache{cachedAt: time.Now(), valid: false} + ttyPathCacheMutex.Unlock() + return "", time.Time{}, false + } + + if !strings.HasPrefix(target, "/dev/pts/") && !strings.HasPrefix(target, "/dev/tty") { + // Cache invalid TTY result (write lock) + ttyPathCacheMutex.Lock() + ttyPathCache[pid] = &ttyCache{cachedAt: time.Now(), valid: false} + ttyPathCacheMutex.Unlock() + return "", time.Time{}, false + } + + // Get access time + var stat syscall.Stat_t + if err := syscall.Stat(target, &stat); err != nil { + // Cache path but failed stat (write lock) + ttyPathCacheMutex.Lock() + ttyPathCache[pid] = &ttyCache{path: target, cachedAt: time.Now(), valid: false} + ttyPathCacheMutex.Unlock() + return target, time.Time{}, false + } + + atime := time.Unix(stat.Atim.Sec, stat.Atim.Nsec) + + // Cache successful result (write lock) + ttyPathCacheMutex.Lock() + + // Trigger cleanup if cache is getting large + if len(ttyPathCache) >= ttyCacheCleanupAt { + cleanupTTYCache() + } + + // Enforce maximum cache size (fallback if cleanup didn't free enough space) + if len(ttyPathCache) >= ttyCacheMaxSize { + // Remove oldest entries until we're comfortably under the cleanup threshold + targetSize := ttyCacheCleanupAt - 50 // Leave some headroom + for len(ttyPathCache) > targetSize { + // Find and remove the oldest entry + oldestTime := time.Now() + var oldestPID string + for cachePID, entry := range ttyPathCache { + if entry.cachedAt.Before(oldestTime) { + oldestTime = entry.cachedAt + oldestPID = cachePID + } + } + if oldestPID != "" { + delete(ttyPathCache, oldestPID) + } else { + // Safety break - shouldn't happen but prevents infinite loop + break + } + } + } + + ttyPathCache[pid] = &ttyCache{ + path: target, + atime: atime, + cachedAt: time.Now(), + valid: true, + } + ttyPathCacheMutex.Unlock() + + return target, atime, true +} + +// getTTYAtime returns the access time of the process's TTY +func getTTYAtime(pid string) time.Time { + _, atime, valid := getCachedTTYInfo(pid) + if !valid { + return time.Time{} + } + return atime +} + +// hasEverReadFromTTY checks if the process has ever read from its TTY +// NOTE: This depends on filesystem access time (atime) being updated. +// On filesystems mounted with 'noatime' or 'relatime', this may not work reliably. +func hasEverReadFromTTY(pid string) bool { + startTime := getProcessStartTime(pid) + if startTime.IsZero() { + return false + } + + ttyAtime := getTTYAtime(pid) + if ttyAtime.IsZero() { + return false + } + + // If TTY was accessed after process started, it has read input + if ttyAtime.After(startTime) { + return true + } + + // Atime failed - fall back to alternative detection methods + logrus.Debugf("CLI Watcher: TTY atime for PID %s unavailable or unreliable, using fallback detection", pid) + return hasInteractiveBehaviorFallback(pid) +} + +// hasInteractiveBehaviorFallback uses alternative methods when atime is unavailable +// Combines: process state analysis, enhanced wchan analysis, and FD analysis +func hasInteractiveBehaviorFallback(pid string) bool { + score := 0 + + // Method #1: Process State Analysis + // Interactive processes are typically sleeping (waiting for input) + if state := getProcessState(pid); state == "S" { + score += 2 // Sleeping = likely waiting for input + } + + // Method #2: Enhanced wchan Analysis (beyond basic TTY read) + wchan := getWaitChannel(pid) + if wchan == "poll_schedule_timeout" || // Polling with timeout (interactive pattern) + wchan == "pipe_wait" || // Waiting on pipe input + wchan == "unix_stream_read_generic" || // Reading from socket + wchan == "select" || // Select/poll waiting for input + wchan == "ep_poll" { // Epoll waiting (event-driven input) + score += 3 // Strong indicator of waiting for input + } + + // Method #3: File Descriptor Analysis + // Check if stdin is actively connected to TTY + if hasActiveTTYConnection(pid) { + score += 2 + } + + // Threshold: score >= 4 indicates interactive behavior + // This is conservative - when in doubt, assume interactive to prevent false negatives + isInteractive := score >= 4 + if isInteractive { + logrus.Debugf("CLI Watcher: PID %s detected as interactive via fallback (score: %d, wchan: %s)", pid, score, wchan) + } + return isInteractive +} + +// getProcessState returns the process state from /proc/[pid]/stat field 3 +func getProcessState(pid string) string { + // Read the raw stat file for state (field 3) + statPath := filepath.Join("/proc", pid, "stat") + data, err := os.ReadFile(statPath) + if err != nil { + return "" + } + + str := string(data) + // Find the last ')' to handle process names with spaces/parens + lastParen := strings.LastIndex(str, ")") + if lastParen == -1 { + return "" + } + + // State is the first field after ')' + fields := strings.Fields(str[lastParen+1:]) + if len(fields) > 0 { + return fields[0] // State (R/S/D/Z/T) + } + return "" +} + +// hasActiveTTYConnection checks if process has active TTY file descriptors +func hasActiveTTYConnection(pid string) bool { + // Check if stdin (fd/0) points to a TTY and is recently accessed + fd0Path := filepath.Join("/proc", pid, "fd", "0") + target, err := os.Readlink(fd0Path) + if err != nil { + return false + } + + // Must be a TTY device + if !strings.HasPrefix(target, "/dev/pts/") && !strings.HasPrefix(target, "/dev/tty") { + return false + } + + // Check if the fd directory itself has been recently modified + // This indicates recent file descriptor activity + fdDir := filepath.Join("/proc", pid, "fd") + stat, err := os.Stat(fdDir) + if err != nil { + return false + } + + // If fd directory was modified recently, there's active FD usage + return time.Since(stat.ModTime()) < 5*time.Minute +} + +// isInteractiveProcess detects if a process is interactive by checking: +// 1. Is it in foreground process group? +// 2. Is it waiting on TTY read OR has it ever read from TTY? +func isInteractiveProcess(pid string) bool { + if !isInForegroundProcessGroup(pid) { + return false // Background processes are not interactive + } + + wchan := getWaitChannel(pid) + + // Currently waiting on TTY/terminal read? + // Use exact matching to avoid false positives (e.g., "spreadsheet", "thread_reading") + if wchan == "read" || // Generic read syscall on TTY + wchan == "wait_woken" || // Terminal I/O wait + wchan == "n_tty_read" || // TTY line discipline read + wchan == "tty_read" || // TTY read + wchan == "tty_write" { // TTY write (also indicates terminal interaction) + return true + } + + // Has it ever read from TTY? + if hasEverReadFromTTY(pid) { + return true + } + + return false // Foreground but never read input = work process +} + +// getInteractiveModeDescription returns a human-readable description of the interactive mode +func getInteractiveModeDescription(mode InteractiveMode) string { + switch mode { + case InteractiveModeAuto: + return "auto-detect TTY" + case InteractiveModeTrue, InteractiveModeYes: + return "interactive (activity check)" + case InteractiveModeFalse, InteractiveModeNo: + return "non-interactive (always active)" + default: + return "unknown" + } +} + +// processHasTTY checks if a process has a controlling TTY +func processHasTTY(pid string) bool { + // Check stdin (fd 0) for TTY + fd0Path := filepath.Join("/proc", pid, "fd", "0") + target, err := os.Readlink(fd0Path) + if err != nil { + return false + } + + // TTY devices are typically /dev/pts/N or /dev/tty* + return strings.HasPrefix(target, "/dev/pts/") || + strings.HasPrefix(target, "/dev/tty") +} + +// hasRecentActivity checks if a process has had recent I/O activity +func hasRecentActivity(activityWindow time.Duration, pid string) bool { + window := activityWindow + if window <= 0 { + window = DefaultActivityWindow + } + + // Check TTY access time for user input activity + return hasTTYActivity(pid, window) +} + +// hasTTYActivity checks if the TTY has been accessed recently +func hasTTYActivity(pid string, window time.Duration) bool { + _, atime, valid := getCachedTTYInfo(pid) + if !valid { + return false + } + + threshold := time.Now().Add(-window) + return atime.After(threshold) +} + // Finds the CLI Watcher configuration file in: // 1. Use explicit override by using "CLI_WATCHER_CONFIG" env. variable, or if not set then // 2. Search for '.noidle' upward from current project directory up to "PROJECTS_ROOT" directory, or @@ -227,14 +1128,29 @@ func getConfigPath() string { } func findUpward(start, stop, filename string) string { - current := start - for { + const maxIterations = 100 // Safety limit to prevent infinite loops + + // Resolve symlinks in start path to ensure consistent traversal + current, err := filepath.EvalSymlinks(start) + if err != nil { + // If symlink resolution fails (e.g., broken symlink), use original path + current = start + } + + // Also resolve stop to ensure comparison works correctly + stopResolved, err := filepath.EvalSymlinks(stop) + if err != nil { + // If symlink resolution fails, use original path + stopResolved = stop + } + + for i := 0; i < maxIterations; i++ { candidate := filepath.Join(current, filename) if _, err := os.Stat(candidate); err == nil { return candidate } - if current == stop || current == "/" { + if current == stopResolved || current == "/" { break } @@ -293,20 +1209,56 @@ func (w *cliWatcher) loadConfig(path string, current *cliWatcherConfig) (*cliWat var newCfg cliWatcherConfig if err := yaml.Unmarshal(data, &newCfg); err != nil { - return current, fmt.Errorf("CLI Watcher: Failed to parse config file: %w", err) + // Log helpful error with context + logrus.Errorf("CLI Watcher: Failed to parse config file at %s", path) + logrus.Errorf(" Error: %v", err) + logrus.Errorf(" Hint: Check that 'watchedCommands' entries are either strings or objects with 'name:' field") + if current != nil { + logrus.Errorf(" Keeping previous valid config until syntax is fixed.") + } + // Return error so caller can distinguish "config broken" from "config unchanged" + return current, fmt.Errorf("failed to parse config file: %w", err) } newCfg._lastModTime = info.ModTime() - newCfg = applyDefaults(newCfg) - newCfg = ignoreExclusions(excludedCommands, newCfg) + newCfg = applyDefaults(newCfg, w.idleTimeout) + newCfg = ignoreExclusions(alwaysIgnoredCommands, newCfg) + // Log config changes logrus.Infof("CLI Watcher: Config reloaded from %s", path) + if current != nil && current.Enabled { + if current._checkPeriodParsed != newCfg._checkPeriodParsed { + logrus.Infof("CLI Watcher: Check period changed: %v → %v", current._checkPeriodParsed, newCfg._checkPeriodParsed) + } + if current._activityWindowParsed != newCfg._activityWindowParsed { + logrus.Infof("CLI Watcher: Activity window changed: %v → %v", current._activityWindowParsed, newCfg._activityWindowParsed) + } + if current._gracePeriodParsed != newCfg._gracePeriodParsed { + logrus.Infof("CLI Watcher: Grace period changed: %v → %v", current._gracePeriodParsed, newCfg._gracePeriodParsed) + } + if current._maxProcessAgeParsed != newCfg._maxProcessAgeParsed { + logrus.Infof("CLI Watcher: Max process age changed: %v → %v", current._maxProcessAgeParsed, newCfg._maxProcessAgeParsed) + } + } + if newCfg.Enabled { - logrus.Infof("CLI Watcher: Detecting active commands: %v...", newCfg.WatchedCommands) + if len(newCfg.WatchedCommands) > 0 { + logrus.Infof("CLI Watcher: Watching ALL user processes with %d explicit override(s):", len(newCfg.WatchedCommands)) + for _, cmd := range newCfg.WatchedCommands { + modeDesc := getInteractiveModeDescription(cmd.Interactive) + logrus.Infof("CLI Watcher: - %s (mode: %s)", cmd.Name, modeDesc) + } + } else { + logrus.Infof("CLI Watcher: Watching ALL user processes (no explicit overrides)") + } if len(newCfg.IgnoredCommands) > 0 { - logrus.Infof("CLI Watcher: Skipping watch for: %v...", newCfg.IgnoredCommands) + logrus.Warnf("CLI Watcher: WARNING: You configured %v in watchedCommands, but these are globally excluded (always ignored). Remove them from your config to silence this warning.", newCfg.IgnoredCommands) } - logrus.Infof("CLI Watcher: Detection period is %d seconds", newCfg.CheckPeriodSeconds) + logrus.Infof("CLI Watcher: Always-ignored commands (never prevent idling): %v", alwaysIgnoredCommands) + logrus.Infof("CLI Watcher: Detection period: %v", newCfg._checkPeriodParsed) + logrus.Infof("CLI Watcher: Activity window: %v", newCfg._activityWindowParsed) + logrus.Infof("CLI Watcher: Grace period: %v", newCfg._gracePeriodParsed) + logrus.Infof("CLI Watcher: Max process age: %v (safety limit)", newCfg._maxProcessAgeParsed) } else { logrus.Infof("CLI Watcher: Disabled by configuration. CLI idling prevention is turned off.") } @@ -316,28 +1268,214 @@ func (w *cliWatcher) loadConfig(path string, current *cliWatcherConfig) (*cliWat // Remove excluded CLIs from the watcher configuration. func ignoreExclusions(exclusions []string, cfg cliWatcherConfig) cliWatcherConfig { - var filtered, ignored []string + var filtered []WatchedCommand + var ignored []string for _, cmd := range cfg.WatchedCommands { - name := strings.ToLower(strings.TrimSpace(cmd)) - if slices.ContainsFunc(exclusions, func(ex string) bool { + name := strings.ToLower(strings.TrimSpace(cmd.Name)) + isAlwaysIgnored := slices.ContainsFunc(exclusions, func(ex string) bool { return strings.EqualFold(strings.TrimSpace(ex), name) - }) { - ignored = append(ignored, cmd) + }) + + if isAlwaysIgnored && !cmd.ForceWatch.isEnabled() { + // Command is in always-ignored list and no override specified + ignored = append(ignored, cmd.Name) continue + } else if isAlwaysIgnored && cmd.ForceWatch.isEnabled() { + // User explicitly wants to watch this normally-ignored command + logrus.Warnf("CLI Watcher: Command '%s' is normally always-ignored but forceWatch=true overrides this. Use with caution.", cmd.Name) } + filtered = append(filtered, cmd) } cfg.WatchedCommands = filtered - cfg.IgnoredCommands = ignored + // Preserve user-specified ignoredCommands and add filtered always-ignored ones + cfg.IgnoredCommands = append(cfg.IgnoredCommands, ignored...) return cfg } -// applyDefaults sets fallback values -func applyDefaults(c cliWatcherConfig) cliWatcherConfig { - if c.CheckPeriodSeconds <= 0 { - c.CheckPeriodSeconds = 60 +// parseDuration parses a duration string or integer (treated as seconds) +func parseDuration(value string, fieldName string, defaultValue time.Duration) time.Duration { + if value == "" { + return defaultValue + } + + // Try parsing as duration first (e.g., "6h", "30m", "3600s") + duration, err := time.ParseDuration(value) + if err != nil { + // Fallback: try parsing as integer seconds (e.g., "21600" or "60") + // Use strconv.ParseInt to ensure the ENTIRE string is numeric and avoid 32-bit overflow + seconds, atoiErr := strconv.ParseInt(value, 10, 64) + if atoiErr == nil && seconds > 0 { + // Prevent time.Duration overflow: max safe value is ~292 years + const maxSafeSeconds = int64(9223372036) // math.MaxInt64 / 1e9, rounded down + if seconds > maxSafeSeconds { + logrus.Warnf("CLI Watcher: %s value '%s' (%d seconds) too large (max ~292 years), using default (%v)", fieldName, value, seconds, defaultValue) + return defaultValue + } + duration = time.Duration(seconds) * time.Second + } else { + // Invalid value - warn and use default + logrus.Warnf("CLI Watcher: Invalid %s value '%s' (not a duration or integer), using default (%v)", fieldName, value, defaultValue) + return defaultValue + } + } + + if duration <= 0 { + logrus.Warnf("CLI Watcher: %s is zero or negative (%v), using default (%v)", fieldName, duration, defaultValue) + return defaultValue + } + + // Add reasonable upper bounds to prevent misconfiguration or potential DoS + var maxAllowed time.Duration + switch fieldName { + case "checkPeriod": + maxAllowed = 1 * time.Hour // No point checking less than once per hour + case "activityWindow": + maxAllowed = 24 * time.Hour // Activity windows longer than a day are impractical + case "gracePeriod": + maxAllowed = 1 * time.Hour // Grace periods longer than an hour are excessive + case "maxProcessAge": + maxAllowed = 7 * 24 * time.Hour // Week-long processes are likely stuck + default: + maxAllowed = 24 * time.Hour // Default maximum for unknown fields + } + + if duration > maxAllowed { + logrus.Warnf("CLI Watcher: %s value '%s' (%v) exceeds maximum (%v), using default (%v)", fieldName, value, duration, maxAllowed, defaultValue) + return defaultValue + } + + return duration +} + +// applyDefaults sets fallback values (user values are never changed, only unspecified fields get smart defaults) +func applyDefaults(c cliWatcherConfig, idleTimeout time.Duration) cliWatcherConfig { + // Parse checkPeriod (new field takes priority over deprecated checkPeriodSeconds) + if c.CheckPeriod != "" { + c._checkPeriodParsed = parseDuration(c.CheckPeriod, "checkPeriod", time.Duration(DefaultCheckPeriod)*time.Second) + + // Warn if both old and new fields are specified with different values + if c.CheckPeriodSeconds > 0 { + deprecatedValue := time.Duration(c.CheckPeriodSeconds) * time.Second + if c._checkPeriodParsed != deprecatedValue { + logrus.Warnf("CLI Watcher: Both 'checkPeriod' (%v) and deprecated 'checkPeriodSeconds' (%v) are set - using 'checkPeriod' value", c._checkPeriodParsed, deprecatedValue) + } + } + } else if c.CheckPeriodSeconds > 0 { + // Backward compatibility: use deprecated checkPeriodSeconds + c._checkPeriodParsed = time.Duration(c.CheckPeriodSeconds) * time.Second + } else { + c._checkPeriodParsed = time.Duration(DefaultCheckPeriod) * time.Second + } + + // Validate check period bounds (must be done AFTER parsing both fields) + minCheckPeriod := time.Duration(MinCheckPeriod) * time.Second + if c._checkPeriodParsed < minCheckPeriod { + logrus.Warnf("CLI Watcher: checkPeriod (%v) is below minimum (%v), using minimum", c._checkPeriodParsed, minCheckPeriod) + c._checkPeriodParsed = minCheckPeriod + } + + // Maximum check period should be reasonable and less than idle timeout + // Absolute max: 10 minutes (no point checking less frequently) + // If idleTimeout known: max 1/4 of idle timeout (ensure we can detect activity in time) + var maxCheckPeriod time.Duration + if idleTimeout > 0 { + maxCheckPeriod = idleTimeout / 4 + if maxCheckPeriod > 10*time.Minute { + maxCheckPeriod = 10 * time.Minute + } + } else { + maxCheckPeriod = 10 * time.Minute + } + + if c._checkPeriodParsed > maxCheckPeriod { + if idleTimeout > 0 { + logrus.Warnf("CLI Watcher: checkPeriod (%v) exceeds maximum (%v, 1/4 of idle timeout %v), using maximum", c._checkPeriodParsed, maxCheckPeriod, idleTimeout) + } else { + logrus.Warnf("CLI Watcher: checkPeriod (%v) exceeds maximum (%v), using maximum", c._checkPeriodParsed, maxCheckPeriod) + } + c._checkPeriodParsed = maxCheckPeriod + } + + // Parse gracePeriod (needed first to calculate activityWindow) + var gracePeriodDefault time.Duration + if c.GracePeriod == "" && idleTimeout > 0 { + // Smart default: use smaller of 5m or 15% of idle timeout + gracePeriodDefault = time.Duration(float64(idleTimeout) * 0.15) + if gracePeriodDefault > DefaultGracePeriod { + gracePeriodDefault = DefaultGracePeriod + } + if gracePeriodDefault < MinGracePeriod { + gracePeriodDefault = MinGracePeriod + } + } else { + gracePeriodDefault = DefaultGracePeriod + } + c._gracePeriodParsed = parseDuration(c.GracePeriod, "gracePeriod", gracePeriodDefault) + + // Parse activityWindow (depends on gracePeriod and idleTimeout) + var activityWindowDefault time.Duration + if c.ActivityWindow == "" && idleTimeout > 0 { + // Smart default: idleTimeout - gracePeriod - buffer + buffer := time.Duration(float64(idleTimeout) * SafetyBufferPercent) + if buffer > SafetyBufferDuration { + buffer = SafetyBufferDuration + } + + calculated := idleTimeout - c._gracePeriodParsed - buffer + if calculated < MinActivityWindow { + activityWindowDefault = MinActivityWindow + if calculated <= 0 { + logrus.Warnf("CLI Watcher: Grace period (%v) + buffer (%v) exceeds idle timeout (%v), using minimum activity window (%v)", c._gracePeriodParsed, buffer, idleTimeout, MinActivityWindow) + } else if idleTimeout < 10*time.Minute { + logrus.Warnf("CLI Watcher: Workspace idle timeout (%v) is very short, using minimum activity window (%v)", idleTimeout, MinActivityWindow) + } else { + logrus.Warnf("CLI Watcher: Calculated activity window too short (%v), using minimum (%v)", calculated, MinActivityWindow) + } + } else { + activityWindowDefault = calculated + } + } else if c.ActivityWindow == "" && c._gracePeriodParsed > 0 { + // No idleTimeout but gracePeriod specified: ensure activityWindow > gracePeriod + activityWindowDefault = c._gracePeriodParsed + 2*time.Minute + if activityWindowDefault < DefaultActivityWindow { + activityWindowDefault = DefaultActivityWindow + } + } else { + activityWindowDefault = DefaultActivityWindow + } + c._activityWindowParsed = parseDuration(c.ActivityWindow, "activityWindow", activityWindowDefault) + + // Parse maxProcessAge + c._maxProcessAgeParsed = parseDuration(c.MaxProcessAge, "maxProcessAge", DefaultMaxProcessAge) + + // Apply defaults to each watched command + for i := range c.WatchedCommands { + if c.WatchedCommands[i].Interactive == "" { + c.WatchedCommands[i].Interactive = DefaultInteractiveMode + } + } + + // Validate configuration (warn about misconfigurations but never change user values) + if c._activityWindowParsed < c._gracePeriodParsed { + logrus.Warnf("CLI Watcher: activityWindow (%v) is less than gracePeriod (%v), interactive processes may not be detected correctly", c._activityWindowParsed, c._gracePeriodParsed) } + + if idleTimeout > 0 { + if c._activityWindowParsed >= idleTimeout { + logrus.Warnf("CLI Watcher: activityWindow (%v) exceeds workspace idle timeout (%v), may not work as expected", c._activityWindowParsed, idleTimeout) + } + if c._gracePeriodParsed >= idleTimeout*8/10 { + logrus.Warnf("CLI Watcher: gracePeriod (%v) is very close to workspace idle timeout (%v)", c._gracePeriodParsed, idleTimeout) + } + } + + checkPeriodDuration := c._checkPeriodParsed + if checkPeriodDuration > c._activityWindowParsed/2 { + logrus.Warnf("CLI Watcher: checkPeriod (%v) may be too long for activityWindow (%v), activity might not be detected in time", checkPeriodDuration, c._activityWindowParsed) + } + return c } diff --git a/timeout/cli-watcher_test.go b/timeout/cli-watcher_test.go new file mode 100644 index 000000000..bb1d80615 --- /dev/null +++ b/timeout/cli-watcher_test.go @@ -0,0 +1,885 @@ +// +// Copyright (c) 2026 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package timeout + +import ( + "fmt" + "os" + "runtime" + "testing" + "time" + + "gopkg.in/yaml.v2" +) + +// Test parseDuration with various input formats +func TestParseDuration(t *testing.T) { + tests := []struct { + name string + value string + fieldName string + defaultValue time.Duration + expected time.Duration + }{ + // Valid duration strings + {"6 hours", "6h", "maxProcessAge", 1 * time.Hour, 6 * time.Hour}, + {"30 minutes", "30m", "activityWindow", 10 * time.Minute, 30 * time.Minute}, + {"90 seconds", "90s", "gracePeriod", 5 * time.Minute, 90 * time.Second}, + {"compound duration", "1h30m", "activityWindow", 10 * time.Minute, 90 * time.Minute}, + {"compound with seconds", "45m30s", "gracePeriod", 5 * time.Minute, 45*time.Minute + 30*time.Second}, + + // Valid integers (treated as seconds) + {"integer 3600", "3600", "maxProcessAge", 1 * time.Hour, 3600 * time.Second}, + {"integer 1800", "1800", "activityWindow", 10 * time.Minute, 1800 * time.Second}, + {"integer 300", "300", "gracePeriod", 5 * time.Minute, 300 * time.Second}, + // Note: parseDuration rejects zero/negative for checkPeriod, returns default + {"integer 0", "0", "checkPeriod", 60 * time.Second, 60 * time.Second}, + + // Empty string (should return default) + {"empty string", "", "activityWindow", 25 * time.Minute, 25 * time.Minute}, + + // Invalid formats (should return default and log warning) + {"invalid format", "invalid", "activityWindow", 10 * time.Minute, 10 * time.Minute}, + {"negative duration", "-5m", "gracePeriod", 5 * time.Minute, 5 * time.Minute}, + {"negative integer", "-300", "gracePeriod", 5 * time.Minute, 5 * time.Minute}, + + // Typo cases - should use default, not silently parse partial integer + // These were previously parsed by fmt.Sscanf which stops at first non-digit + {"typo: 30min", "30min", "activityWindow", 10 * time.Minute, 10 * time.Minute}, + {"typo: 5minutes", "5minutes", "gracePeriod", 5 * time.Minute, 5 * time.Minute}, + {"typo: 1hour", "1hour", "maxProcessAge", 6 * time.Hour, 6 * time.Hour}, + {"typo: 30x", "30x", "activityWindow", 10 * time.Minute, 10 * time.Minute}, + {"typo: 100sec", "100sec", "checkPeriod", 60 * time.Second, 60 * time.Second}, + + // Upper bounds validation - values exceeding maximum should use default + {"checkPeriod exceeds max", "2h", "checkPeriod", 60 * time.Second, 60 * time.Second}, + {"gracePeriod exceeds max", "2h", "gracePeriod", 5 * time.Minute, 5 * time.Minute}, + {"activityWindow exceeds max", "48h", "activityWindow", 10 * time.Minute, 10 * time.Minute}, + {"maxProcessAge exceeds max", "30d", "maxProcessAge", 6 * time.Hour, 6 * time.Hour}, + + // Large integer test - would overflow int32 on 32-bit systems without proper handling + // This value exceeds activityWindow max (24h) so should use default + {"large integer (32-bit overflow test)", "2200000000", "activityWindow", 10 * time.Minute, 10 * time.Minute}, + + // Extreme value test - would overflow time.Duration multiplication + // ~500 years in seconds, should use default + {"extreme duration overflow test", "15768000000000", "checkPeriod", 60 * time.Second, 60 * time.Second}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseDuration(tt.value, tt.fieldName, tt.defaultValue) + if result != tt.expected { + t.Errorf("parseDuration(%q, %q, %v) = %v, want %v", + tt.value, tt.fieldName, tt.defaultValue, result, tt.expected) + } + }) + } +} + +// Test WatchedCommand UnmarshalYAML with both string and object formats +func TestWatchedCommandUnmarshalYAML(t *testing.T) { + tests := []struct { + name string + yaml string + expected WatchedCommand + shouldError bool + }{ + { + name: "simple string", + yaml: "helm", + // Simple strings default to InteractiveModeNo in UnmarshalYAML + expected: WatchedCommand{Name: "helm", Interactive: InteractiveModeNo}, + }, + { + name: "object with name only", + yaml: "name: kubectl", + expected: WatchedCommand{Name: "kubectl", Interactive: ""}, + }, + { + name: "object with interactive auto", + yaml: "name: vim\ninteractive: auto", + expected: WatchedCommand{Name: "vim", Interactive: InteractiveModeAuto}, + }, + { + name: "object with interactive true", + yaml: "name: claude\ninteractive: true", + expected: WatchedCommand{Name: "claude", Interactive: InteractiveModeTrue}, + }, + { + name: "object with interactive false", + yaml: "name: npm\ninteractive: false", + expected: WatchedCommand{Name: "npm", Interactive: InteractiveModeFalse}, + }, + { + name: "object with interactive yes", + yaml: "name: editor\ninteractive: yes", + expected: WatchedCommand{Name: "editor", Interactive: InteractiveModeYes}, + }, + { + name: "object with interactive no", + yaml: "name: build\ninteractive: no", + expected: WatchedCommand{Name: "build", Interactive: InteractiveModeNo}, + }, + { + name: "object with forceWatch true", + yaml: "name: watch\nforceWatch: true", + expected: WatchedCommand{Name: "watch", ForceWatch: ForceWatchModeTrue}, + }, + { + name: "object with forceWatch yes", + yaml: "name: watch\nforceWatch: yes", + expected: WatchedCommand{Name: "watch", ForceWatch: ForceWatchModeYes}, + }, + { + name: "object with forceWatch false", + yaml: "name: watch\nforceWatch: false", + expected: WatchedCommand{Name: "watch", ForceWatch: ForceWatchModeFalse}, + }, + { + name: "object with forceWatch no", + yaml: "name: watch\nforceWatch: no", + expected: WatchedCommand{Name: "watch", ForceWatch: ForceWatchModeNo}, + }, + { + name: "object with all fields", + yaml: "name: top\ninteractive: false\nforceWatch: true", + expected: WatchedCommand{Name: "top", Interactive: InteractiveModeFalse, ForceWatch: ForceWatchModeTrue}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var cmd WatchedCommand + err := yaml.Unmarshal([]byte(tt.yaml), &cmd) + + if tt.shouldError { + if err == nil { + t.Errorf("Expected error but got none") + } + return + } + + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if cmd.Name != tt.expected.Name { + t.Errorf("Name = %q, want %q", cmd.Name, tt.expected.Name) + } + if cmd.Interactive != tt.expected.Interactive { + t.Errorf("Interactive = %q, want %q", cmd.Interactive, tt.expected.Interactive) + } + if cmd.ForceWatch != tt.expected.ForceWatch { + t.Errorf("ForceWatch = %v, want %v", cmd.ForceWatch, tt.expected.ForceWatch) + } + }) + } +} + +// Test ignoreExclusions filters out globally-excluded commands +func TestIgnoreExclusions(t *testing.T) { + tests := []struct { + name string + exclusions []string + inputCommands []WatchedCommand + expectedCount int + expectedIgnored int + }{ + { + name: "no exclusions", + exclusions: []string{}, + inputCommands: []WatchedCommand{ + {Name: "helm", Interactive: ""}, + {Name: "kubectl", Interactive: ""}, + }, + expectedCount: 2, + expectedIgnored: 0, + }, + { + name: "filter tail", + exclusions: []string{"tail"}, + inputCommands: []WatchedCommand{ + {Name: "helm", Interactive: ""}, + {Name: "tail", Interactive: ""}, + {Name: "kubectl", Interactive: ""}, + }, + expectedCount: 2, + expectedIgnored: 1, + }, + { + name: "filter multiple", + exclusions: []string{"tail", "watch", "top"}, + inputCommands: []WatchedCommand{ + {Name: "helm", Interactive: ""}, + {Name: "tail", Interactive: ""}, + {Name: "watch", Interactive: ""}, + {Name: "kubectl", Interactive: ""}, + {Name: "top", Interactive: ""}, + }, + expectedCount: 2, + expectedIgnored: 3, + }, + { + name: "case insensitive", + exclusions: []string{"tail"}, + inputCommands: []WatchedCommand{ + {Name: "Tail", Interactive: ""}, + {Name: "TAIL", Interactive: ""}, + {Name: "helm", Interactive: ""}, + }, + expectedCount: 1, + expectedIgnored: 2, + }, + { + name: "all excluded", + exclusions: []string{"tail", "watch"}, + inputCommands: []WatchedCommand{ + {Name: "tail", Interactive: ""}, + {Name: "watch", Interactive: ""}, + }, + expectedCount: 0, + expectedIgnored: 2, + }, + { + name: "forceWatch overrides exclusion", + exclusions: []string{"watch", "top"}, + inputCommands: []WatchedCommand{ + {Name: "helm", Interactive: ""}, + {Name: "watch", Interactive: "false", ForceWatch: ForceWatchModeTrue}, // Override exclusion + {Name: "top", Interactive: ""}, // Still excluded + }, + expectedCount: 2, // helm + watch (override) + expectedIgnored: 1, // top + }, + { + name: "forceWatch false still excluded", + exclusions: []string{"watch"}, + inputCommands: []WatchedCommand{ + {Name: "watch", Interactive: "", ForceWatch: ForceWatchModeFalse}, // Explicit false + }, + expectedCount: 0, + expectedIgnored: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := cliWatcherConfig{ + WatchedCommands: tt.inputCommands, + } + + result := ignoreExclusions(tt.exclusions, cfg) + + if len(result.WatchedCommands) != tt.expectedCount { + t.Errorf("WatchedCommands count = %d, want %d", + len(result.WatchedCommands), tt.expectedCount) + } + + if len(result.IgnoredCommands) != tt.expectedIgnored { + t.Errorf("IgnoredCommands count = %d, want %d", + len(result.IgnoredCommands), tt.expectedIgnored) + } + + // Verify no excluded commands remain (unless forceWatch=true) + for _, cmd := range result.WatchedCommands { + for _, ex := range tt.exclusions { + if cmd.Name == ex && !cmd.ForceWatch.isEnabled() { + t.Errorf("Excluded command %q still in WatchedCommands without forceWatch", cmd.Name) + } + } + } + }) + } +} + +// Test applyDefaults with various idle timeout scenarios +func TestApplyDefaults(t *testing.T) { + tests := []struct { + name string + config cliWatcherConfig + idleTimeout time.Duration + expectedCheckPeriod time.Duration + expectedGracePeriodMin time.Duration + expectedGracePeriodMax time.Duration + expectedActivityWindowMin time.Duration + }{ + { + name: "all defaults with 30m idle timeout", + config: cliWatcherConfig{ + CheckPeriod: "", + GracePeriod: "", + ActivityWindow: "", + }, + idleTimeout: 30 * time.Minute, + expectedCheckPeriod: 60 * time.Second, // DefaultCheckPeriod + expectedGracePeriodMin: 1 * time.Minute, + expectedGracePeriodMax: 5 * time.Minute, + expectedActivityWindowMin: 2 * time.Minute, + }, + { + name: "user-specified values preserved", + config: cliWatcherConfig{ + CheckPeriod: "45", + GracePeriod: "10m", + ActivityWindow: "20m", + }, + idleTimeout: 30 * time.Minute, + expectedCheckPeriod: 45 * time.Second, + expectedGracePeriodMin: 10 * time.Minute, + expectedGracePeriodMax: 10 * time.Minute, + expectedActivityWindowMin: 20 * time.Minute, + }, + { + name: "no idle timeout (disabled)", + config: cliWatcherConfig{ + CheckPeriod: "", + GracePeriod: "", + ActivityWindow: "", + }, + idleTimeout: -1, + expectedCheckPeriod: 60 * time.Second, // DefaultCheckPeriod + expectedGracePeriodMin: DefaultGracePeriod, + expectedGracePeriodMax: DefaultGracePeriod, + expectedActivityWindowMin: DefaultActivityWindow, + }, + { + name: "very short idle timeout (5m)", + config: cliWatcherConfig{ + CheckPeriod: "", + GracePeriod: "", + ActivityWindow: "", + }, + idleTimeout: 5 * time.Minute, + expectedCheckPeriod: 60 * time.Second, // DefaultCheckPeriod + expectedGracePeriodMin: MinGracePeriod, + expectedGracePeriodMax: MinGracePeriod, + expectedActivityWindowMin: MinActivityWindow, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := applyDefaults(tt.config, tt.idleTimeout) + + // Check checkPeriod + if result._checkPeriodParsed != tt.expectedCheckPeriod { + t.Errorf("checkPeriod = %v, want %v", + result._checkPeriodParsed, tt.expectedCheckPeriod) + } + + // Check gracePeriod (range for adaptive defaults) + if result._gracePeriodParsed < tt.expectedGracePeriodMin || + result._gracePeriodParsed > tt.expectedGracePeriodMax { + t.Errorf("gracePeriod = %v, want between %v and %v", + result._gracePeriodParsed, tt.expectedGracePeriodMin, tt.expectedGracePeriodMax) + } + + // Check activityWindow (minimum check for adaptive defaults) + if result._activityWindowParsed < tt.expectedActivityWindowMin { + t.Errorf("activityWindow = %v, want at least %v", + result._activityWindowParsed, tt.expectedActivityWindowMin) + } + + // Check maxProcessAge always has default + if result._maxProcessAgeParsed == 0 { + t.Errorf("maxProcessAge should not be zero") + } + }) + } +} + +// Test isNumeric helper function +func TestIsNumeric(t *testing.T) { + tests := []struct { + name string + input string + expected bool + }{ + {"empty string", "", false}, // Empty string is NOT numeric (edge case fix) + {"single digit", "5", true}, + {"multiple digits", "12345", true}, + {"zero", "0", true}, + {"leading zeros", "00123", true}, + + // Non-numeric cases + {"letters", "abc", false}, + {"mixed", "123abc", false}, + {"negative", "-123", false}, + {"decimal", "12.34", false}, + {"space", "12 34", false}, + {"duration", "30s", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isNumeric(tt.input) + if result != tt.expected { + t.Errorf("isNumeric(%q) = %v, want %v", tt.input, result, tt.expected) + } + }) + } +} + +// Test getInteractiveModeDescription +func TestGetInteractiveModeDescription(t *testing.T) { + tests := []struct { + mode InteractiveMode + expected string + }{ + {InteractiveModeAuto, "auto-detect TTY"}, + {InteractiveModeTrue, "interactive (activity check)"}, + {InteractiveModeYes, "interactive (activity check)"}, + {InteractiveModeFalse, "non-interactive (always active)"}, + {InteractiveModeNo, "non-interactive (always active)"}, + {InteractiveMode(""), "unknown"}, // Empty string doesn't match any case + {InteractiveMode("invalid"), "unknown"}, // Invalid value + } + + for _, tt := range tests { + t.Run(string(tt.mode), func(t *testing.T) { + result := getInteractiveModeDescription(tt.mode) + if result != tt.expected { + t.Errorf("getInteractiveModeDescription(%q) = %q, want %q", + tt.mode, result, tt.expected) + } + }) + } +} + +// Test getPlatformDefaultClockTicks +func TestGetPlatformDefaultClockTicks(t *testing.T) { + result := getPlatformDefaultClockTicks() + + // Verify result is one of the expected values + if result != 100 && result != 250 { + t.Errorf("getPlatformDefaultClockTicks() = %d, want 100 or 250", result) + } + + // Platform-specific checks (can only validate current platform) + switch runtime.GOARCH { + case "arm", "arm64": + if result != 250 { + t.Errorf("On ARM platform, expected 250 ticks but got %d", result) + } + case "amd64", "386": + if result != 100 { + t.Errorf("On x86 platform, expected 100 ticks but got %d", result) + } + default: + // Other platforms should default to 100 + if result != 100 { + t.Errorf("On platform %s, expected 100 ticks but got %d", runtime.GOARCH, result) + } + } +} + +// Test applyPolicy mode handling +// Note: Full testing of interactive modes (auto, true, yes) requires real /proc filesystem +// These tests cover the non-interactive mode logic which doesn't require process inspection +func TestApplyPolicy(t *testing.T) { + tests := []struct { + name string + mode InteractiveMode + expectedResult bool + note string + }{ + { + name: "non-interactive mode: false", + mode: InteractiveModeFalse, + expectedResult: true, + note: "Should always return true (prevent idling)", + }, + { + name: "non-interactive mode: no", + mode: InteractiveModeNo, + expectedResult: true, + note: "Should always return true (prevent idling)", + }, + { + name: "empty mode (defaults to no)", + mode: InteractiveMode(""), + expectedResult: true, + note: "Empty mode should behave as non-interactive", + }, + // Note: Cannot fully test interactive modes without real /proc: + // - InteractiveModeAuto calls isInteractiveProcess(pid) which needs /proc + // - InteractiveModeTrue/Yes call hasRecentActivity(pid) which needs /proc/[pid]/fd/0 + // These require integration testing with real processes + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Use a non-existent PID since we're only testing non-interactive modes + // which don't call process inspection functions + result := applyPolicy("99999", "testcmd", tt.mode, 60*time.Second, "test") + + if result != tt.expectedResult { + t.Errorf("applyPolicy with mode %q = %v, want %v (%s)", + tt.mode, result, tt.expectedResult, tt.note) + } + }) + } +} + +// Test concurrent access (race detector) +func TestConcurrentStartStop(t *testing.T) { + t.Run("concurrent Start calls", func(t *testing.T) { + watcher := NewCliWatcher(func() {}, 30*time.Minute) + + // Start multiple goroutines trying to start the watcher + done := make(chan bool, 5) + for i := 0; i < 5; i++ { + go func() { + watcher.Start() + done <- true + }() + } + + // Wait for all to complete + for i := 0; i < 5; i++ { + <-done + } + + // Should be started exactly once + if !watcher.started { + t.Error("Watcher should be started after concurrent Start() calls") + } + + watcher.Stop() + }) + + t.Run("concurrent Stop calls", func(t *testing.T) { + watcher := NewCliWatcher(func() {}, 30*time.Minute) + watcher.Start() + time.Sleep(10 * time.Millisecond) // Let it actually start + + // Stop multiple times concurrently + done := make(chan bool, 5) + for i := 0; i < 5; i++ { + go func() { + defer func() { + if r := recover(); r != nil { + t.Errorf("Stop() panicked: %v", r) + } + done <- true + }() + watcher.Stop() + }() + } + + // Wait for all to complete + for i := 0; i < 5; i++ { + <-done + } + }) + + t.Run("concurrent Start and Stop", func(t *testing.T) { + watcher := NewCliWatcher(func() {}, 30*time.Minute) + + done := make(chan bool, 10) + + // 5 goroutines trying to start + for i := 0; i < 5; i++ { + go func() { + watcher.Start() + done <- true + }() + } + + // 5 goroutines trying to stop + for i := 0; i < 5; i++ { + go func() { + defer func() { + if r := recover(); r != nil { + t.Errorf("Concurrent Start/Stop panicked: %v", r) + } + done <- true + }() + watcher.Stop() + }() + } + + // Wait for all to complete + for i := 0; i < 10; i++ { + <-done + } + }) +} + +// Test system clock changes and time edge cases +func TestProcessAgeWithClockSkew(t *testing.T) { + // Note: This test documents expected behavior when system clock changes + // Full testing requires mocking time.Now() which isn't easily done in Go without interfaces + + t.Run("getProcessAge with zero startTime", func(t *testing.T) { + // When getProcessStartTime returns zero (error case), age should be 0 + age := getProcessAge("99999") // Non-existent PID + if age != 0 { + t.Errorf("Process age for invalid PID should be 0, got %v", age) + } + }) + + t.Run("process age validation", func(t *testing.T) { + // Verify that getProcessAge() handles clock skew gracefully + // We can't easily test negative time.Since() without mocking, + // but we verify the current implementation returns non-negative values + + // Test with current process (should always have valid age >= 0) + age := getProcessAge(fmt.Sprintf("%d", os.Getpid())) + if age < 0 { + t.Errorf("Process age should never be negative (clock skew should return 0), got %v", age) + } + + t.Log("✓ getProcessAge() properly handles clock skew by returning 0 for negative durations") + t.Log(" This ensures processes get grace period protection even after backward clock adjustment") + }) +} + +// Test concurrent config access +func TestConcurrentConfigAccess(t *testing.T) { + t.Run("concurrent config reads", func(t *testing.T) { + watcher := NewCliWatcher(func() {}, 30*time.Minute) + + // This test will fail with -race if there's a data race + done := make(chan bool, 10) + + // Start the watcher (which will reload config periodically) + watcher.Start() + defer watcher.Stop() + + time.Sleep(10 * time.Millisecond) // Let watcher start + + // Read config from multiple goroutines + // Note: Direct access to watcher.config requires mutex, but our implementation + // uses configSnapshot pattern, so we can't test direct field access + // Instead, verify the watcher doesn't crash when running concurrently + for i := 0; i < 10; i++ { + go func() { + // Just verify watcher is running without panicking + // The actual config access is protected in Start() via snapshot + time.Sleep(5 * time.Millisecond) + done <- true + }() + } + + // Wait for all reads + for i := 0; i < 10; i++ { + <-done + } + }) +} + +// Test /proc parsing with real process data +// These are integration tests that require a Linux /proc filesystem +func TestProcParsing(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("Skipping /proc parsing tests on non-Linux platform") + } + + t.Run("parseProcStat with current process", func(t *testing.T) { + // Test with our own PID (we know it exists and is valid) + myPID := fmt.Sprintf("%d", os.Getpid()) + + stat, err := parseProcStat(myPID) + if err != nil { + t.Fatalf("parseProcStat(%s) failed: %v", myPID, err) + } + + // Validate returned fields + if stat.ppid == "" { + t.Error("parseProcStat returned empty ppid") + } + if stat.ppid == "0" { + t.Error("parseProcStat returned ppid=0 (invalid for non-init process)") + } + if stat.pgrp <= 0 { + t.Errorf("parseProcStat returned invalid pgrp: %d", stat.pgrp) + } + if stat.startTicks <= 0 { + t.Errorf("parseProcStat returned invalid startTicks: %d", stat.startTicks) + } + + t.Logf("Current process stats: ppid=%s, pgrp=%d, tpgid=%d, startTicks=%d", + stat.ppid, stat.pgrp, stat.tpgid, stat.startTicks) + }) + + t.Run("parseProcStat with init process", func(t *testing.T) { + // PID 1 should always exist on Linux + stat, err := parseProcStat("1") + if err != nil { + t.Fatalf("parseProcStat(1) failed: %v", err) + } + + // Init should have ppid=0 + if stat.ppid != "0" { + t.Errorf("Init process ppid = %s, want 0", stat.ppid) + } + if stat.startTicks <= 0 { + t.Errorf("Init process has invalid startTicks: %d", stat.startTicks) + } + }) + + t.Run("parseProcStat with invalid PID", func(t *testing.T) { + // Very high PID unlikely to exist + _, err := parseProcStat("999999") + if err == nil { + t.Error("parseProcStat(999999) should fail for non-existent PID") + } + }) + + t.Run("getParentPID with current process", func(t *testing.T) { + myPID := fmt.Sprintf("%d", os.Getpid()) + + ppid := getParentPID(myPID) + if ppid == "" { + t.Error("getParentPID returned empty string for valid PID") + } + if ppid == "0" { + t.Error("getParentPID returned 0 (invalid for non-init process)") + } + + t.Logf("Current process parent PID: %s", ppid) + }) + + t.Run("getProcessStartTime with current process", func(t *testing.T) { + myPID := fmt.Sprintf("%d", os.Getpid()) + + startTime := getProcessStartTime(myPID) + if startTime.IsZero() { + t.Error("getProcessStartTime returned zero time for valid PID") + } + + // Start time should be in the past + if startTime.After(time.Now()) { + t.Errorf("Process start time %v is in the future", startTime) + } + + // Start time should be recent (within last hour for test process) + age := time.Since(startTime) + if age > 1*time.Hour { + t.Logf("Warning: Process age is %v (seems old for test process)", age) + } + + t.Logf("Current process started at: %v (age: %v)", startTime, age) + }) + + t.Run("getProcessAge with current process", func(t *testing.T) { + myPID := fmt.Sprintf("%d", os.Getpid()) + + age := getProcessAge(myPID) + if age <= 0 { + t.Error("getProcessAge returned non-positive duration for valid PID") + } + + // Age should be reasonable (less than 1 hour for test) + if age > 1*time.Hour { + t.Logf("Warning: Process age %v seems old for test process", age) + } + + t.Logf("Current process age: %v", age) + }) + + t.Run("isNumeric with various inputs", func(t *testing.T) { + tests := []struct { + input string + expected bool + }{ + {"123", true}, + {"0", true}, + {"999999", true}, + {"abc", false}, + {"12a34", false}, + {"-123", false}, + {"", false}, // Empty string should be false (edge case) + } + + for _, tt := range tests { + result := isNumeric(tt.input) + if result != tt.expected { + t.Errorf("isNumeric(%q) = %v, want %v", tt.input, result, tt.expected) + } + } + }) + + t.Run("processHasTTY detection", func(t *testing.T) { + // Note: This test may vary depending on how the test is run + // (terminal vs CI environment) + myPID := fmt.Sprintf("%d", os.Getpid()) + + hasTTY := processHasTTY(myPID) + t.Logf("Current process has TTY: %v", hasTTY) + + // Init process (PID 1) typically has no TTY + initHasTTY := processHasTTY("1") + if initHasTTY { + t.Log("Note: Init process has TTY (unusual but not necessarily wrong)") + } + }) + + t.Run("getMainUserProcess behavior", func(t *testing.T) { + // This is hard to test deterministically, but we can verify it doesn't crash + myPID := fmt.Sprintf("%d", os.Getpid()) + + mainPID, found := getMainUserProcess(myPID) + t.Logf("getMainUserProcess(%s) = %s, found=%v", myPID, mainPID, found) + + // If found, mainPID should be valid + if found && mainPID == "" { + t.Error("getMainUserProcess returned found=true but empty PID") + } + }) +} + +// Test backward compatibility with deprecated fields +func TestBackwardCompatibility(t *testing.T) { + tests := []struct { + name string + yaml string + expected time.Duration + }{ + { + name: "old checkPeriodSeconds", + yaml: "checkPeriodSeconds: 45", + expected: 45 * time.Second, + }, + { + name: "new checkPeriod string", + yaml: "checkPeriod: 30s", + expected: 30 * time.Second, + }, + { + name: "new checkPeriod integer", + yaml: "checkPeriod: 60", + expected: 60 * time.Second, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var cfg cliWatcherConfig + err := yaml.Unmarshal([]byte(tt.yaml), &cfg) + if err != nil { + t.Fatalf("Unmarshal error: %v", err) + } + + result := applyDefaults(cfg, 30*time.Minute) + + if result._checkPeriodParsed != tt.expected { + t.Errorf("checkPeriod = %v, want %v", + result._checkPeriodParsed, tt.expected) + } + }) + } +} diff --git a/timeout/inactivity.go b/timeout/inactivity.go index d13680051..c396236ae 100644 --- a/timeout/inactivity.go +++ b/timeout/inactivity.go @@ -1,5 +1,5 @@ // -// Copyright (c) 2019-2025 Red Hat, Inc. +// Copyright (c) 2019-2026 Red Hat, Inc. // This program and the accompanying materials are made // available under the terms of the Eclipse Public License 2.0 // which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -122,7 +122,7 @@ func (m inactivityIdleManagerImpl) Start() { } }() - m.watcher = NewCliWatcher(m.Tick) + m.watcher = NewCliWatcher(m.Tick, m.idleTimeout) m.watcher.Start() }