Skip to content

ci#19

Merged
N6REJ merged 13 commits intomainfrom
ci
Nov 23, 2025
Merged

ci#19
N6REJ merged 13 commits intomainfrom
ci

Conversation

@N6REJ
Copy link
Contributor

@N6REJ N6REJ commented Nov 23, 2025

PR Type

Enhancement, Tests


Description

  • Add PostgreSQL 13.23, 14.20, 15.15, 16.11, 17.7, 18.1 configurations

  • Implement comprehensive CI/CD testing workflow for PostgreSQL versions

    • Smart PR version detection using GitHub API
    • Automated test result reporting with status indicators
    • Multi-phase validation: download, extract, verify executables
  • Add detailed documentation for CI/CD testing procedures

  • Include configuration files for each PostgreSQL version


Diagram Walkthrough

flowchart LR
  A["PostgreSQL Versions<br/>13.23-18.1"] -->|"Configuration Files"| B["bearsampp.conf<br/>postgresql.conf.ber<br/>pg_hba.conf.ber<br/>init.bat"]
  C["PR Changes"] -->|"GitHub API Detection"| D["detect-versions Job"]
  D -->|"Version Matrix"| E["test-postgresql Job"]
  E -->|"Download & Extract"| F["Phase 1.1"]
  F -->|"Verify Executables"| G["Phase 1.2"]
  G -->|"Test Functionality"| H["Phase 2"]
  H -->|"Generate Results"| I["report-results Job"]
  I -->|"PR Comment"| J["Test Summary"]
Loading

File Walkthrough

Relevant files
Enhancement
1 files
postgresql-test.yml
Comprehensive PostgreSQL CI/CD testing workflow                   
+544/-0 
Documentation
2 files
CI-CD-TESTING.md
Complete CI/CD testing documentation and guide                     
+295/-0 
README.md
Documentation index for PostgreSQL module                               
+36/-0   
Configuration changes
24 files
init.bat
Database initialization script for PostgreSQL 13.23           
+5/-0     
init.bat
Database initialization script for PostgreSQL 14.20           
+5/-0     
init.bat
Database initialization script for PostgreSQL 15.15           
+5/-0     
init.bat
Database initialization script for PostgreSQL 16.11           
+5/-0     
init.bat
Database initialization script for PostgreSQL 17.7             
+5/-0     
init.bat
Database initialization script for PostgreSQL 18.1             
+5/-0     
postgresql.conf.ber
PostgreSQL configuration template for version 13.23           
+35/-0   
postgresql.conf.ber
PostgreSQL configuration template for version 14.20           
+35/-0   
postgresql.conf.ber
PostgreSQL configuration template for version 15.15           
+35/-0   
postgresql.conf.ber
PostgreSQL configuration template for version 16.11           
+35/-0   
postgresql.conf.ber
PostgreSQL configuration template for version 17.7             
+35/-0   
postgresql.conf.ber
PostgreSQL configuration template for version 18.1             
+35/-0   
bearsampp.conf
Bearsampp module configuration for PostgreSQL 13.23           
+14/-0   
bearsampp.conf
Bearsampp module configuration for PostgreSQL 14.20           
+14/-0   
bearsampp.conf
Bearsampp module configuration for PostgreSQL 15.15           
+14/-0   
bearsampp.conf
Bearsampp module configuration for PostgreSQL 16.11           
+14/-0   
bearsampp.conf
Bearsampp module configuration for PostgreSQL 17.7             
+14/-0   
bearsampp.conf
Bearsampp module configuration for PostgreSQL 18.1             
+14/-0   
pg_hba.conf.ber
Host-based authentication configuration for 13.23               
+3/-0     
pg_hba.conf.ber
Host-based authentication configuration for 14.20               
+3/-0     
pg_hba.conf.ber
Host-based authentication configuration for 15.15               
+3/-0     
pg_hba.conf.ber
Host-based authentication configuration for 16.11               
+3/-0     
pg_hba.conf.ber
Host-based authentication configuration for 17.7                 
+3/-0     
pg_hba.conf.ber
Host-based authentication configuration for 18.1                 
+3/-0     
Miscellaneous
1 files
cache.properties
Gradle build cache properties file                                             
+2/-0     
Additional files
4 files
gc.properties [link]   
gc.properties [link]   
gc.properties [link]   
problems-report.html +659/-0 

@github-actions
Copy link

github-actions bot commented Nov 23, 2025

🐘 PostgreSQL Module Tests - Results

Test Date: 2025-11-23 11:03:53 UTC
Status: ✅ All tests passed

Results: 5 of 5 versions tested

PostgreSQL 17.0-RC1

Phase 1: Installation Validation

  • Download & Extract: ✅ PASS
  • Verify Executables: ✅ PASS

Phase 2: Basic Functionality

  • Test Executables: ✅ PASS

Overall Status: ✅ ALL TESTS PASSED

PostgreSQL 17.2.1

Phase 1: Installation Validation

  • Download & Extract: ✅ PASS
  • Verify Executables: ✅ PASS

Phase 2: Basic Functionality

  • Test Executables: ✅ PASS

Overall Status: ✅ ALL TESTS PASSED

PostgreSQL 17.2.3

Phase 1: Installation Validation

  • Download & Extract: ✅ PASS
  • Verify Executables: ✅ PASS

Phase 2: Basic Functionality

  • Test Executables: ✅ PASS

Overall Status: ✅ ALL TESTS PASSED

PostgreSQL 17.4

Phase 1: Installation Validation

  • Download & Extract: ✅ PASS
  • Verify Executables: ✅ PASS

Phase 2: Basic Functionality

  • Test Executables: ✅ PASS

Overall Status: ✅ ALL TESTS PASSED

PostgreSQL 17.5

Phase 1: Installation Validation

  • Download & Extract: ✅ PASS
  • Verify Executables: ✅ PASS

Phase 2: Basic Functionality

  • Test Executables: ✅ PASS

Overall Status: ✅ ALL TESTS PASSED


📋 Test Phases

Each version is tested through the following phases:

  • Phase 1: Installation Validation (Download, Extract, Verify Executables)
  • Phase 2: Basic Functionality (Test Executable Versions)

Check artifacts for detailed logs.

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 23, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Insecure auth configuration

Description: The database cluster is initialized with "-A trust" (no authentication), which allows
passwordless local access to the superuser and is insecure if used beyond isolated CI;
this pattern appears in all added init.bat scripts across versions.
init.bat [3-3]

Referred Code
%~dp0bin\initdb.exe -U postgres -A trust -E utf8 -D "%~dp0data" > "~BEARSAMPP_WIN_PATH~\logs\postgresql-install.log" 2>&1
copy /y "%~dp0postgresql.conf.ber" "%~dp0data\postgresql.conf"
Insecure pg_hba trust

Description: pg_hba.conf templates allow "trust" authentication for all users/databases on 127.0.0.1/32
and ::1/128, which disables password checks and is unsafe if these templates are deployed
to non-test environments; this is duplicated across all versioned pg_hba.conf.ber files.
pg_hba.conf.ber [2-3]

Referred Code
host        all           all             127.0.0.1/32            trust
host 		    all           all              ::1/128	      				trust
World-writable logs

Description: Logging is configured with log_file_mode=0777, granting world-writable permissions to
logs, which can enable log tampering or privilege misuse if used outside ephemeral CI
contexts; this is present in all versioned postgresql.conf.ber files.
postgresql.conf.ber [23-31]

Referred Code
log_destination = 'stderr'
logging_collector = on
log_directory = '~BEARSAMPP_LIN_PATH~/logs'
log_filename = 'postgresql.log'
log_file_mode = 0777
log_truncate_on_rotation = off
log_rotation_age = 0
log_rotation_size = 0
CI server exposure

Description: The workflow starts a PostgreSQL server with trust authentication and runs as the default
superuser without isolating network exposure (listen_addresses may default to all
interfaces in configs), which could risk unauthorized access on shared runners if binding
becomes non-local; verify it binds only to localhost during CI.
postgresql-test.yml [318-336]

Referred Code
Write-Host "Starting PostgreSQL server..."
$output = & $pgCtlExe -D $dataDir -l $logFile start 2>&1 | Out-String
Write-Host $output

# Wait for server to be ready
Write-Host "Waiting for server to be ready..."
Start-Sleep -Seconds 5

