fix: address all code review issues in BOLT protocol

- CRITICAL: Remove database.close() call in cleanup() - database is managed by server
- CRITICAL: Add logging for all ResultSet closure failures
- HIGH: Add position validation in ridToId() to prevent overflow
- HIGH: Fix authentication error handling to send failure message to client
- HIGH: Document precision loss for BigInteger/BigDecimal conversions
- MEDIUM: Add database health check in handleReset() to force re-auth if needed
- MEDIUM: Add rollback attempt in transaction exception handler
- MEDIUM: Sanitize authentication error messages to prevent information disclosure
- LOW: Add query analysis failure logging with improved verbosity control
- LOW: Add test cases for database persistence, connection limits, and path structures

Co-authored-by: Luca Garulli <lvca@users.noreply.github.com>

Author: github-actions[bot]

bolt/src/main/java/com/arcadedb/bolt/BoltNetworkExecutor.java

@@ -355,7 +355,7 @@ private void handleReset() throws IOException {
       try {
         currentResultSet.close();
       } catch (final Exception e) {
-        // Ignore
+        LogManager.instance().log(this, Level.WARNING, "Failed to close ResultSet during RESET", e);
       }
     }
 
@@ -374,7 +374,13 @@ private void handleReset() throws IOException {
     firstResult = null;
 
     sendSuccess(Map.of());
-    state = State.READY;
+
+    // Check database health and force re-authentication if database is unavailable
+    if (database == null || !database.isOpen()) {
+      state = State.AUTHENTICATION;
+    } else {
+      state = State.READY;
+    }
   }
 
   /**
@@ -523,7 +529,7 @@ private void handlePull(final PullMessage message) throws IOException {
         try {
           currentResultSet.close();
         } catch (final Exception e) {
-          // Ignore
+          LogManager.instance().log(this, Level.WARNING, "Failed to close ResultSet during PULL completion", e);
         }
         currentResultSet = null;
         currentFields = null;
@@ -565,7 +571,7 @@ private void handleDiscard(final DiscardMessage discardMessage) throws IOExcepti
       try {
         currentResultSet.close();
       } catch (final Exception e) {
-        // Ignore
+        LogManager.instance().log(this, Level.WARNING, "Failed to close ResultSet during DISCARD", e);
       }
     }
 
@@ -613,6 +619,14 @@ private void handleBegin(final BeginMessage beginMessage) throws IOException {
       state = State.TX_READY;
 
     } catch (final Exception e) {
+      // Attempt to rollback in case transaction was partially started
+      try {
+        if (database != null) {
+          database.rollback();
+        }
+      } catch (final Exception rollbackError) {
+        LogManager.instance().log(this, Level.WARNING, "Failed to rollback after BEGIN error", rollbackError);
+      }
       final String errorMsg = e.getMessage() != null ? e.getMessage() : "Transaction error";
       sendFailure(BoltException.TRANSACTION_ERROR, errorMsg);
       state = State.FAILED;
@@ -673,7 +687,7 @@ private void handleRollback() throws IOException {
         try {
           currentResultSet.close();
         } catch (final Exception e) {
-          // Ignore
+          LogManager.instance().log(this, Level.WARNING, "Failed to close ResultSet during ROLLBACK", e);
         }
       }
       if (database != null) {
@@ -810,7 +824,10 @@ private boolean isWriteQuery(final String query) {
       return !database.getQueryEngine("opencypher").analyze(query).isIdempotent();
     } catch (final Exception e) {
       // If analysis fails, assume it's a write operation to be safe
-      LogManager.instance().log(this, Level.WARNING, "Failed to analyze query, assuming write operation: " + query, e);
+      // Log at FINE level to avoid spam for complex but valid queries
+      LogManager.instance().log(this, Level.FINE,
+          "Query analysis failed for: " + (query.length() > 100 ? query.substring(0, 100) + "..." : query) +
+          " - assuming write operation", e);
       return true;
     }
   }
@@ -832,19 +849,22 @@ private String generateBookmark() {
    */
   private boolean authenticateUser(final String principal, final String credentials) throws IOException {
     if (principal == null || credentials == null) {
+      sendFailure(BoltException.AUTHENTICATION_ERROR, "Missing credentials");
+      state = State.FAILED;
       return false;
     }
 
     try {
       user = server.getSecurity().authenticate(principal, credentials, databaseName);
       if (user == null) {
-        sendFailure(BoltException.AUTHENTICATION_ERROR, "Invalid credentials");
+        sendFailure(BoltException.AUTHENTICATION_ERROR, "Authentication failed");
         state = State.FAILED;
         return false;
       }
       return true;
     } catch (final ServerSecurityException e) {
-      sendFailure(BoltException.AUTHENTICATION_ERROR, e.getMessage());
+      // Sanitize error message to avoid information disclosure
+      sendFailure(BoltException.AUTHENTICATION_ERROR, "Authentication failed");
       state = State.FAILED;
       return false;
     }
@@ -904,7 +924,7 @@ private void cleanup() {
         currentResultSet = null;
       }
     } catch (final Exception e) {
-      // Ignore
+      LogManager.instance().log(this, Level.WARNING, "Failed to close ResultSet during cleanup", e);
     }
 
     try {
@@ -915,14 +935,9 @@ private void cleanup() {
       // Ignore
     }
 
-    try {
-      if (database != null) {
-        database.close();
-        database = null;
-      }
-    } catch (final Exception e) {
-      // Ignore
-    }
+    // Database is managed by the server - just release our reference
+    // DO NOT close the shared database instance
+    database = null;
 
     try {
       socket.close();

bolt/src/main/java/com/arcadedb/bolt/structure/BoltStructureMapper.java

@@ -280,6 +280,19 @@ private static Map<String, Object> toMap(final Map<?, ?> map) {
 
   /**
    * Convert a Number to PackStream-compatible number.
+   * <p>
+   * <strong>Precision Loss Warning:</strong> BigInteger values that exceed Long.MAX_VALUE
+   * and BigDecimal values are converted to double, which may lose precision for very large
+   * or very precise numbers. This is a limitation of the BOLT protocol's numeric type system.
+   * <p>
+   * For example:
+   * <ul>
+   *   <li>BigInteger larger than 2^63-1 will lose precision when converted to double</li>
+   *   <li>BigDecimal with high precision will be rounded to double precision (~15-17 digits)</li>
+   * </ul>
+   *
+   * @param number the number to convert
+   * @return a Long or Double compatible with BOLT PackStream
    */
   private static Object toNumber(final Number number) {
     if (number instanceof Byte || number instanceof Short || number instanceof Integer || number instanceof Long) {
@@ -289,14 +302,16 @@ private static Object toNumber(final Number number) {
       return number.doubleValue();
     }
     if (number instanceof BigInteger bigInt) {
-      // Try to fit in long, otherwise convert to double
+      // Try to fit in long, otherwise convert to double (with potential precision loss)
       try {
         return bigInt.longValueExact();
       } catch (final ArithmeticException e) {
+        // Precision loss: BigInteger too large for long, converting to double
         return bigInt.doubleValue();
       }
     }
     if (number instanceof BigDecimal bigDec) {
+      // Precision loss: BigDecimal always converted to double
       return bigDec.doubleValue();
     }
     // Default to double
@@ -316,7 +331,14 @@ public static long ridToId(final RID rid) {
     if (bucketId < 0 || bucketId > 0xFFFF) {
       throw new IllegalArgumentException("Bucket ID out of range for BOLT ID conversion: " + bucketId);
     }
+
+    // Validate position to prevent overflow (max 48 bits)
+    final long position = rid.getPosition();
+    if (position < 0 || position > 0xFFFFFFFFFFFFL) {
+      throw new IllegalArgumentException("Position out of range for BOLT ID conversion: " + position);
+    }
+
     // Combine bucket ID (high bits) and position (low bits)
-    return ((long) bucketId << 48) | (rid.getPosition() & 0xFFFFFFFFFFFFL);
+    return ((long) bucketId << 48) | position;
   }
 }

bolt/src/test/java/com/arcadedb/bolt/BoltProtocolIT.java

@@ -981,4 +981,87 @@ void testMultipleErrorsInSequence() {
       }
     }
   }
+
+  @Test
+  void testDatabasePersistsAfterConnectionClose() {
+    // CRITICAL TEST: Verify that closing one connection doesn't close the database for others
+    // This test catches the bug where database.close() was incorrectly called in cleanup()
+    try (Driver driver = getDriver()) {
+      // Session 1: Create some data
+      try (Session session1 = driver.session(SessionConfig.forDatabase(getDatabaseName()))) {
+        session1.run("CREATE (n:PersistenceTest {id: 1, name: 'test'})").consume();
+      } // session1 closes here
+
+      // Session 2: Verify the data still exists and database is still open
+      try (Session session2 = driver.session(SessionConfig.forDatabase(getDatabaseName()))) {
+        Result result = session2.run("MATCH (n:PersistenceTest {id: 1}) RETURN n.name AS name");
+        assertThat(result.hasNext()).isTrue();
+        assertThat(result.next().get("name").asString()).isEqualTo("test");
+      }
+
+      // Session 3: Verify database is still accessible after multiple connection cycles
+      try (Session session3 = driver.session(SessionConfig.forDatabase(getDatabaseName()))) {
+        Result result = session3.run("MATCH (n:PersistenceTest) RETURN count(n) AS cnt");
+        assertThat(result.next().get("cnt").asLong()).isGreaterThanOrEqualTo(1L);
+      }
+    }
+  }
+
+  @Test
+  void testPathStructure() {
+    // Test returning path structures from Cypher queries
+    try (Driver driver = getDriver()) {
+      try (Session session = driver.session(SessionConfig.forDatabase(getDatabaseName()))) {
+        // Create a simple path: (a)-[:REL]->(b)
+        session.run("CREATE (a:PathNode {name: 'start'})-[:CONNECTS_TO {weight: 1.0}]->(b:PathNode {name: 'end'})").consume();
+
+        // Query for the path
+        Result result = session.run("MATCH p = (a:PathNode {name: 'start'})-[r:CONNECTS_TO]->(b:PathNode {name: 'end'}) RETURN p");
+        assertThat(result.hasNext()).isTrue();
+
+        org.neo4j.driver.Record record = result.next();
+        // Note: Path support depends on OpenCypher implementation
+        // At minimum, verify the query doesn't throw an error
+        assertThat(record).isNotNull();
+      }
+    }
+  }
+
+  @Test
+  void testConnectionLimit() {
+    // Test that connection limiting works if configured
+    final int maxConnections = GlobalConfiguration.BOLT_MAX_CONNECTIONS.getValueAsInteger();
+
+    // Only run this test if connection limiting is enabled
+    if (maxConnections > 0 && maxConnections < 100) {
+      List<Driver> drivers = new ArrayList<>();
+      try {
+        // Create connections up to the limit
+        for (int i = 0; i < maxConnections; i++) {
+          Driver driver = getDriver();
+          driver.verifyConnectivity();
+          drivers.add(driver);
+        }
+
+        // Attempt to create one more connection beyond the limit
+        // This should either be rejected or one of the existing connections should be replaced
+        try (Driver extraDriver = getDriver()) {
+          // If we get here, the server either increased the limit or is using connection pooling
+          extraDriver.verifyConnectivity();
+        } catch (Exception e) {
+          // Connection was rejected due to limit - this is expected behavior
+          assertThat(e.getMessage()).contains("connection");
+        }
+      } finally {
+        // Clean up all drivers
+        for (Driver driver : drivers) {
+          try {
+            driver.close();
+          } catch (Exception e) {
+            // Ignore cleanup errors
+          }
+        }
+      }
+    }
+  }
 }