Skip to content

Fix check_update_lock() to provide user guidance for potentially stale lock files#133

Merged
paolomainardi merged 3 commits into
masterfrom
copilot/fix-132
Aug 18, 2025
Merged

Fix check_update_lock() to provide user guidance for potentially stale lock files#133
paolomainardi merged 3 commits into
masterfrom
copilot/fix-132

Conversation

Copilot AI commented Aug 18, 2025

Copy link
Copy Markdown
Contributor

User description

The check_update_lock() function in bin/sparkdock.macos was missing handling for a specific edge case where a lock file exists but the process holding it is no longer running (and the lock file is less than 1 hour old).

Problem

The function handled two scenarios correctly:

  • Lock files older than 1 hour → automatically removed
  • Active processes → displays warning and exits

However, it was missing the case where:

  • Lock file exists with a PID of a process that's no longer running
  • Lock file is less than 1 hour old (so not auto-removed)

In this scenario, users would encounter a "stuck" lock file with no guidance on how to resolve it manually.

Solution

Added an else clause to detect this scenario and provide informative guidance:

else
    print_warning "Lock file exists but the process (PID: $pid) is no longer running."
    print_warning "If you believe this is a stale lock file, you can remove it manually by running:"
    print_warning "  rm -f $LOCK_FILE"
    exit 1

Behavior Change

Before: Lock files with non-running processes would cause the function to continue execution, potentially creating unexpected behavior.

After: Users receive clear information about the situation and conditional guidance on manual resolution, allowing them to make an informed decision about whether to remove /tmp/sparkdock.lock.

The change ensures users are never left without guidance when encountering potentially stuck lock files, while avoiding assumptions about the lock file's validity.

Fixes #132.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.


PR Type

Bug fix


Description

  • Added handling for stale lock files with dead processes

  • Provides manual removal instructions for stuck lock files

  • Prevents silent continuation when lock process is dead

  • Improves user experience with clear guidance messages


Changes walkthrough 📝

Relevant files
Bug fix
sparkdock.macos
Handle stale lock files with user guidance                             

bin/sparkdock.macos

  • Added else clause to handle dead process lock files
  • Added warning messages with manual fix instructions
  • Ensures proper exit when lock file is stale
  • +5/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Co-authored-by: paolomainardi <8747+paolomainardi@users.noreply.github.com>
    Copilot AI changed the title [WIP] check_update_lock() print how to fix manually if lock file gets stucked Fix check_update_lock() to display manual fix instructions for stale lock files Aug 18, 2025
    Copilot AI requested a review from paolomainardi August 18, 2025 20:37
    Comment thread bin/sparkdock.macos
    …view feedback
    
    Co-authored-by: paolomainardi <8747+paolomainardi@users.noreply.github.com>
    Copilot AI changed the title Fix check_update_lock() to display manual fix instructions for stale lock files Fix check_update_lock() to provide user guidance for potentially stale lock files Aug 18, 2025
    Copilot AI requested a review from paolomainardi August 18, 2025 20:42
    @paolomainardi paolomainardi marked this pull request as ready for review August 18, 2025 20:43
    Copilot AI review requested due to automatic review settings August 18, 2025 20:43

    Copilot AI left a comment

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Copilot wasn't able to review any files in this pull request.


    You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

    @paolomainardi paolomainardi merged commit efa1b8e into master Aug 18, 2025
    3 checks passed
    @sparkfabrik-ai-bot

    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    132 - Fully compliant

    Compliant requirements:

    • Print how to fix manually if lock file gets stuck
    • Provide guidance when lock file becomes stale
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Logic Gap

    The added else clause handles dead processes but doesn't verify if the lock file is actually stale. It provides removal instructions for any dead process regardless of how recently the lock was created, which could lead to premature removal of legitimate locks from recently crashed processes.

    else
        print_warning "Lock file exists but the process (PID: $pid) is no longer running."
        print_warning "If you believe this is a stale lock file, you can remove it manually by running:"
        print_warning "  rm -f $LOCK_FILE"
        exit 1

    @sparkfabrik-ai-bot

    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Quote variable in shell command

    The variable $LOCK_FILE should be quoted to prevent word splitting and pathname
    expansion. This ensures the command works correctly even if the path contains spaces
    or special characters.

    bin/sparkdock.macos [67-71]

     else
         print_warning "Lock file exists but the process (PID: $pid) is no longer running."
         print_warning "If you believe this is a stale lock file, you can remove it manually by running:"
    -    print_warning "  rm -f $LOCK_FILE"
    +    print_warning "  rm -f \"$LOCK_FILE\""
         exit 1
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly identifies that $LOCK_FILE should be quoted in the warning message to prevent word splitting. This is a good shell scripting practice that improves robustness when paths contain spaces or special characters.

    Low

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

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    check_update_lock() print how to fix manually if lock file gets stucked

    3 participants