fix: clean up stale lock files and validate installation

[ashwin-ant]

• Remove ~/.claude/.locks before each retry attempt to prevent "another process is currently installing" errors after timeout
• Validate claude binary is in PATH before reporting success
• Add missing PATH setup to base-action/action.yml

🤖 Generated with Claude Code

[claude]

Performance & Logic Optimization: Consider moving the lock cleanup to only execute after a failed attempt rather than before every attempt (including the first). This would be more logically correct since locks only exist after a timeout/failure.

Suggested change

	rm -rf ~/.claude/.locks 2>/dev/null || true
	if command -v timeout &> /dev/null; then
	timeout 120 bash -c "curl -fsSL https://claude.ai/install.sh | bash -s -- $CLAUDE_CODE_VERSION" && break
	else
	curl -fsSL https://claude.ai/install.sh | bash -s -- "$CLAUDE_CODE_VERSION" && break
	fi
	if [ $attempt -eq 3 ]; then
	echo "Failed to install Claude Code after 3 attempts"
	exit 1
	fi
	# Clean up stale lock files after failure, before retry
	echo "Installation failed, cleaning up stale locks before retry..."
	rm -rf ~/.claude/.locks 2>/dev/null || true

[claude]

Documentation: The comment "Add to PATH and validate installation" is slightly misleading since the PATH setup already existed before this PR. Consider updating to better reflect that this is primarily adding validation:

Suggested change

	# Add to PATH and validate installation
	# Ensure PATH is updated and validate claude binary is accessible

[claude]

Enhancement: Consider adding debugging context to help diagnose installation failures in CI logs:

Suggested change

	if ! command -v claude &> /dev/null; then
	echo "Installation failed: claude binary not found in PATH"
	exit 1
	fi
	if ! command -v claude &> /dev/null; then
	echo "Installation failed: claude binary not found in PATH"
	echo "Current PATH: $PATH"
	echo "Contents of ~/.local/bin:"
	ls -la ~/.local/bin/ 2>&1 || echo "Directory does not exist"
	exit 1
	fi

[claude]

Performance & Logic Optimization: Same as in action.yml - consider moving lock cleanup to only execute after failures, not before the first attempt.

Suggested change

	rm -rf ~/.claude/.locks 2>/dev/null || true
	if command -v timeout &> /dev/null; then
	timeout 120 bash -c "curl -fsSL https://claude.ai/install.sh | bash -s -- $CLAUDE_CODE_VERSION" && break
	else
	curl -fsSL https://claude.ai/install.sh | bash -s -- "$CLAUDE_CODE_VERSION" && break
	fi
	if [ $attempt -eq 3 ]; then
	echo "Failed to install Claude Code after 3 attempts"
	exit 1
	fi
	# Clean up stale lock files after failure, before retry
	echo "Installation failed, cleaning up stale locks before retry..."
	rm -rf ~/.claude/.locks 2>/dev/null || true

[ar03]

The lock cleanup path may need adjustment. Community workarounds in issue #709 report success with ~/.local/state/claude/locks rather than ~/.claude/.locks.

From the comments:

• rm -rf ~/.local/state/claude/locks has 52+ positive reactions
• Confirmed working on Ubuntu, macOS, Fedora, and Windows (C:\Users{user}.local\state\claude)

Consider cleaning both paths to cover different Claude Code versions:

rm -rf ~/.local/state/claude/locks 2>/dev/null || true
rm -rf ~/.claude/.locks 2>/dev/null || true

[claude]

Code Review Summary

I've completed a comprehensive review using specialized review agents. Overall, this PR makes solid improvements to installation reliability, but there are a few noteworthy issues to consider:

Key Findings

Positives:

• ✅ Lock cleanup addresses real timeout-related failures
• ✅ Binary validation prevents silent installation failures
• ✅ Good fail-fast error handling
• ✅ Negligible performance overhead

Issues Identified:

1. Security (Medium) - Custom executable paths (lines 233-234 in action.yml, 159-160 in base-action/action.yml) are added to PATH without validation, creating potential PATH injection vulnerability. Should validate executable exists and is executable before adding to PATH.

2. Code Duplication (Important) - Installation logic is duplicated across both action files. Consider extracting to a shared shell script for maintainability.

3. Logic Optimization - Lock cleanup runs before every attempt including the first. More logical to only clean up after failures, before retries.

4. Documentation Inaccuracy - PR description states "Add missing PATH setup to base-action/action.yml", but PATH setup already existed. The PR adds validation after PATH setup, not the PATH setup itself.

5. Test Coverage Gap (Critical) - The installation logic has zero automated test coverage because it's embedded in YAML shell scripts. This is difficult to test but worth documenting as a known limitation.

Inline Comments

I've posted inline comments on specific lines with detailed recommendations and code suggestions. The most critical issue is the PATH injection vulnerability with custom executables.

Recommendation

The changes improve reliability significantly, but consider addressing the security issue before merging. The other improvements (duplication, testing, optimization) can be follow-up work.