Merge pull request #8 from nutthead/fix/workflows

ci: Improve and fix various issues in workflows

Author: behrangsa

.github/scripts/coverage-comment/package-lock.json

@@ -0,0 +1,10 @@
+{
+  "name": "coverage-comment",
+  "version": "1.0.0",
+  "description": "Dependencies for CI coverage PR comment script",
+  "private": true,
+  "dependencies": {
+    "fast-xml-parser": "5.2.5",
+    "dedent": "1.7.0"
+  }
+}

.github/scripts/coverage-comment/package.json

@@ -46,19 +46,17 @@ env:
   CARGO_TERM_COLOR: always  # Enable colored output in CI logs
   CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse  # Use sparse registry protocol
   RUST_BACKTRACE: short  # Provide backtraces without excessive noise
-  RUSTFLAGS: "-D warnings"  # Treat warnings as errors
 
 permissions:
   contents: read
-  pull-requests: write
   actions: read
-  checks: write
 
 jobs:
   # Fast-fail checks run before the expensive test matrix
   quick-check:
     name: Quick Checks
     runs-on: ubuntu-latest
+    timeout-minutes: 15
     steps:
       - name: Checkout repository
         uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
@@ -89,11 +87,26 @@ jobs:
         env:
           RUSTDOCFLAGS: "-D warnings"
 
+      - name: Validate changelog format
+        run: |
+          if [ ! -f "CHANGELOG.md" ]; then
+            echo "⚠️  CHANGELOG.md not found (will be created by release-plz)"
+            exit 0
+          fi
+
+          if ! grep -q "^# Changelog" CHANGELOG.md; then
+            echo "❌ Invalid changelog format - missing '# Changelog' header"
+            exit 1
+          fi
+
+          echo "✅ Changelog format is valid"
+
   # Security audit runs in parallel with quick-check
   # Continues on error since advisory warnings shouldn't block PRs
   security:
     name: Security Audit
     runs-on: ubuntu-latest
+    timeout-minutes: 20
     continue-on-error: true  # Advisory warnings are informational only
     steps:
       - name: Checkout repository
@@ -130,12 +143,13 @@ jobs:
         with:
           name: security-audit
           path: audit.json
-          retention-days: 30
+          retention-days: 90  # Long-term retention for compliance/security artifacts
 
   # Unit tests run across multiple platforms
   unit-tests:
     name: Unit Tests ${{ matrix.name }}
     needs: [quick-check]
+    timeout-minutes: 30
     strategy:
       fail-fast: false
       matrix:
@@ -187,14 +201,18 @@ jobs:
           sudo apt-get install -y musl-tools
 
       - name: Run unit tests
-        run: cargo test --locked --target ${{ matrix.target }} ${{ matrix.test_args }} -- --test-threads=1
+        run: cargo test --locked --target ${{ matrix.target }} ${{ matrix.test_args }}
 
   # Generates code coverage report using tarpaulin
   # Uploads to Codecov and posts PR comment with coverage summary
   coverage:
     name: Code Coverage
     needs: [unit-tests]
     runs-on: ubuntu-latest
+    timeout-minutes: 45
+    permissions:
+      contents: read
+      pull-requests: write  # Required to post coverage comments on PRs
     steps:
       - name: Checkout repository
         uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
@@ -214,6 +232,7 @@ jobs:
         uses: cargo-bins/cargo-binstall@38e8f5e4c386b611d51e8aa997b9a06a3c8eb67a # v1.15.6
 
       - name: Install and run tarpaulin
+        id: tarpaulin
         run: |
           cargo binstall --force --no-confirm --locked cargo-tarpaulin || cargo install --locked cargo-tarpaulin
 
@@ -222,14 +241,25 @@ jobs:
             cargo install --locked cargo-tarpaulin
           fi
 
