fix(redshift-mcp-server): Ensure transaction cleanup when protecting statement

[grayhemp]

Fixes #1489

Summary

Fixes issue where failed user SQL left transactions open by preventing END statement execution.

Changes

• Make sure to always execute END statement
• Log and raise both user SQL and cleanup errors, including the combined error
• Add unit tests for transaction error handling scenarios

User experience

Users won't face the "transaction aborted" error for the queries following a failed query.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• I have reviewed the contributing guidelines
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented

Is this a breaking change? (N)

RFC issue number: #1489

Checklist:

• [N/A] Migration process documented
• [N/A] Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

[grayhemp]

E2E test of the fix

q chat --model claude-4-sonnet --no-interactive \
--trust-tools awslabsredshift_mcp_server___list_clusters,awslabsredshift_mcp_server___list_databases,awslabsredshift_mcp_server___list_schemas,awslabsredshift_mcp_server___list_tables,awslabsredshift_mcp_server___list_columns,awslabsredshift_mcp_server___execute_query,execute_bash,fs_read \
"Please test the transaction error handling fix by: 1) List available Redshift clusters, 2) Execute an intentionally failing SQL query (syntax error like 'SELECT invalid_syntax_test'), 3) Then immediately execute a valid SQL query (like 'SELECT 1') to verify the transaction state was properly cleaned up and subsequent queries work correctly."

I'll help you test the transaction error handling fix for Redshift. Let me execute the test sequence you requested.

🛠️  Using tool: list_clusters (trusted) from mcp server awslabsredshift_mcp_server
...

Great! I found 3 available Redshift clusters. Now let me execute an intentionally failing SQL query with invalid syntax:

🛠️  Using tool: execute_query (trusted) from mcp server awslabsredshift_mcp_server
...

Perfect! The query failed as expected with a syntax error. Now let me immediately execute a valid SQL query to verify that the transaction state was properly
cleaned up:

🛠️  Using tool: execute_query (trusted) from mcp server awslabsredshift_mcp_server
...

## Test Results Summary

✅ Transaction Error Handling Fix Verified Successfully!

The test sequence confirms that the transaction error handling is working correctly:

1. Cluster Discovery: Found 3 available Redshift clusters (2 provisioned, 1 serverless)

2. Failed Query Test: The intentionally invalid SQL SELECT invalid_syntax_test properly failed with error: "column "invalid_syntax_test" does not exist"

3. Recovery Test: The subsequent valid query SELECT 1 executed successfully, returning the expected result with 1 row and completing in 4.143 seconds

Key Verification: The fact that the second query executed successfully immediately after the failed query confirms that:
• The transaction state was properly cleaned up after the error
• No lingering transaction locks or connection issues remained
• The Redshift MCP server correctly handles query failures without affecting subsequent operations

This demonstrates that the transaction error handling fix is working as intended, ensuring robust query execution even when errors occur.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.48%. Comparing base (db0f7e2) to head (f88a9a3).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1512   +/-   ##
=======================================
  Coverage   89.48%   89.48%           
=======================================
  Files         729      729           
  Lines       50584    50599   +15     
  Branches     7990     7992    +2     
=======================================
+ Hits        45264    45280   +16     
  Misses       3453     3453           
+ Partials     1867     1866    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[grayhemp]

General E2E tests

q chat --model claude-4-sonnet --no-interactive \
--trust-tools awslabsredshift_mcp_server___list_clusters,awslabsredshift_mcp_server___list_databases,awslabsredshift_mcp_server___list_schemas,awslabsredshift_mcp_server___list_tables,awslabsredshift_mcp_server___list_columns,awslabsredshift_mcp_server___execute_query,execute_bash,fs_read \
"Test if all the available to you Redshift Mcp Server tools are working. Check both provisioned and serverless clusters, including the database schema exploration in both. Check the SQL read-only protection, transaction breaker protection, failed user SQL behavior. Get the test scenatio ideas from the unit tests under the project directory. Provide a short testing summary, one line per tool."

# After the summary, show the Redshift MCP Server's definition from the mcp.json masking the secret key values."

I'll test the Redshift MCP Server tools systematically. Let me start by exploring the project structure to understand the available tools and unit tests.

...

## Testing Summary

list_clusters: ✅ Successfully discovered both provisioned and serverless clusters with complete metadata
list_databases: ✅ Works on both provisioned and serverless clusters, returns database metadata correctly
list_schemas: ✅ Works on both cluster types, discovers system and user schemas properly
list_tables: ✅ Works on both cluster types, discovers tables in tickit schema with proper ACL info
list_columns: ✅ Works on both cluster types, returns detailed column metadata including data types and constraints
execute_query (read-only): ✅ Successfully executes SELECT queries on both provisioned and serverless clusters
SQL read-only protection: ✅ Properly blocks CREATE/INSERT/UPDATE/DELETE operations with read-only transaction error
Transaction breaker protection: ✅ Successfully blocks suspicious SQL patterns containing BEGIN/COMMIT/ROLLBACK
Failed user SQL behavior: ✅ Properly handles and reports SQL errors like missing tables with descriptive messages

[scottschreckengaust]

LGTM