Skip to content

Add CLI activation and single process IPC#8

Open
kjerk wants to merge 4 commits intomainfrom
add-cli-actions
Open

Add CLI activation and single process IPC#8
kjerk wants to merge 4 commits intomainfrom
add-cli-actions

Conversation

@kjerk
Copy link
Copy Markdown
Owner

@kjerk kjerk commented Feb 14, 2026

resolves #1

adds --start-timer "Better Type Fast" capability, either spawning that way or firing on the running process.

@kjerk
Copy link
Copy Markdown
Owner Author

kjerk commented Feb 14, 2026

@claude can you code review this as well as provide a small readme for what the changes entail at a user level

@kjerk kjerk added the run-tests Maintainer toggled command to run tests on a PR label Feb 14, 2026
@claude
Copy link
Copy Markdown

claude bot commented Feb 14, 2026

Claude finished @kjerk's task in 2m 8s —— View job


Code Review & Documentation

Todo List:

  • Read repository guidelines (CLAUDE.md)
  • Read all changed files
  • Perform code review
  • Create user-level README
  • Post findings

Code Review

I've reviewed the PR and overall the implementation is well-structured and functional. The code demonstrates a clean approach to single-instance management with IPC communication. Here are my findings:

Architecture & Design ✅

Strengths:

  • Clean separation of concerns with dedicated CLI namespace
  • Well-designed IPC protocol using named pipes
  • Proper use of Mutex for single-instance enforcement
  • Smart identifier resolution with fallback hierarchy (GUID → DisplayLabel → ProcessName base → ProcessName exe)

Potential Issues & Improvements

1. Race Condition in Mutex Disposal (src/TimeToKill.App/Cli/SingleInstanceManager.cs:94-96)

The Dispose method has a potential race condition:

if (_isFirstInstance) {
    try { _mutex?.ReleaseMutex(); } catch { }
}
_mutex?.Dispose();

If _isFirstInstance is true, you should release the mutex before disposing. However, there's a subtle issue: if the mutex was abandoned or already released, ReleaseMutex() will throw. The try-catch handles this, but it's better practice to track whether the mutex is currently owned.

Recommendation: Consider adding a _mutexOwned flag to track ownership state more explicitly.

2. Silent Exception Swallowing (Multiple locations)

Several locations silently swallow exceptions without logging:

  • SingleInstanceManager.cs:51 - Connection-level errors
  • SingleInstanceManager.cs:68 - Malformed message errors
  • SingleInstanceManager.cs:83 - Send command errors
  • SingleInstanceManager.cs:91 - Task wait errors
  • SingleInstanceManager.cs:95 - Mutex release errors

Recommendation: Consider adding logging or diagnostic output for debugging. While these may be expected in some cases, having visibility into failures would help with troubleshooting.

3. Unsafe HasCommands Implementation (src/TimeToKill.App/Cli/CliOptions.cs:23)

public bool HasCommands => StartTimers?.GetEnumerator().MoveNext() == true;

This implementation creates an enumerator without disposing it, causing a potential resource leak. While IEnumerable<string> enumerators are typically lightweight, this violates the IDisposable pattern.

Recommendation:

public bool HasCommands => StartTimers?.Any() == true;

Or if you want to avoid LINQ:

public bool HasCommands {
    get {
        if (StartTimers == null) return false;
        using var e = StartTimers.GetEnumerator();
        return e.MoveNext();
    }
}

Fix this →

4. Missing Null Check in CommandHandler (src/TimeToKill.App/Cli/CommandHandler.cs:34)

The foreach loop doesn't verify that command.Arguments is non-null:

foreach (var identifier in command.Arguments) {

While IpcCommand initializes Arguments to a new list, if deserialization produces a null, this could throw.

Recommendation: Add defensive null check:

if (command.Arguments == null) return;
foreach (var identifier in command.Arguments) {

5. Incomplete Help Implementation (src/TimeToKill.App/Cli/CliOptions.cs:38-44)

The PrintHelp() method exists but appears to never be called based on the code flow. The Help option is defined but not handled in Program.Main().

Expected behavior: When --help is passed, the app should print help and exit. Currently, if --help is passed alone, the parser succeeds but nothing happens.

Fix this →

6. Unused NativeMethods (src/TimeToKill.Shared/Tools/NativeMethods.cs)

The NativeMethods.AttachToParentConsole() method is defined but never used in the codebase. Based on the commit message mentioning "Disable cli attach and help for now," this appears to be intentionally deferred work.

Note: If you plan to implement console attachment for help text output, this will be useful. Otherwise, consider removing until needed.

7. JSON Deserialization Without Options (src/TimeToKill.App/Cli/SingleInstanceManager.cs:64)

var command = JsonSerializer.Deserialize<IpcCommand>(json);

Using JsonSerializer without options means:

  • Property name matching is case-sensitive by default
  • If the JSON structure changes, deserialization fails silently (returns null)

Recommendation: Consider defining JsonSerializerOptions with PropertyNameCaseInsensitive = true for robustness, or at minimum add a comment documenting the expected JSON format.

8. Potential Dispatcher Thread Deadlock (src/TimeToKill.App/Cli/CommandHandler.cs:42-53)

Nested Dispatcher.UIThread.Post() calls could theoretically cause issues if the UI thread is blocked. The current implementation uses Post (which is non-blocking), so this is likely fine, but it's worth noting that if the UI thread is heavily loaded, these commands could be delayed significantly.

Recommendation: This is acceptable for current use, but if responsiveness becomes an issue, consider using InvokeAsync with proper error handling.

Code Style & Minor Issues

9. Inconsistent String Handling (src/TimeToKill.App/Cli/CommandHandler.cs:46-48)

var name = preset.DisplayLabel;
if (string.IsNullOrWhiteSpace(name))
    name = preset.ProcessName;

This could be simplified:

var name = !string.IsNullOrWhiteSpace(preset.DisplayLabel) 
    ? preset.DisplayLabel 
    : preset.ProcessName;

10. Whitespace-Only Change (src/TimeToKill.Shared/Tools/ProcessNameHelper.cs:6)

Line 6 removes a blank line. This is fine but unrelated to the feature.


Security Considerations

Named Pipe Security

The current implementation uses named pipes without explicit security descriptors. On Windows, the default ACL should restrict access to the current user, which is acceptable for this use case. However, be aware that:

  • Any process running as the same user can connect to the pipe
  • Malicious local processes could send crafted commands

Recommendation: For added security, consider:

  1. Validating the IpcCommand structure more strictly
  2. Adding a simple authentication token or process ID verification
  3. Documenting that this is designed for single-user scenarios

This is not a critical issue for a desktop productivity app, but worth considering for future enhancements.


Testing Recommendations

  1. Test timeout scenarios: What happens if the pipe server is unresponsive when sending commands?
  2. Test malformed JSON: Verify that sending invalid JSON to the pipe doesn't crash the server
  3. Test mutex edge cases: What if the mutex is abandoned? (e.g., app crashes while holding it)
  4. Test identifier resolution ambiguity: Verify error messages when multiple presets match

User-Level README for Changes

CLI Commands & Single Instance Management

This PR adds command-line interface (CLI) support to TimeToKill, allowing you to control timers from the command line or scripts. The application now enforces single-instance behavior, so only one copy runs at a time.

What's New

Start Timers from Command Line

You can now start timers by passing arguments when launching the app:

TimeToKill.exe --start-timer "discord"
TimeToKill.exe --start-timer "Better Type Fast"
TimeToKill.exe --start-timer "95e9f8d1-3c4a-4b2e-8f6d-1a2b3c4d5e6f"

Multiple Timers at Once

Start multiple timers in a single command:

TimeToKill.exe --start-timer "discord" --start-timer "steam"

How Timer Lookup Works

When you specify a timer to start, TimeToKill searches in this order:

  1. GUID - Exact match on the timer's unique ID
  2. Display Label - Case-insensitive match on the custom name you gave the timer
  3. Process Name (base) - Match on process name without extension (e.g., "discord" matches "discord.exe")
  4. Process Name (exact) - Match on full process name (e.g., "discord.exe")

If multiple timers match your identifier, you'll get an error message listing how many conflicts were found. Use a more specific identifier (like the GUID or exact display label) to resolve ambiguity.

Single Instance Behavior

TimeToKill now ensures only one instance runs at a time:

  • First launch: The application starts normally and runs in the system tray
  • Subsequent launches: Instead of opening a new window, commands are sent to the already-running instance
    • If you pass --start-timer, the running instance receives the command and starts the timer
    • If you launch without arguments, the existing instance does nothing (use the tray icon to show the window)

This prevents multiple copies of the app from running and conflicting with each other.

In-App Notifications

When a timer is started via CLI, you'll see a notification at the bottom of the app window (if visible) confirming which timer started. If there's an error (timer not found, ambiguous name, etc.), the error message appears in the notification area.

Use Cases

Launch games with automatic timers:

start steam://rungameid/123456 && TimeToKill.exe --start-timer "game-timer"

Script-based timer control:

REM Start your focus session
TimeToKill.exe --start-timer "Social Media Blocker"
REM Do your work...

Desktop shortcuts:
Create a Windows shortcut to TimeToKill.exe --start-timer "Daily Limit" for one-click timer activation.

Technical Details

  • Commands are sent via Windows Named Pipes for inter-process communication
  • Single instance is enforced using a named mutex (TimeToKill_SingleInstance)
  • Timer lookups are case-insensitive for user convenience
  • If the running instance is unresponsive, command sends timeout after 3 seconds

Summary

Overall Assessment: This is a well-implemented feature with clean architecture. The main issues are:

  • Minor resource leak in HasCommands property (easy fix)
  • Incomplete help functionality (mentioned as WIP in commits)
  • Several places where exceptions are silently swallowed (consider adding logging)
  • Some defensive null checks could improve robustness

The code is production-ready with the resource leak fix applied. The other issues are minor and can be addressed in follow-up PRs.

Recommendation: ✅ Approve with suggested fixes for the HasCommands property and help functionality.


@github-actions
Copy link
Copy Markdown

Screenshot Tests: 3 captured · 4cf8f0b

01_empty-state.png 02_populated-state.png 03_edit-form-open.png
01_empty-state.png 02_populated-state.png 03_edit-form-open.png

workflow run

@github-actions github-actions bot removed the run-tests Maintainer toggled command to run tests on a PR label Feb 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FR: Accept timer-start requests via CLI Arguments (and communicate to already-running service if one exists)

1 participant