-          # Run tarpaulin and capture exit code
-          # Use --no-fail-fast to ensure XML is generated even if coverage is low
-          cargo tarpaulin --timeout 120 --avoid-cfg-tarpaulin || {
-            EXIT_CODE=$?
-            echo "⚠️  Tarpaulin exited with code $EXIT_CODE (possibly due to coverage threshold)"
-            echo "Continuing to upload coverage data..."
-            exit 0  # Don't fail the step
-          }
+          # Run tarpaulin - differentiate between test failures and coverage threshold
+          set +e  # Don't exit script on error
+          cargo tarpaulin --timeout 120 --avoid-cfg-tarpaulin
+          EXIT_CODE=$?
+          set -e
+
+          # Store exit code for later evaluation
+          echo "exit_code=$EXIT_CODE" >> "$GITHUB_OUTPUT"
+
+          if [ $EXIT_CODE -eq 0 ]; then
+            echo "✅ Coverage passed (above threshold)"
+          elif [ $EXIT_CODE -eq 2 ]; then
+            echo "⚠️  Coverage below threshold - will fail after uploading report"
+            echo "Uploading report for analysis..."
+          else
+            echo "❌ Tarpaulin failed (exit code: $EXIT_CODE)"
+            echo "This indicates test failures or compilation errors"
+            exit $EXIT_CODE  # Fail immediately on real errors
+          fi
 
       - name: Upload coverage to Codecov
         if: always()
@@ -240,18 +270,35 @@ jobs:
           name: codecov-umbrella
           fail_ci_if_error: false
 
+      - name: Determine if PR comment should be posted
+        id: check-pr
+        if: always()
+        run: |
+          SHOULD_COMMENT="false"
+
+          # Check all required conditions
+          if [[ "${{ github.event_name }}" == "pull_request" ]] && \
+             [[ "${{ github.event.pull_request.head.repo.fork }}" == "false" ]] && \
+             [[ -f "target/tarpaulin/cobertura.xml" ]]; then
+            SHOULD_COMMENT="true"
+          fi
+
+          echo "should_comment=${SHOULD_COMMENT}" >> "$GITHUB_OUTPUT"
+          echo "PR comment needed: ${SHOULD_COMMENT}"
+
       - name: Install coverage parser
-        if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && hashFiles('target/tarpaulin/cobertura.xml') != ''
-        run: npm install fast-xml-parser@5.2.5 dedent@1.7.0 --no-save
+        if: always() && steps.check-pr.outputs.should_comment == 'true'
+        working-directory: .github/scripts/coverage-comment
+        run: npm ci
 
       - name: Comment coverage on PR
-        if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && hashFiles('target/tarpaulin/cobertura.xml') != ''
+        if: always() && steps.check-pr.outputs.should_comment == 'true'
         uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
         with:
           script: |
             const fs = require('fs');
-            const { XMLParser } = require('fast-xml-parser');
-            const dedent = require('dedent');
+            const { XMLParser } = require('.github/scripts/coverage-comment/node_modules/fast-xml-parser');
+            const dedent = require('.github/scripts/coverage-comment/node_modules/dedent');
 
             const parser = new XMLParser({
               ignoreAttributes: false,
@@ -342,23 +389,33 @@ jobs:
               body
             });
 
+      - name: Enforce coverage threshold
+        if: always() && steps.tarpaulin.outputs.exit_code == '2'
+        run: |
+          echo "❌ Coverage is below the required threshold (70%)"
+          echo "Review the coverage report uploaded to Codecov for details"
+          exit 1
+
   # Final status check required by branch protection
   ci-success:
     name: CI Success
-    if: always()
+    # Use GitHub's native conditional syntax for clearer intent
+    # Job runs only if all critical jobs pass and security doesn't fail
+    if: |
+      always() &&
+      needs.quick-check.result == 'success' &&
+      needs.unit-tests.result == 'success' &&
+      needs.coverage.result == 'success' &&
+      (needs.security.result == 'success' || needs.security.result == 'skipped')
     needs: [quick-check, security, unit-tests, coverage]
     runs-on: ubuntu-latest
+    timeout-minutes: 5
+    permissions:
+      contents: read
+      checks: write  # Needed for merge queue status checks
     steps:
-      - name: Check status
-        run: |
-          # Check all job results
-          if [[ "${{ needs.quick-check.result }}" != "success" ]] ||
-             [[ "${{ needs.unit-tests.result }}" != "success" ]] ||
-             [[ "${{ needs.coverage.result }}" != "success" ]]; then
-            echo "❌ CI failed"
-            exit 1
-          fi
-          echo "✅ All CI checks passed"
+      - name: Report success
+        run: echo "✅ All CI checks passed"
 
       - name: Set merge queue status
         if: github.event_name == 'merge_group'

.github/workflows/ci.yml

@@ -34,6 +34,7 @@ jobs:
   publish:
     name: Publish to crates.io
     runs-on: ubuntu-latest
+    timeout-minutes: 25
     steps:
       - name: Normalize version
         id: version
@@ -52,6 +53,15 @@ jobs:
           echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
           echo "Publishing version: ${VERSION}"
 
+      - name: Install dependencies
+        run: |
+          # Ensure jq is installed (used for JSON parsing)
+          if ! command -v jq >/dev/null 2>&1; then
+            echo "Installing jq..."
+            sudo apt-get update && sudo apt-get install -y jq
+          fi
+          jq --version
+
       - name: Check if release exists
         run: |
           VERSION="${{ steps.version.outputs.version }}"
@@ -105,7 +115,8 @@ jobs:
       - name: Verify Cargo.toml version matches
         run: |
           VERSION="${{ steps.version.outputs.version }}"
-          CARGO_VERSION=$(grep '^version' Cargo.toml | head -1 | cut -d'"' -f2)
+          # Use cargo metadata for robust version parsing
+          CARGO_VERSION=$(cargo metadata --format-version 1 --no-deps | jq -r '.packages[0].version')
 
           if [ "$VERSION" != "$CARGO_VERSION" ]; then
             echo "❌ Version mismatch!"

.github/workflows/publish-crate.yml

@@ -30,6 +30,7 @@ jobs:
   release-tag:
     name: Create Release Tag
     runs-on: ubuntu-latest
+    timeout-minutes: 15
     # Prevent infinite loops from github-actions bot or [skip ci]
     if: |
       github.event.pusher.name != 'github-actions[bot]' &&
@@ -46,9 +47,18 @@ jobs:
       - name: Check for version change
         id: check
         run: |
+          # Check if HEAD~1 exists (handles first commit case)
+          if ! git rev-parse HEAD~1 >/dev/null 2>&1; then
+            echo "First commit detected - no previous version to compare"
+            echo "should_release=false" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+
           if git diff HEAD~1 HEAD --name-only | grep -q "^Cargo.toml$"; then
+            # Previous version: use grep on git show output (historical file)
             PREV_VERSION=$(git show HEAD~1:Cargo.toml | grep '^version' | head -1 | cut -d'"' -f2)
-            CURR_VERSION=$(grep '^version' Cargo.toml | head -1 | cut -d'"' -f2)
+            # Current version: use cargo metadata for robust parsing
+            CURR_VERSION=$(cargo metadata --format-version 1 --no-deps | jq -r '.packages[0].version')
 
             if [[ "$PREV_VERSION" != "$CURR_VERSION" ]]; then
               echo "Version changed from $PREV_VERSION to $CURR_VERSION"
@@ -84,6 +94,5 @@ jobs:
         uses: MarcoIeni/release-plz-action@acb9246af4d59a270d1d4058a8b9af8c3f3a2559 # v0.5.117
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }}
         with:
           command: release

.github/workflows/release-plz.yml

@@ -17,7 +17,6 @@ on:
       - 'Cargo.toml'
       - 'Cargo.lock'
       - '.github/workflows/release-pr.yml'
-      - '.tarpaulin.toml'
 
 permissions:
   contents: write
@@ -32,6 +31,7 @@ jobs:
   create-release-pr:
     name: Create Release PR
     runs-on: ubuntu-latest
+    timeout-minutes: 20
     # Skip if commit message contains [skip ci] to prevent unnecessary runs
     if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }}
     steps: