Skip to content

Conversation

dachi-dev
Copy link

@dachi-dev dachi-dev commented Aug 25, 2025

This PR fixes critical issues with merge commit file detection that was causing incorrect fallbacks to API mode and adds comprehensive production safety features.

Key Issues Resolved

Git Show Bug Fixed

  • Removed --format=%n from all git show calls which was producing empty arrays ['', ''] on merge commits
  • Added merge-aware fallback using git diff --name-only <parent^1> for proper merge commit file detection

Customer Guidance:

  • Buildkite users: Keep CI_PIPELINE_SOURCE=merge_request_event override for GitLab-triggered pipelines
  • Squash merge opt-out: SOCKET_GIT_DISABLE_SQUASH_HEURISTIC=1 available if heuristic detection causes false positives (default: heuristic enabled)

Enhanced Warnings:

  • Octopus merges log a first-parent warning when 3+ parents detected
  • Clear messaging about limitations: "Using first-parent diff only - may not show all changes from all branches"

Detection Transparency:

  • Frozen DETECTION SUMMARY log schema for monitoring integration
  • Method tracking: mr-diff | merge-diff | single-commit-show
  • Git command logging for debugging support escalations

Production Safety:

  • Parent commit validation prevents accidental huge diffs
  • Graceful error handling with clear failure messages
  • Lazy loading for improved performance

What This Fixes

  • Empty file detection on merge commits (['', ''] → actual changed files)
  • Incorrect API mode fallbacks on merge commits
  • Missing transparency in file detection logic
  • No guardrails against dangerous Git operations

Real Detection Examples
Single Commit:
DETECTION SUMMARY: method=single-commit-show files=11 sha=095b0ccc cmd="git show --name-only 095b0ccc..."
Merge Commit:
DETECTION SUMMARY: method=merge-diff files=1 sha=b459b2e3 cmd="git diff --name-only 89ca8e3a..b459b2e3"

Testing

  • 21 new comprehensive unit tests added covering all detection scenarios
  • 19/21 tests passing (2 intentionally skipped with documentation)
  • Real squash merge and octopus merge detection tested with actual Git repositories
  • Environment variable integration testing for GitLab MR, GitHub PR, and Bitbucket contexts
  • All existing functionality preserved with no new test failures introduced

Public Changelog

N/A

🔧 Fixed Git Show Bug:
- Removed --format=%n from all git show calls (was producing ['', ''] empty arrays)
- Added merge-aware fallback: git diff --name-only <parent^1> <commit> for merge commits

🚀 Customer Guidance:
- Buildkite users: Keep CI_PIPELINE_SOURCE=merge_request_event override for GitLab-triggered pipelines
- Squash merge opt-out: SOCKET_GIT_DISABLE_SQUASH_HEURISTIC=1 available if heuristic detection causes false positives (default: heuristic enabled)

⚠️ Enhanced Warnings:
- Octopus merges log a first-parent warning when 3+ parents detected
- Clear messaging about limitations: 'Using first-parent diff only - may not show all changes from all branches'

📊 Detection Transparency:
- Frozen DETECTION SUMMARY log schema for monitoring integration
- Method tracking: mr-diff | merge-diff | single-commit-show
- Git command logging for debugging support escalations

🛡️ Production Safety:
- Parent commit validation prevents accidental huge diffs
- Graceful error handling with clear failure messages
- Lazy loading for improved performance

✅ What This Fixes:
- Empty file detection on merge commits (['', ''] → actual changed files)
- Incorrect API mode fallbacks on merge commits
- Missing transparency in file detection logic
- No guardrails against dangerous Git operations

Real Detection Examples:
- Single Commit: DETECTION SUMMARY: method=single-commit-show files=11 sha=095b0ccc cmd="git show --name-only 095b0cc..."
- Merge Commit: DETECTION SUMMARY: method=merge-diff files=1 sha=b459b2e3 cmd="git diff --name-only 89ca8e3a..b459b2e3"
@dachi-dev dachi-dev requested a review from a team as a code owner August 25, 2025 22:23
@dachi-dev dachi-dev requested review from BarrensZeppelin and hemanthkini and removed request for a team August 25, 2025 22:23
Copy link

github-actions bot commented Aug 25, 2025

🚀 Preview package published!

Install with:

pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple socketsecurity==2.3.0.dev1

Docker image: socketdev/cli:pr-115

Copy link
Collaborator

@dacoburn dacoburn left a comment

Choose a reason for hiding this comment

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

Looks fine but lets test with Github Actions and Gitlab Pipelines before merging.

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.

2 participants