From 75ca28acabc9ff2bc8e7ead86b0d01c65fb108cb Mon Sep 17 00:00:00 2001 From: Daniel Chalef <131175+danielchalef@users.noreply.github.com> Date: Thu, 30 Oct 2025 23:37:01 -0700 Subject: [PATCH] Address code review findings - fix critical checkout ref bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix all critical and high-priority issues from code review: 1. Fix checkout ref logic (CRITICAL) - Simplified to: ref: ${{ inputs.tag || github.ref }} - Works correctly for both workflow_dispatch and push events - Removes conditional logic that would fail for manual triggers 2. Consolidate tag validation - Remove duplicate validation logic - Single validation path for both trigger types - Clearer error messages with received value 3. Add PyPI error handling - Use curl -sf for proper error codes - Validate GRAPHITI_VERSION is not empty - Exit with clear error if PyPI fetch fails 4. Improve docker-compose comments - Add concrete version tag examples - Show users how to pin specific versions - Clarify when local build vs registry pull is used 5. Update workflow_dispatch description - Clarify tag must already exist in repo - Prevent user confusion about tag creation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .github/workflows/release-mcp-server.yml | 30 +++++++++++-------- mcp_server/docker/docker-compose-falkordb.yml | 5 ++-- mcp_server/docker/docker-compose-neo4j.yml | 5 ++-- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/.github/workflows/release-mcp-server.yml b/.github/workflows/release-mcp-server.yml index f3ffcefc..76f29c24 100644 --- a/.github/workflows/release-mcp-server.yml +++ b/.github/workflows/release-mcp-server.yml @@ -6,7 +6,7 @@ on: workflow_dispatch: inputs: tag: - description: 'Tag to release (e.g., mcp-v1.0.0)' + description: 'Existing tag to release (e.g., mcp-v1.0.0) - tag must exist in repo' required: true type: string @@ -41,7 +41,7 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 with: - ref: ${{ github.event_name == 'workflow_dispatch' && inputs.tag || github.ref }} + ref: ${{ inputs.tag || github.ref }} - name: Set up Python 3.11 uses: actions/setup-python@v5 @@ -51,29 +51,26 @@ jobs: - name: Extract and validate version id: version run: | - # Get tag from either push event or manual workflow_dispatch input + # Extract tag from either push event or manual workflow_dispatch input if [ "${{ github.event_name }}" == "workflow_dispatch" ]; then TAG_FULL="${{ inputs.tag }}" - # Validate tag format for manual triggers - if [[ ! $TAG_FULL =~ ^mcp-v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then - echo "Error: Tag must follow format: mcp-vX.Y.Z (e.g., mcp-v1.0.0)" - echo "Received: $TAG_FULL" - exit 1 - fi TAG_VERSION=${TAG_FULL#mcp-v} else TAG_VERSION=${GITHUB_REF#refs/tags/mcp-v} fi + # Validate semantic versioning format if ! [[ $TAG_VERSION =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then - echo "Tag must follow semantic versioning: mcp-vX.Y.Z" + echo "Error: Tag must follow semantic versioning: mcp-vX.Y.Z (e.g., mcp-v1.0.0)" + echo "Received: mcp-v$TAG_VERSION" exit 1 fi + # Validate against pyproject.toml version PROJECT_VERSION=$(python -c "import tomllib; print(tomllib.load(open('mcp_server/pyproject.toml', 'rb'))['project']['version'])") if [ "$TAG_VERSION" != "$PROJECT_VERSION" ]; then - echo "Tag version mcp-v$TAG_VERSION does not match mcp_server/pyproject.toml version $PROJECT_VERSION" + echo "Error: Tag version mcp-v$TAG_VERSION does not match mcp_server/pyproject.toml version $PROJECT_VERSION" exit 1 fi @@ -92,8 +89,15 @@ jobs: - name: Get latest graphiti-core version from PyPI id: graphiti run: | - # Query PyPI for the latest graphiti-core version - GRAPHITI_VERSION=$(curl -s https://pypi.org/pypi/graphiti-core/json | python -c "import sys, json; print(json.load(sys.stdin)['info']['version'])") + # Query PyPI for the latest graphiti-core version with error handling + set -e + GRAPHITI_VERSION=$(curl -sf https://pypi.org/pypi/graphiti-core/json | python -c "import sys, json; data=json.load(sys.stdin); print(data['info']['version'])" 2>&1) + + if [ -z "$GRAPHITI_VERSION" ] || [ "$?" -ne 0 ]; then + echo "Error: Failed to fetch graphiti-core version from PyPI" + exit 1 + fi + echo "graphiti_version=${GRAPHITI_VERSION}" >> $GITHUB_OUTPUT echo "Latest Graphiti Core version from PyPI: ${GRAPHITI_VERSION}" diff --git a/mcp_server/docker/docker-compose-falkordb.yml b/mcp_server/docker/docker-compose-falkordb.yml index 513207c3..1f99cba2 100644 --- a/mcp_server/docker/docker-compose-falkordb.yml +++ b/mcp_server/docker/docker-compose-falkordb.yml @@ -17,8 +17,9 @@ services: graphiti-mcp: image: zepai/knowledge-graph-mcp:standalone - # Note: When building locally, this will use the local build. - # For production, pull from registry with the version tag you need + # For specific versions, replace 'standalone' with a version tag: + # image: zepai/knowledge-graph-mcp:1.0.0-standalone + # When building locally, the build section below will be used. build: context: .. dockerfile: docker/Dockerfile.standalone diff --git a/mcp_server/docker/docker-compose-neo4j.yml b/mcp_server/docker/docker-compose-neo4j.yml index 069663d4..ea911214 100644 --- a/mcp_server/docker/docker-compose-neo4j.yml +++ b/mcp_server/docker/docker-compose-neo4j.yml @@ -21,8 +21,9 @@ services: graphiti-mcp: image: zepai/knowledge-graph-mcp:standalone - # Note: When building locally, this will use the local build. - # For production, pull from registry with the version tag you need + # For specific versions, replace 'standalone' with a version tag: + # image: zepai/knowledge-graph-mcp:1.0.0-standalone + # When building locally, the build section below will be used. build: context: .. dockerfile: docker/Dockerfile.standalone