# Check server status
$statusOutput = & $pgCtlExe -D $dataDir status 2>&1 | Out-String
Write-Host $statusOutput

if ($statusOutput -match "server is running") {
  Write-Host "✅ PostgreSQL server is running"
  echo "success=true" >> $env:GITHUB_OUTPUT
  echo "log-file=$logFile" >> $env:GITHUB_OUTPUT
} else {
  Write-Host "❌ Server is not running"
  if (Test-Path $logFile) {
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Insecure logging: PostgreSQL is configured to log to a shared logs directory with log_file_mode 0777, which
is overly permissive and risks exposure of sensitive operational data.

Referred Code
log_destination = 'stderr'
logging_collector = on
log_directory = '~BEARSAMPP_LIN_PATH~/logs'
log_filename = 'postgresql.log'
log_file_mode = 0777
log_truncate_on_rotation = off
log_rotation_age = 0
log_rotation_size = 0

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Trust authentication: The pg_hba configuration uses trust authentication for localhost which disables password
checks and creates a security risk even in testing contexts.

Referred Code
# TYPE      DATABASE        USER            ADDRESS                 METHOD
  host        all           all             127.0.0.1/32            trust
  host 		    all           all              ::1/128	      				trust

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Lacks audit logs: The new CI and init scripts perform critical actions (download, initdb, start/stop server,
DB create/drop) without emitting structured audit logs that include user ID and explicit
action outcomes.

Referred Code
- name: Phase 1.1 - Download PostgreSQL
  id: download-postgresql
  continue-on-error: true
  run: |
    $ErrorActionPreference = "Continue"
    $version = "${{ matrix.version }}"

    Write-Host "=== Phase 1.1: Download PostgreSQL $version ==="

    # Read releases.properties to get download URL
    $releasesFile = "releases.properties"
    $downloadUrl = ""

    if (Test-Path $releasesFile) {
      $content = Get-Content $releasesFile
      foreach ($line in $content) {
        if ($line -match "^$version\s*=\s*(.+)$") {
          $downloadUrl = $matches[1].Trim()
          break
        }
      }


 ... (clipped 408 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Partial error context: While many steps capture errors, some messages are generic and inputs like release URL or
archive structure aren’t fully validated across all branches, risking silent assumptions
in edge cases.

Referred Code
Write-Host "Download URL: $downloadUrl"

try {
  $fileName = [System.IO.Path]::GetFileName($downloadUrl)
  $downloadPath = Join-Path "test-postgresql" $fileName

  Write-Host "Downloading PostgreSQL $version..."
  Write-Host "Target file: $downloadPath"

  try {
    Invoke-WebRequest -Uri $downloadUrl -OutFile $downloadPath -UseBasicParsing -TimeoutSec 300
  } catch {
    Write-Host "❌ ERROR: Download failed!"
    Write-Host "Error details: $($_.Exception.Message)"
    Write-Host "Status Code: $($_.Exception.Response.StatusCode.value__)"
    Write-Host "URL attempted: $downloadUrl"
    echo "success=false" >> $env:GITHUB_OUTPUT
    echo "error=Download failed: $($_.Exception.Message)" >> $env:GITHUB_OUTPUT
    exit 1
  }



 ... (clipped 67 lines)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 23, 2025

PR Code Suggestions ✨

Latest suggestions up to 1f7341d

CategorySuggestion                                                                                                                                    Impact
Possible issue
Validate executables actually run

Enhance the executable verification step to not only check for file existence
but also attempt to run each executable with --version to detect runtime issues
like missing DLLs.

.github/workflows/postgresql-test.yml [301-316]

 $requiredExes = @("postgres.exe", "psql.exe", "pg_ctl.exe", "initdb.exe", "createdb.exe", "dropdb.exe")
-...
+$allFound = $true
+$verifyResults = @{}
+
 foreach ($exe in $requiredExes) {
   $exePath = Join-Path $binPath $exe
   if (Test-Path $exePath) {
     Write-Host "✅ Found: $exe"
     $verifyResults[$exe] = @{ found = $true; path = $exePath }
+    try {
+      $null = & $exePath --version 2>&1
+      if ($LASTEXITCODE -ne 0) {
+        Write-Host "❌ $exe failed to run (DLL/dependency issue?)"
+        $verifyResults[$exe]["runnable"] = $false
+        $allFound = $false
+      } else {
+        $verifyResults[$exe]["runnable"] = $true
+      }
+    } catch {
+      Write-Host "❌ $exe failed to start: $($_.Exception.Message)"
+      $verifyResults[$exe]["runnable"] = $false
+      $allFound = $false
+    }
   } else {
     Write-Host "❌ Missing: $exe"
-    $verifyResults[$exe] = @{ found = $false }
+    $verifyResults[$exe] = @{ found = $false; runnable = $false }
     $allFound = $false
   }
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: This is a valuable improvement that makes the tests more robust by verifying not just the existence of executables but also their ability to run, which can catch critical packaging issues like missing DLLs early.

Medium
Validate matrix JSON output

Add a validation step using jq to ensure the VERSIONS variable contains valid
JSON before it is passed to the job matrix, preventing potential workflow
failures.

.github/workflows/postgresql-test.yml [131-132]

+# Validate versions JSON
+if ! echo "$VERSIONS" | jq -e . >/dev/null 2>&1; then
+  echo "❌ ERROR: Computed versions is not valid JSON: $VERSIONS"
+  echo "has-changes=false" >> $GITHUB_OUTPUT
+  echo "versions=[]" >> $GITHUB_OUTPUT
+  exit 1
+fi
+
 echo "versions=$VERSIONS" >> $GITHUB_OUTPUT
 echo "Versions to test: $VERSIONS"
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential failure point where invalid JSON in the VERSIONS variable could break the workflow and proposes a robust validation step to prevent it.

Low
Verify 7-Zip availability first

Add a check to verify that the 7z command is available on the runner before
attempting to use it for extraction, and provide a clear error if it is not
found.

.github/workflows/postgresql-test.yml [227-273]

+# Ensure 7z is available
+if (-not (Get-Command 7z -ErrorAction SilentlyContinue)) {
+  Write-Host "❌ ERROR: 7-Zip (7z) is not available on the runner."
+  Write-Host "Install 7zip or switch to Expand-Archive for .zip files."
+  echo "success=false" >> $env:GITHUB_OUTPUT
+  echo "error=7-Zip not found on runner" >> $env:GITHUB_OUTPUT
+  exit 1
+}
+
 $extractOutput = & 7z x $downloadPath -o"test-postgresql" -y 2>&1
-
 if ($LASTEXITCODE -eq 0) {
   Write-Host "✅ Extraction successful"
   ...
 } else {
   Write-Host "❌ ERROR: Extraction failed with exit code: $LASTEXITCODE"
   Write-Host "7z output:"
   Write-Host $extractOutput
   echo "success=false" >> $env:GITHUB_OUTPUT
   echo "error=Extraction failed with exit code $LASTEXITCODE" >> $env:GITHUB_OUTPUT
   exit 1
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 4

__

Why: The suggestion improves robustness by adding a pre-check for the 7z executable, which provides a clearer error message if it's missing, even though it's pre-installed on the specified runner.

Low
Security
Prevent potential token exposure

Replace the curl command with the gh api CLI to securely fetch PR file details,
avoiding potential exposure of the GITHUB_TOKEN.

.github/workflows/postgresql-test.yml [80-83]

-PATCH=$(curl -s -H "Authorization: token ${{ github.token }}" \
-  -H "Accept: application/vnd.github.v3+json" \
-  "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
-  jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || echo "")
+PATCH=$(
+  gh api \
+    repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files \
+    --jq '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null \
+  || echo ""
+)
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a security best practice violation by passing the github.token directly to curl and proposes a more secure alternative using the gh CLI.

Medium
General
Guard nullable response access

Safely access the exception's response status code to prevent a null reference
error in cases like DNS failures, ensuring more reliable error reporting.

.github/workflows/postgresql-test.yml [206]

-Write-Host "Status Code: $($_.Exception.Response.StatusCode.value__)"
+$statusCode = $null
+try { $statusCode = $_.Exception.Response.StatusCode.value__ } catch { $statusCode = "N/A" }
+Write-Host "Status Code: $statusCode"
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a potential null reference exception when accessing the status code and provides a safe way to handle it, improving the script's error handling robustness.

Low
  • More

Previous suggestions

✅ Suggestions up to commit dba06de
CategorySuggestion                                                                                                                                    Impact
General
Robustly detect extracted install directory

Improve the detection of the PostgreSQL installation directory by recursively
searching for bin\postgres.exe instead of relying on a fixed top-level folder
name like postgresql
.
*

.github/workflows/postgresql-test.yml [221-231]

+# Extract the archive
+Write-Host "Extracting archive..."
 $extractOutput = & 7z x $downloadPath -o"test-postgresql" -y 2>&1
-...
-$pgDir = Get-ChildItem -Path "test-postgresql" -Directory | Where-Object { $_.Name -match "^postgresql" } | Select-Object -First 1
 
+if ($LASTEXITCODE -eq 0) {
+  Write-Host "✅ Extraction successful"
+
+  # Try to find a directory containing bin\postgres.exe
+  $candidateDirs = Get-ChildItem -Path "test-postgresql" -Directory -Recurse -ErrorAction SilentlyContinue
+  $pgDir = $null
+  foreach ($dir in $candidateDirs) {
+    $postgresExe = Join-Path $dir.FullName "bin\postgres.exe"
+    if (Test-Path $postgresExe) {
+      $pgDir = $dir
+      break
+    }
+  }
+
+  if (-not $pgDir) {
+    # Fallback to top-level postgresql* directory
+    $pgDir = Get-ChildItem -Path "test-postgresql" -Directory | Where-Object { $_.Name -match "^postgresql" } | Select-Object -First 1
+  }
+
+  if ($pgDir) {
+    $pgPath = $pgDir.FullName
+    Write-Host "✅ PostgreSQL directory found: $pgPath"
+    $binPath = Join-Path $pgPath "bin"
+    if (Test-Path $binPath) {
+      Write-Host "✅ bin directory exists"
+      echo "pg-path=$pgPath" >> $env:GITHUB_OUTPUT
+      echo "success=true" >> $env:GITHUB_OUTPUT
+    } else {
+      Write-Host "❌ ERROR: bin directory not found in $pgPath"
+      echo "success=false" >> $env:GITHUB_OUTPUT
+      echo "error=bin directory not found in extracted archive" >> $env:GITHUB_OUTPUT
+      exit 1
+    }
+  } else {
+    Write-Host "❌ ERROR: PostgreSQL directory not found after extraction"
+    echo "success=false" >> $env:GITHUB_OUTPUT
+    echo "error=PostgreSQL directory not found after extraction" >> $env:GITHUB_OUTPUT
+    exit 1
+  }
+} else {
+  Write-Host "❌ ERROR: Extraction failed with exit code: $LASTEXITCODE"
+  Write-Host "7z output:"
+  Write-Host $extractOutput
+  echo "success=false" >> $env:GITHUB_OUTPUT
+  echo "error=Extraction failed with exit code $LASTEXITCODE" >> $env:GITHUB_OUTPUT
+  exit 1
+}
+
Suggestion importance[1-10]: 8

__

Why: This suggestion significantly improves the robustness of the test workflow by replacing a brittle directory search (^postgresql*) with a more reliable method that searches for a key executable (bin\postgres.exe), making the script resilient to variations in archive packaging.

Medium
Validate 7z signature before extraction

Enhance download validation by checking the file signature (magic bytes) to
confirm it is a valid 7z archive, rather than just checking its size.

.github/workflows/postgresql-test.yml [212-217]

-if ($fileSize -lt 0.1) {
-  Write-Host "❌ ERROR: Downloaded file is too small ($([math]::Round($fileSize, 2)) MB), likely corrupted"
+# Quick file signature check for 7z (magic bytes: 37 7A BD 27)
+$fs = [System.IO.File]::OpenRead($downloadPath)
+try {
+  $buffer = New-Object byte[] 4
+  $read = $fs.Read($buffer, 0, 4)
+  $is7z = ($read -eq 4) -and ($buffer[0] -eq 0x37) -and ($buffer[1] -eq 0x7A) -and ($buffer[2] -eq 0xBC) -and ($buffer[3] -eq 0xAF -or $buffer[3] -eq 0x27)
+} finally {
+  $fs.Dispose()
+}
+
+if (-not $is7z) {
+  Write-Host "❌ ERROR: Downloaded file does not appear to be a valid 7z archive (bad signature)"
   echo "success=false" >> $env:GITHUB_OUTPUT
-  echo "error=Downloaded file is too small or corrupted" >> $env:GITHUB_OUTPUT
+  echo "error=Downloaded file is not a 7z archive" >> $env:GITHUB_OUTPUT
   exit 1
 }
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that file size is a weak validation method and proposes a more robust check by verifying the file's magic bytes, which prevents attempting to extract invalid or corrupted archives.

Low
Make artifact counting shell-safe

Make the file counting in the shell script more robust by using find ... -print0
| xargs ... | wc -l to avoid potential issues with whitespace and ensure
cross-platform compatibility.

.github/workflows/postgresql-test.yml [466]

-ACTUAL_COUNT=$(find all-results -name "summary.md" 2>/dev/null | wc -l)
+if [ -d "all-results" ]; then
+  EXPECTED_COUNT=$(echo '${{ needs.detect-versions.outputs.versions }}' | jq '. | length')
+  ACTUAL_COUNT=$(find all-results -type f -name "summary.md" -print0 | xargs -0 -I{} echo {} | wc -l | tr -d '[:space:]')
+  echo "**Results:** $ACTUAL_COUNT of $EXPECTED_COUNT versions tested" >> comment.md
+  ...
+else
+  echo "⚠️ No test results available" >> comment.md
+  echo "" >> comment.md
+fi
Suggestion importance[1-10]: 4

__

Why: The suggestion provides a more robust way to count files that is less prone to issues with whitespace or different find implementations, which is a good practice for shell scripting, although the current implementation is likely sufficient for the GitHub Actions environment.

Low
Possible issue
Use GITHUB_TOKEN via env for API calls

To ensure reliable API authentication, pass the GITHUB_TOKEN to the curl command
using an environment variable instead of direct interpolation.

.github/workflows/postgresql-test.yml [78-81]

-PATCH=$(curl -s -H "Authorization: token ${{ github.token }}" \
-  -H "Accept: application/vnd.github.v3+json" \
-  "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
-  jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || echo "")
+- name: Get PostgreSQL Versions
+  id: get-versions
+  env:
+    GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+  run: |
+    if [ "${{ github.event_name }}" == "workflow_dispatch" ]; then
+      # Manual workflow
+      if [ "${{ github.event.inputs.test_mode }}" == "Specific version" ] && [ "${{ github.event.inputs.version }}" != "" ]; then
+        VERSION="${{ github.event.inputs.version }}"
+        echo "Testing specific version: $VERSION"
+        VERSIONS="[\"$VERSION\"]"
+      else
+        echo "Testing latest 5 versions (including RC/beta/alpha)"
+        VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
+          cut -d'=' -f1 | \
+          tr -d ' ' | \
+          jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
+      fi
+    elif [ "${{ github.event_name }}" == "pull_request" ]; then
+      echo "Detecting versions changed in PR #${{ github.event.pull_request.number }}"
+      PATCH=$(curl -s -H "Authorization: token ${GH_TOKEN}" \
+        -H "Accept: application/vnd.github.v3+json" \
+        "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
+        jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || echo "")
+      ...
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that passing ${{ github.token }} directly in a shell script is not robust and recommends the best practice of using an environment variable, which improves the reliability of the API call.

Medium
✅ Suggestions up to commit 61ed1e3
CategorySuggestion                                                                                                                                    Impact
Possible issue
Use proper GitHub token in curl

Replace ${{ github.token }} with the $GITHUB_TOKEN environment variable in the
curl command to correctly authenticate with the GitHub API.

.github/workflows/postgresql-test.yml [78-81]

-PATCH=$(curl -s -H "Authorization: token ${{ github.token }}" \
+PATCH=$(curl -s -H "Authorization: token $GITHUB_TOKEN" \
   -H "Accept: application/vnd.github.v3+json" \
   "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
   jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || echo "")
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that ${{ github.token }} is not available as a shell variable in run steps, and the proposed fix using the $GITHUB_TOKEN environment variable is correct to prevent API authentication failures.

Medium
Fix semantic version sorting
Suggestion Impact:The commit changed the version sorting logic to use `sort -V -r | head -5`, replacing the previous jq-based sorting. While not the exact suggested jq solution, it addresses the semantic version sorting issue the suggestion raised.

code diff:

               VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
                 cut -d'=' -f1 | \
                 tr -d ' ' | \
-                jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
+                sort -V -r | \
+                head -5 | \
+                jq -R -s -c 'split("\n") | map(select(length > 0)) | unique')
             fi

Improve the version sorting logic in the jq command to handle semantic
versioning correctly, ensuring versions like "10.0" are sorted after "9.6".

.github/workflows/postgresql-test.yml [68-71]

-VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
-  cut -d'=' -f1 | \
-  tr -d ' ' | \
-  jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
+VERSIONS=$(grep -E "^[0-9]+(\.[0-9]+){1,2}\s*=" releases.properties | \
+  cut -d'=' -f1 | tr -d ' ' | sed 's/=$//' | \
+  jq -R -s -c '
+    split("\n")
+    | map(select(length > 0))
+    | unique
+    | map(split(".") | map(tonumber) | (.[2] //= 0) | {orig: (.[0]|tostring)+"."+((.[1]|tostring))+("."+(.[2]|tostring)), parts: .})
+    | sort_by(.parts[0], .parts[1], .parts[2])
+    | reverse
+    | map(.orig)
+    | .[0:5]
+  ')
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a flaw in the version sorting logic, which would fail to sort versions like 10.1 and 9.2 correctly, and proposes a robust fix using jq for proper semantic version sorting.

Medium
Robustly find extracted postgres path

Add a recursive search fallback to locate the PostgreSQL installation directory
after extraction, making the detection more robust against variations in archive
structure.

.github/workflows/postgresql-test.yml [221-231]

 $extractOutput = & 7z x $downloadPath -o"test-postgresql" -y 2>&1
-...
+if ($LASTEXITCODE -ne 0) {
+  Write-Host "❌ ERROR: Extraction failed with exit code: $LASTEXITCODE"
+  Write-Host "7z output:`n$extractOutput"
+  echo "success=false" >> $env:GITHUB_OUTPUT
+  echo "error=Extraction failed with exit code $LASTEXITCODE" >> $env:GITHUB_OUTPUT
+  exit 1
+}
+Write-Host "✅ Extraction successful"
+# Try direct match first
 $pgDir = Get-ChildItem -Path "test-postgresql" -Directory | Where-Object { $_.Name -match "^postgresql" } | Select-Object -First 1
+if (-not $pgDir) {
+  # Fallback: search recursively for postgres.exe and infer root
+  $pgExe = Get-ChildItem -Path "test-postgresql" -Recurse -Filter "postgres.exe" -File -ErrorAction SilentlyContinue | Select-Object -First 1
+  if ($pgExe) {
+    $pgDir = $pgExe.Directory.Parent
+  }
+}
+if ($pgDir) {
+  $pgPath = $pgDir.FullName
+  $binPath = Join-Path $pgPath "bin"
+  if (Test-Path $binPath) {
+    Write-Host "✅ PostgreSQL directory found: $pgPath"
+    echo "pg-path=$pgPath" >> $env:GITHUB_OUTPUT
+    echo "success=true" >> $env:GITHUB_OUTPUT
+  } else {
+    Write-Host "❌ ERROR: bin directory not found in $pgPath"
+    echo "success=false" >> $env:GITHUB_OUTPUT
+    echo "error=bin directory not found in extracted archive" >> $env:GITHUB_OUTPUT
+    exit 1
+  }
+} else {
+  Write-Host "❌ ERROR: PostgreSQL directory not found after extraction"
+  echo "success=false" >> $env:GITHUB_OUTPUT
+  echo "error=PostgreSQL directory not found after extraction" >> $env:GITHUB_OUTPUT
+  exit 1
+}
Suggestion importance[1-10]: 5

__

Why: The suggestion improves the robustness of the script by adding a fallback mechanism to find the PostgreSQL directory, making the workflow less brittle to changes in the archive structure.

Low
✅ Suggestions up to commit 037c4d1
CategorySuggestion                                                                                                                                    Impact
Possible issue
Use Bearer auth and handle pagination

Replace the curl command with gh api --paginate to correctly handle GitHub API
pagination and prevent missed version detection in large pull requests.

.github/workflows/postgresql-test.yml [62-65]

-PATCH=$(curl -s -H "Authorization: token ${{ github.token }}" \
-  -H "Accept: application/vnd.github.v3+json" \
-  "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
-  jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || echo "")
+PATCH=$(\
+  gh api \
+    -H "Accept: application/vnd.github+json" \
+    /repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files --paginate \
+  | jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null \
+)
+if [ -z "$PATCH" ]; then
+  echo "No changes to releases.properties detected"
+  # fallback...
+fi
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a critical flaw where the workflow would fail to detect changes in releases.properties in PRs with more than 30 files due to lack of API pagination, and it proposes a robust fix using gh api.

Medium
Fix misleading PR phase summary
Suggestion Impact:The commit modified the PR comment's "Test Phases" to list only Installation Validation and Basic Functionality, removing the unimplemented server initialization, DB operations, and cleanup phases. It also adjusted the artifact note, aligning with the suggestion’s intent.

code diff:

@@ -475,12 +488,10 @@
           echo "### 📋 Test Phases" >> comment.md
           echo "" >> comment.md
           echo "Each version is tested through the following phases:" >> comment.md
-          echo "- **Phase 1:** Installation Validation (Download, Extract, Verify)" >> comment.md
-          echo "- **Phase 2:** Server Initialization (Init Cluster, Start Server)" >> comment.md
-          echo "- **Phase 3:** Database Operations (Connect, Create DB, Delete DB)" >> comment.md
-          echo "- **Phase 4:** Cleanup (Stop Server)" >> comment.md
+          echo "- **Phase 1:** Installation Validation (Download, Extract, Verify Executables)" >> comment.md
+          echo "- **Phase 2:** Basic Functionality (Test Executable Versions)" >> comment.md
           echo "" >> comment.md
-          echo "_Check artifacts for detailed logs and server output._" >> comment.md
+          echo "_Check artifacts for detailed logs._" >> comment.md
           

Update the PR comment generation to accurately reflect the implemented test
phases, as it currently reports on unimplemented server initialization and
database operation tests.

.github/workflows/postgresql-test.yml [475-483]

 - name: Generate PR Comment
   run: |
     echo "## 🐘 PostgreSQL Module Tests - Results" > comment.md
     echo "" >> comment.md
     echo "**Test Date:** $(date -u '+%Y-%m-%d %H:%M:%S UTC')" >> comment.md
     ...
     echo "### 📋 Test Phases" >> comment.md
     echo "" >> comment.md
     echo "Each version is tested through the following phases:" >> comment.md
     echo "- **Phase 1:** Installation Validation (Download, Extract, Verify)" >> comment.md
-    echo "- **Phase 2:** Server Initialization (Init Cluster, Start Server)" >> comment.md
-    echo "- **Phase 3:** Database Operations (Connect, Create DB, Delete DB)" >> comment.md
-    echo "- **Phase 4:** Cleanup (Stop Server)" >> comment.md
+    echo "- **Phase 2:** Basic Functionality (version checks for postgres/psql/initdb)" >> comment.md
     echo "" >> comment.md
-    echo "_Check artifacts for detailed logs and server output._" >> comment.md
+    echo "_Planned phases (server init/start, DB ops, cleanup) are not yet implemented in this workflow run._" >> comment.md
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the generated PR comment and documentation describe test phases that are not implemented, which is misleading. Fixing this improves the accuracy of the test reporting.

Medium
Avoid null Response access on failure

Add a null check for $_.Exception.Response in the catch block for
Invoke-WebRequest to prevent a secondary exception and ensure the original
download error is reported correctly.

.github/workflows/postgresql-test.yml [184-192]

-- name: Phase 1.1 - Download PostgreSQL
-  id: download-postgresql
-  continue-on-error: true
-  run: |
-    $ErrorActionPreference = "Continue"
-    $version = "${{ matrix.version }}"
-    ...
-    try {
-      $fileName = [System.IO.Path]::GetFileName($downloadUrl)
-      $downloadPath = Join-Path "test-postgresql" $fileName
-      ...
-      try {
-        Invoke-WebRequest -Uri $downloadUrl -OutFile $downloadPath -UseBasicParsing -TimeoutSec 300
-      } catch {
-        Write-Host "❌ ERROR: Download failed!"
-        Write-Host "Error details: $($_.Exception.Message)"
-        Write-Host "Status Code: $($_.Exception.Response.StatusCode.value__)"
-        Write-Host "URL attempted: $downloadUrl"
-        echo "success=false" >> $env:GITHUB_OUTPUT
-        echo "error=Download failed: $($_.Exception.Message)" >> $env:GITHUB_OUTPUT
-        exit 1
-      }
+try {
+  Invoke-WebRequest -Uri $downloadUrl -OutFile $downloadPath -UseBasicParsing -TimeoutSec 300
+} catch {
+  Write-Host "❌ ERROR: Download failed!"
+  $msg = $_.Exception.Message
+  $statusCode = $null
+  if ($_.Exception.Response -ne $null -and $_.Exception.Response.StatusCode -ne $null) {
+    $statusCode = $_.Exception.Response.StatusCode.value__
+    Write-Host "Status Code: $statusCode"
+  } else {
+    Write-Host "Status Code: (unavailable)"
+  }
+  Write-Host "Error details: $msg"
+  Write-Host "URL attempted: $downloadUrl"
+  echo "success=false" >> $env:GITHUB_OUTPUT
+  echo "error=Download failed: $msg" >> $env:GITHUB_OUTPUT
+  exit 1
+}
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential null reference exception in the error handling for network failures, which would mask the original error. The proposed fix makes the error reporting more robust.

Medium
General
Robustly locate bin folder post-extraction

Improve the logic for finding the PostgreSQL directory after extraction by
performing a recursive search, making it more robust against different archive
structures.

.github/workflows/postgresql-test.yml [208-218]

-- name: Phase 1.1 - Download PostgreSQL
-  id: download-postgresql
-  continue-on-error: true
-  run: |
-    $ErrorActionPreference = "Continue"
-    ...
-    $extractOutput = & 7z x $downloadPath -o"test-postgresql" -y 2>&1
-    ...
-    $pgDir = Get-ChildItem -Path "test-postgresql" -Directory | Where-Object { $_.Name -match "^postgresql" } | Select-Object -First 1
+# Extract the archive
+Write-Host "Extracting archive..."
+$extractOutput = & 7z x $downloadPath -o"test-postgresql" -y 2>&1
 
+if ($LASTEXITCODE -eq 0) {
+  Write-Host "✅ Extraction successful"
+
+  # Find a directory that contains a bin folder (recursive)
+  $pgDir = Get-ChildItem -Path "test-postgresql" -Directory -Recurse |
+    Where-Object {
+      $_.Name -match "^postgresql" -or (Test-Path (Join-Path $_.FullName "bin"))
+    } | Select-Object -First 1
+
+  if ($pgDir) {
+    $pgPath = $pgDir.FullName
+    $binPath = Join-Path $pgPath "bin"
+    if (-not (Test-Path $binPath)) {
+      # Try one more level: if current is "postgresqlXX" with nested path
+      $binPath = Get-ChildItem -Path $pgPath -Directory -Recurse |
+        Where-Object { Test-Path (Join-Path $_.FullName "bin") } |
+        Select-Object -ExpandProperty FullName -First 1
+    }
+    if ($binPath -and (Test-Path $binPath)) {
+      Write-Host "✅ bin directory found at: $binPath"
+      echo "pg-path=$pgPath" >> $env:GITHUB_OUTPUT
+      echo "success=true" >> $env:GITHUB_OUTPUT
+    } else {
+      Write-Host "❌ ERROR: bin directory not found after extraction"
+      echo "success=false" >> $env:GITHUB_OUTPUT
+      echo "error=bin directory not found in extracted archive" >> $env:GITHUB_OUTPUT
+      exit 1
+    }
+  } else {
+    Write-Host "❌ ERROR: PostgreSQL directory not found after extraction"
+    echo "success=false" >> $env:GITHUB_OUTPUT
+    echo "error=PostgreSQL directory not found after extraction" >> $env:GITHUB_OUTPUT
+    exit 1
+  }
+} else {
+  Write-Host "❌ ERROR: Extraction failed with exit code: $LASTEXITCODE"
+  Write-Host "7z output:"
+  Write-Host $extractOutput
+  echo "success=false" >> $env:GITHUB_OUTPUT
+  echo "error=Extraction failed with exit code $LASTEXITCODE" >> $env:GITHUB_OUTPUT
+  exit 1
+}
+
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the script is not robust against variations in archive structure. The proposed recursive search for the postgresql directory would make the workflow more resilient to different packaging formats.

Medium
Sanitize and sort versions robustly

Refactor the version parsing logic to be more robust by stripping comments,
normalizing line endings, and using a more reliable sorting method before
selecting the latest versions to test.

.github/workflows/postgresql-test.yml [70-74]

-VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
+VERSIONS=$(sed 's/\r$//' releases.properties | \
+  sed 's/#.*$//' | \
+  grep -E "^[0-9]+\.[0-9]+(\.[0-9]+)?\s*=" | \
   grep -v -E "(RC|beta|alpha)" | \
   cut -d'=' -f1 | \
   tr -d ' ' | \
-  jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
+  awk 'NF' | \
+  sort -t. -k1,1n -k2,2n -k3,3n | \
+  tac | \
+  awk '!seen[$0]++' | \
+  head -n 5 | \
+  jq -R -s -c 'split("\n") | map(select(length > 0))')
Suggestion importance[1-10]: 5

__

Why: The suggestion offers a more robust way to parse the releases.properties file by handling comments and line endings, but the current implementation is sufficient for the file's expected format. The proposed change is a good defensive improvement but not critical.

Low
✅ Suggestions up to commit 8580b85
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix missing step reference for logs
Suggestion Impact:The commit removed the entire "Upload Server Logs" step that had the faulty condition referencing a non-existent step, effectively preventing the error. While the suggestion proposed relaxing the condition and adding if-no-files-found, the commit resolved it by deleting the step.

code diff:

-      - name: Upload Server Logs
-        if: always() && steps.start-server.outputs.success == 'true'
-        uses: actions/upload-artifact@v4
-        with:
-          name: server-logs-postgresql-${{ matrix.version }}
-          path: test-results/postgresql.log
-          retention-days: 7
-

Fix the conditional for uploading server logs, which incorrectly references a
non-existent step start-server, preventing logs from being uploaded.

.github/workflows/postgresql-test.yml [419-425]

 - name: Upload Server Logs
-  if: always() && steps.start-server.outputs.success == 'true'
+  if: always()
   uses: actions/upload-artifact@v4
   with:
     name: server-logs-postgresql-${{ matrix.version }}
     path: test-results/postgresql.log
     retention-days: 7
+    if-no-files-found: ignore
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a bug where a condition references a non-existent step start-server, which would prevent server logs from ever being uploaded, and provides a correct fix.

High
Use GITHUB_TOKEN env for PR API calls

Replace the direct use of ${{ github.token }} with an environment variable
GITHUB_TOKEN populated from secrets.GITHUB_TOKEN for more reliable API
authentication in the workflow script.

.github/workflows/postgresql-test.yml [49-113]

 - name: Get PostgreSQL Versions
   id: get-versions
+  env:
+    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
   run: |
+    set -e
     if [ "${{ github.event.inputs.version }}" != "" ]; then
-      # Manual workflow with specific version
       VERSION="${{ github.event.inputs.version }}"
       echo "Testing specific version: $VERSION"
       VERSIONS="[\"$VERSION\"]"
     elif [ "${{ github.event_name }}" == "pull_request" ]; then
-      # For PRs, detect which versions were added or modified using GitHub API
       echo "Detecting versions changed in PR #${{ github.event.pull_request.number }}"
-      
-      # Get the diff from GitHub API using curl
-      PATCH=$(curl -s -H "Authorization: token ${{ github.token }}" \
+      PATCH=$(curl -fS -s -H "Authorization: token $GITHUB_TOKEN" \
         -H "Accept: application/vnd.github.v3+json" \
         "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
         jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || echo "")
-      
       if [ -z "$PATCH" ]; then
         echo "No changes to releases.properties detected"
         echo "Testing latest 5 stable versions as fallback"
         VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
           grep -v -E "(RC|beta|alpha)" | \
-          cut -d'=' -f1 | \
-          tr -d ' ' | \
+          cut -d'=' -f1 | tr -d ' ' | \
           jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
       else
         echo "Analyzing diff for added/modified versions..."
-        
-        # Extract added lines (lines starting with +) that contain version numbers
         CHANGED_VERSIONS=$(echo "$PATCH" | \
-          grep "^+" | \
-          grep -v "^+++" | \
-          grep -E "^\+[0-9]+\.[0-9]+" | \
-          sed 's/^+//' | \
-          cut -d'=' -f1 | \
-          tr -d ' ' | \
-          grep -v -E "(RC|beta|alpha)" || echo "")
-        
+          grep "^+" | grep -v "^+++" | \
+          grep -E "^\+[0-9]+\.[0-9]+" | sed 's/^+//' | \
+          cut -d'=' -f1 | tr -d ' ' | \
+          grep -v -E "(RC|beta|alpha)" || true)
         if [ -z "$CHANGED_VERSIONS" ]; then
           echo "No new stable versions found in PR"
           echo "Testing latest 5 stable versions as fallback"
           VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
             grep -v -E "(RC|beta|alpha)" | \
-            cut -d'=' -f1 | \
-            tr -d ' ' | \
+            cut -d'=' -f1 | tr -d ' ' | \
             jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
         else
           echo "Changed versions detected:"
           echo "$CHANGED_VERSIONS"
           VERSIONS=$(echo "$CHANGED_VERSIONS" | jq -R -s -c 'split("\n") | map(select(length > 0)) | unique')
         fi
       fi
     else
-      # For other events, test latest 5 stable versions
       echo "Testing latest 5 stable versions (excluding RC/beta/alpha)"
       VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
         grep -v -E "(RC|beta|alpha)" | \
-        cut -d'=' -f1 | \
-        tr -d ' ' | \
+        cut -d'=' -f1 | tr -d ' ' | \
         jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
     fi
-    
     echo "versions=$VERSIONS" >> $GITHUB_OUTPUT
     echo "Versions to test: $VERSIONS"
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that using ${{ github.token }} directly in a script is not best practice and proposes using secrets.GITHUB_TOKEN via an environment variable, which is more robust for API authentication.

Medium
Normalize pg_hba.conf whitespace

Normalize the whitespace in all pg_hba.conf.ber files by replacing mixed tabs
and spaces with single spaces to prevent potential parsing errors by PostgreSQL.

bin/postgresql13.23/pg_hba.conf.ber [1-3]

-# TYPE      DATABASE        USER            ADDRESS                 METHOD
-  host        all           all             127.0.0.1/32            trust
-  host 		    all           all              ::1/128	      				trust
+# TYPE  DATABASE  USER  ADDRESS     METHOD
+host    all       all   127.0.0.1/32  trust
+host    all       all   ::1/128       trust
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies inconsistent whitespace and tabs in pg_hba.conf.ber files, which could cause parsing issues for PostgreSQL, and recommends normalizing it for robustness.

Medium
General
Add resilient download retries

Implement a retry loop with exponential backoff for the Invoke-WebRequest
command to make the download process more resilient to transient network issues.

.github/workflows/postgresql-test.yml [175-192]

-- name: Phase 1.1 - Download PostgreSQL
-  id: download-postgresql
-  continue-on-error: true
-  run: |
-    $ErrorActionPreference = "Continue"
-    $version = "${{ matrix.version }}"
-    ...
+try {
+  $fileName = [System.IO.Path]::GetFileName($downloadUrl)
+  $downloadPath = Join-Path "test-postgresql" $fileName
+
+  Write-Host "Downloading PostgreSQL $version..."
+  Write-Host "Target file: $downloadPath"
+
+  $attempt = 0
+  $maxAttempts = 3
+  $delaySeconds = 5
+  $downloaded = $false
+
+  while (-not $downloaded -and $attempt -lt $maxAttempts) {
+    $attempt++
     try {
-      $fileName = [System.IO.Path]::GetFileName($downloadUrl)
-      $downloadPath = Join-Path "test-postgresql" $fileName
-      
-      Write-Host "Downloading PostgreSQL $version..."
-      Write-Host "Target file: $downloadPath"
-      
-      try {
-        Invoke-WebRequest -Uri $downloadUrl -OutFile $downloadPath -UseBasicParsing -TimeoutSec 300
-      } catch {
-        Write-Host "❌ ERROR: Download failed!"
-        Write-Host "Error details: $($_.Exception.Message)"
-        Write-Host "Status Code: $($_.Exception.Response.StatusCode.value__)"
-        Write-Host "URL attempted: $downloadUrl"
+      Invoke-WebRequest -Uri $downloadUrl -OutFile $downloadPath -UseBasicParsing -TimeoutSec 300
+      $downloaded = $true
+    } catch {
+      $statusCode = if ($_.Exception.Response) { $_.Exception.Response.StatusCode.value__ } else { "N/A" }
+      Write-Host "⚠️ Download attempt $attempt failed (Status: $statusCode): $($_.Exception.Message)"
+      if ($attempt -lt $maxAttempts) {
+        Write-Host "Retrying in $delaySeconds seconds..."
+        Start-Sleep -Seconds $delaySeconds
+        $delaySeconds *= 2
+      } else {
+        Write-Host "❌ ERROR: Download failed after $maxAttempts attempts"
         echo "success=false" >> $env:GITHUB_OUTPUT
         echo "error=Download failed: $($_.Exception.Message)" >> $env:GITHUB_OUTPUT
         exit 1
       }
+    }
+  }
+} catch {
+  Write-Host "❌ ERROR: Unexpected error occurred"
+  Write-Host "Error message: $($_.Exception.Message)"
+  Write-Host "Stack trace: $($_.ScriptStackTrace)"
+  echo "success=false" >> $env:GITHUB_OUTPUT
+  echo "error=$($_.Exception.Message)" >> $env:GITHUB_OUTPUT
+  exit 1
+}
Suggestion importance[1-10]: 6

__

Why: The suggestion improves the robustness of the download step by adding a retry mechanism, which is a good practice for network operations in CI to mitigate transient failures.

Low
Quote paths and verify required DLLs

Enhance the verification step to check for the existence of required DLLs, such
as libpq.dll, in addition to executables to catch potential runtime issues
earlier.

.github/workflows/postgresql-test.yml [287-297]

-- name: Phase 1.2 - Verify PostgreSQL Installation
-  id: verify-postgresql
-  if: steps.download-postgresql.outputs.success == 'true'
-  continue-on-error: true
-  run: |
-    $ErrorActionPreference = "Continue"
-    $pgPath = "${{ steps.download-postgresql.outputs.pg-path }}"
-    ...
-    foreach ($exe in $requiredExes) {
-      $exePath = Join-Path $binPath $exe
-      if (Test-Path $exePath) {
-        Write-Host "✅ Found: $exe"
-        $verifyResults[$exe] = @{ found = $true; path = $exePath }
-      } else {
-        Write-Host "❌ Missing: $exe"
-        $verifyResults[$exe] = @{ found = $false }
-        $allFound = $false
-      }
-    }
+$requiredExes = @("postgres.exe", "psql.exe", "pg_ctl.exe", "initdb.exe", "createdb.exe", "dropdb.exe")
+$requiredDlls = @("libpq.dll")
 
+$allFound = $true
+$verifyResults = @{}
+
+foreach ($exe in $requiredExes) {
+  $exePath = Join-Path $binPath $exe
+  if (Test-Path $exePath) {
+    Write-Host "✅ Found: $exe"
+    $verifyResults[$exe] = @{ found = $true; path = $exePath }
+  } else {
+    Write-Host "❌ Missing: $exe"
+    $verifyResults[$exe] = @{ found = $false }
+    $allFound = $false
+  }
+}
+
+foreach ($dll in $requiredDlls) {
+  $dllPath = Join-Path $binPath $dll
+  if (Test-Path $dllPath) {
+    Write-Host "✅ Found: $dll"
+    $verifyResults[$dll] = @{ found = $true; path = $dllPath }
+  } else {
+    Write-Host "❌ Missing: $dll"
+    $verifyResults[$dll] = @{ found = $false }
+    $allFound = $false
+  }
+}
+
Suggestion importance[1-10]: 4

__

Why: The suggestion to check for required DLLs is a good enhancement for early failure detection, although the primary concern about unquoted paths is less critical as the path is unlikely to contain spaces in this context.

Low
✅ Suggestions up to commit 69c5b6f
CategorySuggestion                                                                                                                                    Impact
Possible issue
Harden PR version detection and fallbacks

To prevent CI failures on pull requests from forks, add a fallback mechanism. If
the GitHub API call for changed files fails, use git diff to determine which
PostgreSQL versions to test.

.github/workflows/postgresql-test.yml [49-113]

 - name: Get PostgreSQL Versions
   id: get-versions
+  shell: bash
   run: |
+    set -euo pipefail
+
+    fallback_latest() {
+      if [ ! -f releases.properties ]; then
+        echo "releases.properties not found for fallback"
+        echo "[]"
+        return 0
+      fi
+      grep -E "^[0-9]+\.[0-9]+" releases.properties | \
+      grep -v -E "(RC|beta|alpha)" | \
+      cut -d'=' -f1 | tr -d ' ' | \
+      jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]'
+    }
+
     if [ "${{ github.event.inputs.version }}" != "" ]; then
-      # Manual workflow with specific version
       VERSION="${{ github.event.inputs.version }}"
       echo "Testing specific version: $VERSION"
-      VERSIONS="[\"$VERSION\"]"
+      VERSIONS=$(jq -n --arg v "$VERSION" '[ $v ]')
     elif [ "${{ github.event_name }}" == "pull_request" ]; then
-      # For PRs, detect which versions were added or modified using GitHub API
       echo "Detecting versions changed in PR #${{ github.event.pull_request.number }}"
-      
-      # Get the diff from GitHub API using curl
-      PATCH=$(curl -s -H "Authorization: token ${{ github.token }}" \
-        -H "Accept: application/vnd.github.v3+json" \
-        "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
-        jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || echo "")
-      
+      PATCH="$(curl -sS -H "Authorization: token ${{ github.token }}" \
+               -H "Accept: application/vnd.github.v3+json" \
+               "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
+               jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || true)"
+
       if [ -z "$PATCH" ]; then
-        echo "No changes to releases.properties detected"
-        echo "Testing latest 5 stable versions as fallback"
-        VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
-          grep -v -E "(RC|beta|alpha)" | \
-          cut -d'=' -f1 | \
-          tr -d ' ' | \
-          jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
+        echo "GitHub API did not return a patch. Falling back to local git diff."
+        # Fallback: use git to compute diff for releases.properties
+        git fetch --no-tags --prune --depth=50 origin "${{ github.base_ref }}" || true
+        BASE_SHA="$(git merge-base HEAD "origin/${{ github.base_ref }}")" || BASE_SHA="${{ github.event.pull_request.base.sha }}"
+        PATCH="$(git diff --unified=0 "$BASE_SHA"...HEAD -- releases.properties || true)"
+      fi
+
+      if [ -z "$PATCH" ]; then
+        echo "No changes detected to releases.properties; using latest 5 stable as fallback"
+        VERSIONS="$(fallback_latest)"
       else
-        echo "Analyzing diff for added/modified versions..."
-        
-        # Extract added lines (lines starting with +) that contain version numbers
-        CHANGED_VERSIONS=$(echo "$PATCH" | \
-          grep "^+" | \
-          grep -v "^+++" | \
-          grep -E "^\+[0-9]+\.[0-9]+" | \
-          sed 's/^+//' | \
-          cut -d'=' -f1 | \
-          tr -d ' ' | \
-          grep -v -E "(RC|beta|alpha)" || echo "")
-        
+        CHANGED_VERSIONS="$(echo "$PATCH" | grep "^+" | grep -v "^+++" | \
+          grep -E "^\+[0-9]+\.[0-9]+" | sed 's/^+//' | cut -d'=' -f1 | tr -d ' ' | \
+          grep -v -E "(RC|beta|alpha)" || true)"
         if [ -z "$CHANGED_VERSIONS" ]; then
-          echo "No new stable versions found in PR"
-          echo "Testing latest 5 stable versions as fallback"
-          VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
-            grep -v -E "(RC|beta|alpha)" | \
-            cut -d'=' -f1 | \
-            tr -d ' ' | \
-            jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
+          echo "No new stable versions found in diff; using latest 5 stable as fallback"
+          VERSIONS="$(fallback_latest)"
         else
-          echo "Changed versions detected:"
-          echo "$CHANGED_VERSIONS"
-          VERSIONS=$(echo "$CHANGED_VERSIONS" | jq -R -s -c 'split("\n") | map(select(length > 0)) | unique')
+          VERSIONS="$(echo "$CHANGED_VERSIONS" | jq -R -s -c 'split("\n") | map(select(length > 0)) | unique')"
         fi
       fi
     else
-      # For other events, test latest 5 stable versions
       echo "Testing latest 5 stable versions (excluding RC/beta/alpha)"
-      VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
-        grep -v -E "(RC|beta|alpha)" | \
-        cut -d'=' -f1 | \
-        tr -d ' ' | \
-        jq -R -s -c 'split("\n") | map(select(length > 0)) | unique | sort_by(split(".") | map(tonumber)) | reverse | .[0:5]')
+      VERSIONS="$(fallback_latest)"
     fi
-    
-    echo "versions=$VERSIONS" >> $GITHUB_OUTPUT
+
+    if [ -z "${VERSIONS:-}" ] || [ "$(echo "$VERSIONS" | jq 'length')" -eq 0 ]; then
+      echo "Computed versions list is empty; defaulting to latest"
+      VERSIONS="$(fallback_latest)"
+    fi
+
+    echo "versions=$VERSIONS" >> "$GITHUB_OUTPUT"
     echo "Versions to test: $VERSIONS"
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical flaw in the workflow that would cause it to fail for pull requests from forks due to github.token permissions, and provides a robust git diff fallback, significantly improving CI reliability for external contributors.

High
Bind PostgreSQL to a free port and wait

To prevent port conflicts on CI runners, modify the server start-up script to
use a dynamically assigned free port instead of the hardcoded port 5432. Also,
replace the fixed sleep with a more reliable readiness probe.

.github/workflows/postgresql-test.yml [357-431]

 - name: Phase 2.2 - Start PostgreSQL Server
   id: start-server
   if: steps.init-database.outputs.success == 'true'
   continue-on-error: true
-  timeout-minutes: 3
+  timeout-minutes: 5
   run: |
     $ErrorActionPreference = "Continue"
     $binPath = "${{ steps.verify-postgresql.outputs.bin-path }}"
     $dataDir = "${{ steps.init-database.outputs.data-dir }}"
     $logFile = Join-Path (Get-Location) "test-results\postgresql.log"
-    
+
+    # Pick a free TCP port
+    $listener = [System.Net.Sockets.TcpListener]::new([System.Net.IPAddress]::Loopback,0)
+    $listener.Start()
+    $port = ($listener.LocalEndpoint).Port
+    $listener.Stop()
+    Write-Host "Selected free port: $port"
+
+    # Ensure postgresql.conf uses our port (override via -o)
+    $pgCtlExe = Join-Path $binPath "pg_ctl.exe"
+
     Write-Host "=== Phase 2.2: Start PostgreSQL Server ==="
-    
     try {
-      $pgCtlExe = Join-Path $binPath "pg_ctl.exe"
-      
-      # Start the server with timeout
-      Write-Host "Starting PostgreSQL server..."
-      $startJob = Start-Job -ScriptBlock {
-        param($pgCtl, $data, $log)
-        & $pgCtl -D $data -l $log -w -t 30 start 2>&1 | Out-String
-      } -ArgumentList $pgCtlExe, $dataDir, $logFile
-      
-      # Wait for the job with timeout
-      $completed = Wait-Job -Job $startJob -Timeout 60
-      
-      if ($completed) {
-        $output = Receive-Job -Job $startJob
-        Write-Host $output
-        Remove-Job -Job $startJob -Force
-      } else {
-        Write-Host "⚠️  Server start timed out after 60 seconds"
-        Stop-Job -Job $startJob
-        Remove-Job -Job $startJob -Force
+      Write-Host "Starting PostgreSQL server on port $port..."
+      $startArgs = @("-D", $dataDir, "-l", $logFile, "-w", "-t", "60", "start", "-o", "`"-p $port -h 127.0.0.1`"")
+      & $pgCtlExe @startArgs 2>&1 | Tee-Object -Variable pgout | Out-String | Write-Host
+
+      # Probe readiness with simple TCP connect retries
+      $maxWait = 30
+      $ready = $false
+      for ($i=0; $i -lt $maxWait; $i++) {
+        try {
+          $tcp = New-Object System.Net.Sockets.TcpClient
+          $async = $tcp.BeginConnect("127.0.0.1", $port, $null, $null)
+          $done = $async.AsyncWaitHandle.WaitOne(1000)
+          if ($done -and $tcp.Connected) {
+            $tcp.EndConnect($async)
+            $tcp.Close()
+            $ready = $true
+            break
+          }
+          $tcp.Close()
+        } catch { }
       }
-      
-      # Wait a bit for server to stabilize
-      Write-Host "Waiting for server to stabilize..."
-      Start-Sleep -Seconds 3
-      
-      # Check server status
-      Write-Host "Checking server status..."
+      if (-not $ready) {
+        Write-Host "❌ Server did not become ready on port $port"
+        if (Test-Path $logFile) { Get-Content $logFile | Write-Host }
+        echo "success=false" >> $env:GITHUB_OUTPUT
+        exit 1
+      }
+
+      # Status
       $statusOutput = & $pgCtlExe -D $dataDir status 2>&1 | Out-String
       Write-Host $statusOutput
-      
+
       if ($statusOutput -match "server is running") {
-        Write-Host "✅ PostgreSQL server is running"
-        
-        # Display server log
-        if (Test-Path $logFile) {
-          Write-Host "Server log (last 20 lines):"
-          Get-Content $logFile -Tail 20 | Write-Host
-        }
-        
+        Write-Host "✅ PostgreSQL server is running on port $port"
         echo "success=true" >> $env:GITHUB_OUTPUT
         echo "log-file=$logFile" >> $env:GITHUB_OUTPUT
+        echo "port=$port" >> $env:GITHUB_OUTPUT
       } else {
         Write-Host "❌ Server is not running"
-        if (Test-Path $logFile) {
-          Write-Host "Server log:"
-          Get-Content $logFile | Write-Host
-        }
+        if (Test-Path $logFile) { Get-Content $logFile | Write-Host }
         echo "success=false" >> $env:GITHUB_OUTPUT
         exit 1
       }
     } catch {
       Write-Host "❌ Error starting server: $_"
-      Write-Host "Error details: $($_.Exception.Message)"
-      if (Test-Path $logFile) {
-        Write-Host "Server log:"
-        Get-Content $logFile | Write-Host
-      }
+      if (Test-Path $logFile) { Get-Content $logFile | Write-Host }
       echo "success=false" >> $env:GITHUB_OUTPUT
       exit 1
     }
Suggestion importance[1-10]: 9

__

Why: The suggestion addresses a major source of test flakiness by replacing a hardcoded port with dynamic port allocation and swapping a fixed sleep with a reliable TCP readiness probe, which are best practices for robust CI testing.

High
Use host/port in all client commands

Update all PostgreSQL client commands (psql, createdb, dropdb) to explicitly use
the host and the dynamically assigned port from the server start-up step,
ensuring they connect to the correct database instance.

.github/workflows/postgresql-test.yml [433-464]

 - name: Phase 3.1 - Test Database Connection
   id: test-connection
   if: steps.start-server.outputs.success == 'true'
   continue-on-error: true
   run: |
     $ErrorActionPreference = "Continue"
     $binPath = "${{ steps.verify-postgresql.outputs.bin-path }}"
-    
+    $port = "${{ steps.start-server.outputs.port }}"
+    if (-not $port) { $port = 5432 } # fallback
+
     Write-Host "=== Phase 3.1: Test Database Connection ==="
-    
     try {
       $psqlExe = Join-Path $binPath "psql.exe"
-      
-      # Test connection to default postgres database
-      Write-Host "Testing connection to postgres database..."
-      $output = & $psqlExe -U postgres -d postgres -c "SELECT version();" 2>&1 | Out-String
-      
+      Write-Host "Testing connection to postgres database on port $port..."
+      $output = & $psqlExe -h 127.0.0.1 -p $port -U postgres -d postgres -c "SELECT version();" 2>&1 | Out-String
       if ($LASTEXITCODE -eq 0) {
         Write-Host "✅ Connection successful"
         Write-Host $output
         echo "success=true" >> $env:GITHUB_OUTPUT
       } else {
         Write-Host "❌ Connection failed"
         Write-Host $output
         echo "success=false" >> $env:GITHUB_OUTPUT
         exit 1
       }
     } catch {
       Write-Host "❌ Error testing connection: $_"
       echo "success=false" >> $env:GITHUB_OUTPUT
       exit 1
     }
+- name: Phase 3.2 - Test Database Creation
+  id: test-create-db
+  if: steps.test-connection.outputs.success == 'true'
+  continue-on-error: true
+  run: |
+    $ErrorActionPreference = "Continue"
+    $binPath = "${{ steps.verify-postgresql.outputs.bin-pa...

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 23, 2025

PR Reviewer Guide 🔍

(Review updated until commit 1f7341d)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

Docs claim RC/beta/alpha are excluded for PRs, but the version detection step includes them; either adjust detection logic or update docs to avoid misleading expectations.

  echo "Analyzing diff for added/modified versions..."

  # Extract ALL added lines (lines starting with +) that contain version numbers
  # Test ALL versions added in the PR (including RC/beta/alpha)
  CHANGED_VERSIONS=$(echo "$PATCH" | \
    grep "^+" | \
    grep -v "^+++" | \
    grep -E "^\+[0-9]+\.[0-9]+" | \
    sed 's/^+//' | \
    cut -d'=' -f1 | \
    tr -d ' ' || echo "")

  if [ -z "$CHANGED_VERSIONS" ]; then
    echo "No version changes found in PR"
    echo "Testing latest 5 versions as fallback"
    VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
      cut -d'=' -f1 | \
      tr -d ' ' | \
      sort -V -r | \
      head -5 | \
      jq -R -s -c 'split("\n") | map(select(length > 0)) | unique')
  else
    echo "Versions detected in PR:"
    echo "$CHANGED_VERSIONS"
    VERSIONS=$(echo "$CHANGED_VERSIONS" | jq -R -s -c 'split("\n") | map(select(length > 0)) | unique')
  fi
fi
Robustness

Using ${{ github.token }} directly in curl risks hitting API limits or permission edge cases; consider the GITHUB_TOKEN env and pagination for large diffs to ensure reliable PR file parsing.

# Get the diff from GitHub API using curl
PATCH=$(curl -s -H "Authorization: token ${{ github.token }}" \
  -H "Accept: application/vnd.github.v3+json" \
  "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" | \
  jq -r '.[] | select(.filename == "releases.properties") | .patch' 2>/dev/null || echo "")

if [ -z "$PATCH" ]; then
  echo "No changes to releases.properties detected"
  echo "Testing latest 5 versions as fallback"
  VERSIONS=$(grep -E "^[0-9]+\.[0-9]+" releases.properties | \
Portability

The init script uses a Windows log path placeholder while postgresql.conf.ber uses BEARSAMPP_LIN_PATH; verify placeholders are consistent across Windows runners and resulting configs.

%~dp0bin\initdb.exe -U postgres -A trust -E utf8 -D "%~dp0data" > "~BEARSAMPP_WIN_PATH~\logs\postgresql-install.log" 2>&1
copy /y "%~dp0postgresql.conf.ber" "%~dp0data\postgresql.conf"
copy /y "%~dp0pg_hba.conf.ber" "%~dp0data\pg_hba.conf"

@N6REJ N6REJ merged commit d02be8a into main Nov 23, 2025
15 checks passed
@N6REJ N6REJ deleted the ci branch November 23, 2025 11:04
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