fixed test flakiness in replication tests

[zachmu]

Caused by eventual consistency of asynchronously replicated postgres db with no writes. Added more replication tests specifically targeting the postgres DB.

Also changed an implicitly defined DB name which doesn't exist to be an error.

[itoqa]

Commit: a04a905: 11 test cases ran, 1 failed ❌, 8 passed ✅, 2 additional findings ⚠️.

Summary

Coverage spans database startup and authentication, successful and failed connection readiness, terminal error handling, retry behavior for status queries, and primary/standby availability and failover workflows. It exercises both normal operation and edge cases involving missing databases, unavailable servers, misleading query text, and unsupported replication behavior.

Safe to merge — the only failure attributable to this PR is a minor query-classification issue that can unnecessarily delay some error responses, without evidence of data loss or security impact. Separate high-severity replication and failover findings are not caused by this PR and remain caveats for follow-up.

Tests run by Ito

Result	Severity	Type	Description
❌		Status	Nine of ten STATUS-2 subtests failed. Queries such as select 1 -- see dolt_cluster_status, select 'dolt_cluster_status' as label, and select * from dolt_cluster_status_archive were classified as status queries, and the comment-only case showed the retry budget changing from 5 to 300.
✅	—	Database	A raw PostgreSQL StartupMessage without a database parameter was authenticated as no_db_user and correctly rejected because the username-derived database does not exist. The server returned a fatal SQLSTATE 3D000 ErrorResponse from InitPostgres and never sent ReadyForQuery.
✅	—	Database	A raw PostgreSQL startup message authenticated as postgres without a database parameter, received ReadyForQuery, and returned postgres from SELECT current_database().
✅	—	Database	pgx and database/sql both classify a nonexistent explicit database as a terminal SQLSTATE 3D000 FATAL error from InitPostgres, distinguish it from a transient TCP refusal, and successfully reconnect to postgres afterward.
✅	—	Startup	OpenDB and ConnectDB both returned usable database connections after startup retries, and SELECT 1 succeeded on each path well within the 200-attempt budget.
✅	—	Startup	Both ConnectDB and OpenDB correctly return nil database handles and terminal errors when connecting to the ready server with the nonexistent invalid_db database. The errors retain the database name and SQLSTATE 3D000 after the 200-attempt readiness budget, so the fatal database error is not converted into a successful session or hidden as a startup failure.
✅	—	Startup	Both OpenDB and ConnectDB exhaust the 200-attempt readiness budget against a closed loopback port, return nil database handles, and surface the final connection-refused error.
✅	—	Startup	All six boundary cases passed: reachable servers succeeded through ConnectDB and OpenDB, dead ports returned errors after the 200-attempt retry budget, and invalid_db returned terminal errors containing the database name and SQLSTATE 3D000. Both successful paths also closed cleanly.
✅	—	Status	Status queries receive the 300-attempt eventual-consistency retry budget, including when no retry count is configured, while unrelated queries keep their configured retries.
⚠️		Replication	The primary accepted CREATE TABLE and INSERT statements, but the standby returned ERROR: table not found: vals instead of returning count=5.
⚠️		Replication	The demotion query expected a {0} result but returned function: 'dolt_assume_cluster_role' not found (errno 1105). The failover scenario therefore cannot preserve or reverse replication because role transition stops before those assertions execute.

Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟠 Primary rows do not reach standby

• Severity: High
• Description: The primary accepted CREATE TABLE and INSERT statements, but the standby returned ERROR: table not found: vals instead of returning count=5.
• Impact: Configured PostgreSQL standby users cannot see tables or rows committed on the primary, so the standby cannot serve reads or provide the expected failover data for this workflow. The primary data remains available, but using the standby requires falling back to the primary or disabling the unsupported replication configuration.
• Steps to Reproduce:
  1. Start two local Doltgres servers with reciprocal cluster remotes, server1 as primary and server2 as standby.
  2. Connect to the primary's postgres database and create vals(i INT PRIMARY KEY).
  3. Insert five rows into vals on the primary after setting dolt_cluster_ack_writes_timeout_secs to 10.
  4. Connect to the standby's postgres database and run SELECT count(*) FROM vals.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run. The test used two local Doltgres 0.57.0 servers with reciprocal cluster configuration and real SQL writes and reads.
• Code Analysis: The production configuration accepts and adapts cluster YAML through DoltgresConfig.ClusterConfig at servercfg/cfgdetails/config.go:566-570 and exposes standby remotes and role values through the adapter at lines 573-599, but ClusterCfg itself is marked minver:TBD at lines 194-198. There is no production replication implementation connected to this adapter in the inspected repository, and integration-tests/go-sql-server-driver/tests/sql-server-cluster.yaml explicitly states at lines 3-6 that Doltgres does not yet implement cluster replication or the remotes API. The smallest practical fix is to implement and wire the cluster replication controller for the configured PostgreSQL databases, or reject/disable these cluster settings until that controller exists; increasing test retries cannot make writes propagate.

Evidence Package

• text/REPLICATION-1/REPLICATION-1-primary-to-standby-evidence.txt — 3 KB

🟠 Cluster failover role transition is unavailable

• Severity: High
• Description: The demotion query expected a {0} result but returned function: 'dolt_assume_cluster_role' not found (errno 1105). The failover scenario therefore cannot preserve or reverse replication because role transition stops before those assertions execute.
• Impact: Deployments relying on Doltgres cluster replication cannot demote the primary or reverse replication during failover, leaving the advertised high-availability workflow unavailable. The primary may continue serving data, but users have no functioning role-transition path when failover is required.
• Steps to Reproduce:
  1. Start server1 as a cluster primary and server2 as a standby with reciprocal standby remotes.
  2. Connect to the postgres database on server1, create vals, and insert rows 1, 2, and 3.
  3. Run select dolt_assume_cluster_role('standby', 2) on server1 to demote the primary.
  4. Observe that the query fails before server2 can be promoted or failover data continuity can be checked.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: server/functions/dolt_procedures.go lines 68-76 register dolt_assume_cluster_role as a dynamically resolved procedure rather than providing its implementation. Its resolver at lines 99-110 requires the session provider to implement sql.ExternalStoredProcedureProvider and returns function-not-found when that provider or procedure is absent. The repository search found no ExternalStoredProcedures implementation, while servercfg/cfgdetails/config.go line 198 still marks cluster configuration minver:TBD. The PR diff only adds the integration scenario and test-driver changes; it does not implement the production cluster controller or provider. The smallest practical fix is to wire the cluster replication controller/provider into the configured server so it registers and executes the role-transition procedures, or to explicitly reject/disable cluster failover configuration until that implementation exists.

Evidence Package

• text/REPLICATION-2/failover-role-transition.txt — 3 KB

Tip

Reply with @itoqa to send us feedback on this test run.

[itoqa]

Unrelated query text inflates the replication retry budget

What failed: Nine of ten STATUS-2 subtests failed. Queries such as select 1 -- see dolt_cluster_status, select 'dolt_cluster_status' as label, and select * from dolt_cluster_status_archive were classified as status queries, and the comment-only case showed the retry budget changing from 5 to 300.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug

• Severity: Minor
• Impact: Queries whose comments, string literals, or similarly named identifiers contain dolt_cluster_status can wait about 15 seconds before reporting a persistent error. The affected query eventually reports the error, with no evidence of data loss, corruption, or security exposure.
• Steps to Reproduce:
  1. Run the STATUS-2 Go test with go test -run TestSTATUS2 -count=1 -v in integration-tests/go-sql-server-driver.
  2. Pass an ordinary query containing dolt_cluster_status only in a SQL comment, string literal, or unrelated identifier, with RetryAttempts set to 5.
  3. Observe that observesReplicationStatus returns true and RunQuery uses 300 attempts rather than the configured budget.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: The PR-added observesReplicationStatus implementation at lines 652-655 lowercases q.Query and performs an unrestricted strings.Contains match, so it does not distinguish SQL comments or literals from referenced tables and does not enforce identifier boundaries. RunQuery at lines 640-645 consumes that result and promotes attempts below statusEventualConsistencyAttempts, which is 300 at line 637. The smallest practical fix is to parse or sanitize SQL sufficiently to remove comments and literals, then match the exact dolt_cluster_status identifier or referenced relation rather than an arbitrary substring.
• Why this is likely a bug: The failing cases are deterministic and exercise ordinary SQL text that is not a reference to the dolt_cluster_status system table, while the source directly shows that any occurrence of the text triggers the status retry path. This causes a measurable, user-visible delay in the integration driver's normal error reporting and is not explained by setup or environment behavior.

Relevant code

integration-tests/go-sql-server-driver/testdef.go:637-645

const statusEventualConsistencyAttempts = 300; RunQuery promotes attempts to this value when observesReplicationStatus(q) is true.

integration-tests/go-sql-server-driver/testdef.go:652-655

func observesReplicationStatus(q driver.Query) bool { return q.Query != "" && strings.Contains(strings.ToLower(q.Query), "dolt_cluster_status") }

Evidence Package

• text/STATUS-2/STATUS-2-retry-budget-evidence.txt — 4 KB

Copy prompt for an agent

Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Minor severity — Unrelated query text inflates the replication retry budget**

**What failed:** Nine of ten STATUS-2 subtests failed. Queries such as select 1 -- see dolt_cluster_status, select 'dolt_cluster_status' as label, and select * from dolt_cluster_status_archive were classified as status queries, and the comment-only case showed the retry budget changing from 5 to 300.

- **Impact:** Queries whose comments, string literals, or similarly named identifiers contain dolt_cluster_status can wait about 15 seconds before reporting a persistent error. The affected query eventually reports the error, with no evidence of data loss, corruption, or security exposure.
- **Steps to reproduce:**
  1. Run the STATUS-2 Go test with go test -run TestSTATUS2 -count=1 -v in integration-tests/go-sql-server-driver.
  2. Pass an ordinary query containing dolt_cluster_status only in a SQL comment, string literal, or unrelated identifier, with RetryAttempts set to 5.
  3. Observe that observesReplicationStatus returns true and RunQuery uses 300 attempts rather than the configured budget.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The PR-added observesReplicationStatus implementation at lines 652-655 lowercases q.Query and performs an unrestricted strings.Contains match, so it does not distinguish SQL comments or literals from referenced tables and does not enforce identifier boundaries. RunQuery at lines 640-645 consumes that result and promotes attempts below statusEventualConsistencyAttempts, which is 300 at line 637. The smallest practical fix is to parse or sanitize SQL sufficiently to remove comments and literals, then match the exact dolt_cluster_status identifier or referenced relation rather than an arbitrary substring.
- **Why this is likely a bug:** The failing cases are deterministic and exercise ordinary SQL text that is not a reference to the dolt_cluster_status system table, while the source directly shows that any occurrence of the text triggers the status retry path. This causes a measurable, user-visible delay in the integration driver's normal error reporting and is not explained by setup or environment behavior.

**Relevant code:**

`integration-tests/go-sql-server-driver/testdef.go:637-645`

~~~go
const statusEventualConsistencyAttempts = 300; RunQuery promotes attempts to this value when observesReplicationStatus(q) is true.
~~~

`integration-tests/go-sql-server-driver/testdef.go:652-655`

~~~go
func observesReplicationStatus(q driver.Query) bool { return q.Query != "" && strings.Contains(strings.ToLower(q.Query), "dolt_cluster_status") }
~~~

[github-actions]

	Main	PR
covering_index_scan_postgres	1993.37/s	1937.93/s	-2.8%
groupby_scan_postgres	122.05/s	122.75/s	+0.5%
index_join_postgres	696.97/s	701.18/s	+0.6%
index_join_scan_postgres	876.91/s	881.09/s	+0.4%
index_scan_postgres	31.08/s	31.10/s	0.0%
oltp_delete_insert_postgres	905.32/s	907.77/s	+0.2%
oltp_insert	769.19/s	768.09/s	-0.2%
oltp_point_select	3642.05/s	3632.54/s	-0.3%
oltp_read_only	3475.89/s	3471.14/s	-0.2%
oltp_read_write	2589.57/s	2595.75/s	+0.2%
oltp_update_index	822.42/s	824.12/s	+0.2%
oltp_update_non_index	876.48/s	882.32/s	+0.6%
oltp_write_only	1918.14/s	1932.66/s	+0.7%
select_random_points	2046.91/s	2037.88/s	-0.5%
select_random_ranges	1627.26/s	1632.70/s	+0.3%
table_scan_postgres	29.79/s	29.97/s	+0.6%
types_delete_insert_postgres	868.51/s	870.37/s	+0.2%
types_table_scan_postgres	13.03/s	13.07/s	+0.3%

[itoqa]

Ito Diff Report — a04a905 → 6601ef6: 18 test cases ran, 1 still failing ❌, 2 fixed ✅, 15 passing ✅.

Diff Summary

The run covers database query behavior across recursive graph traversal, type and array handling, replication, failover, startup readiness, and malformed-input rejection. It exercises both normal flows and edge cases such as cycles, self-loops, nulls, invalid declarations, changing replication lag, and unavailable peers.

Safe to merge — the PR-attributable issue is minor and limited to unnecessary retry delays for unrelated SQL containing a status-table name, without incorrect results, data loss, or security impact. The broader functional and edge-case coverage is healthy, with this classification defect remaining as a caveat.

Tests run by Ito

Result	State	Severity	Type	Description
❌	❌->❌ Still Failing		Status	The expected behavior is that only a genuine nonempty query reading the dolt_cluster_status table receives the eventual-consistency budget. The implementation lower-cases the entire Query string and checks for a substring, so comments and string literals are classified as status queries; the browser runner could not execute the required two-server Go harness, but the source-level behavior is deterministic.
✅	❌->✅ Fixed	—	Replication	Five rows inserted into the primary PostgreSQL database were committed on the standby, where a count query returned 5.
✅	❌->✅ Fixed	—	Replication	The promoted standby retained the three existing rows, accepted two new rows, and replicated all five rows back to the demoted server.
✅	Passing	—	Record	The CYCLE query returned the expected four-row path containing integer and floating-point record fields, and the assertion framework normalized the nested values consistently with the SQL output.
✅	Passing	—	Record	A cyclic query using TEXT composite keys returned the expected record path, and the assertion framework matched each string element as a Text value.
✅	Passing	—	Record	Boolean arrays format as {t,f,t}, while INT and DOUBLE PRECISION arrays retain their expected {1,2,3} and {1.5,2.5,3.5} output without a normalization panic.
✅	Passing	—	Recursive	A local Doltgres query preserved both key fields, constructed the expected record path through A to B to C and back to A, marked the closing row as a cycle, and terminated the branch.
✅	Passing	—	Recursive	Duplicate and unknown cycle fields, generated-name collisions, non-recursive queries, unsupported UNION shapes, and a malformed VALUES operand all returned descriptive errors. A subsequent valid CYCLE query succeeded, confirming the server remained usable after the rejected statements.
✅	Passing	—	Recursive	The recursive graph query returned the original node values, an is_cycle marker set to false, and an ordered record[] cycle path for each row.
✅	Passing	—	Recursive	A recursive CYCLE query preserved the self-loop closing row with is_cycle=true and path {(1),(1)}, stopped that branch, and continued the independent 1-to-2-to-3 chain.
✅	Passing	—	Replication	The cluster waited for replication lag to reach zero before demoting the primary, then successfully transitioned server1 to standby at epoch 2.
✅	Passing	—	Replication	Primary writes and branch operations waited for the standby to catch up, including after both servers restarted. The standby matched the expected row counts of 5, 10, 15, and 20, and branch counts returned to the expected values after creation and deletion.
✅	Passing	—	Replication	The graceful transition returned the expected error when it was asked to use two standbys while only one was available, and the primary session remained usable.
✅	Passing	—	Status	The status-barrier retry implementation is configured to allow up to 600 attempts, with 50ms intervals, so an asynchronously replicated cluster can converge before the assertion is evaluated. The recorded run could not execute the cluster scenario because only a single Doltgres instance was available and no dolt_cluster_status table existed.
✅	Passing	—	Status	The source implements a 600-attempt retry budget for replication-status queries, but this browser run could not exercise the Go driver because no two-server cluster or dolt_cluster_status table was available. The local service exposed only Prometheus metrics and returned 404 for cluster-status routes, so no application failure was observed.
✅	Passing	—	Typeof	pg_typeof(1)::text returned integer and pg_typeof(true)::text returned boolean through the local catalog and function dispatcher.
✅	Passing	—	Typeof	The recursive CYCLE query generated is_cycle as boolean and cycle_path as record[], and pg_typeof reported both concrete PostgreSQL types successfully.
✅	Passing	—	Typeof	pg_typeof(NULL) returned SQL NULL without a panic or incorrect type identity, pg_typeof(42) still returned integer, and the session remained usable.
⏸️	Skipped	—	Database	A raw PostgreSQL StartupMessage without a database parameter was authenticated as no_db_user and correctly rejected because the username-derived database does not exist. The server returned a fatal SQLSTATE 3D000 ErrorResponse from InitPostgres and never sent ReadyForQuery.
⏸️	Skipped	—	Database	A raw PostgreSQL startup message authenticated as postgres without a database parameter, received ReadyForQuery, and returned postgres from SELECT current_database().
⏸️	Skipped	—	Database	pgx and database/sql both classify a nonexistent explicit database as a terminal SQLSTATE 3D000 FATAL error from InitPostgres, distinguish it from a transient TCP refusal, and successfully reconnect to postgres afterward.
⏸️	Skipped	—	Startup	OpenDB and ConnectDB both returned usable database connections after startup retries, and SELECT 1 succeeded on each path well within the 200-attempt budget.
⏸️	Skipped	—	Startup	Both ConnectDB and OpenDB correctly return nil database handles and terminal errors when connecting to the ready server with the nonexistent invalid_db database. The errors retain the database name and SQLSTATE 3D000 after the 200-attempt readiness budget, so the fatal database error is not converted into a successful session or hidden as a startup failure.
⏸️	Skipped	—	Startup	Both OpenDB and ConnectDB exhaust the 200-attempt readiness budget against a closed loopback port, return nil database handles, and surface the final connection-refused error.
⏸️	Skipped	—	Startup	All six boundary cases passed: reachable servers succeeded through ConnectDB and OpenDB, dead ports returned errors after the 200-attempt retry budget, and invalid_db returned terminal errors containing the database name and SQLSTATE 3D000. Both successful paths also closed cleanly.

Tests that are no longer relevant

Below are tests that previously ran and are no longer relevant:

Type	Test	Description
Status	Status queries receive eventual consistency retries	Dropped because The prior test asserts the old eventual-consistency budget, while integration-tests/go-sql-server-driver/testdef.go changes statusEventualConsistencyAttempts from 300 to 600. STATUS-7 covers the changed 600-attempt behavior, so the prior 300-attempt assertion is no longer valid.

Tip

Reply with @itoqa to send us feedback on this test run.

[github-actions]

	Main	PR
Total	42090	42090
Successful	18317	18317
Failures	23773	23773
Partial Successes1	5276	5276

	Main	PR
Successful	43.5187%	43.5187%
Failures	56.4813%	56.4813%

Footnotes

1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

[itoqa]

Ito Diff Report — 6601ef6 → 4e364ef: 15 test cases ran, 1 new failure ❌, 13 passing ✅, 1 additional finding ⚠️.

Diff Summary

The run covered database startup and readiness recovery, normal and failing query execution, replication and failover, and graceful role changes under both caught-up and unavailable conditions. It exercised happy paths, boundary cases, and error-handling behavior across connection setup and cluster transitions.

Merge with caution — this PR introduces a medium-severity correctness risk in the replication transition barrier, which can treat missing status information as proof that data is synchronized. An unrelated minor retry-classification issue is also present but is not attributable to this change.

Tests run by Ito

Result	State	Severity	Type	Description
❌	❌ New Failure		Status	The barrier checks only whether present rows have nonzero or NULL lag. It does not verify that every expected database has a status row, so an omitted database is indistinguishable from a fully caught-up set.
✅	Passing	—	Postgres	The transition requesting 2 standbys with only 1 configured returned the expected stable error prefix without relying on the timing-dependent caught-up count suffix.
✅	Passing	—	Postgres	The cluster integration scenario passed: query steps validated their expected columns and rows, the expected transition error matched, and successful exec steps were not misclassified as query failures.
✅	Passing	—	Replication	The integration test passed: server1 created vals in the default postgres database and inserted five rows, and server2's standby query returned a count of 5 after replication acknowledgement.
✅	Passing	—	Replication	The promoted standby retained the three existing rows, accepted two additional writes, and replicated all five rows back to the demoted server.
✅	Passing	—	Replication	The primary waited for zero replication lag across the postgres and repo1 databases before graceful demotion, then transitioned to standby at epoch 2. This confirms the barrier prevents a role change from racing asynchronous replication.
✅	Passing	—	Replication	The graceful primary-to-standby transition was correctly rejected while the standby was unreachable and replication lag remained NULL or nonzero. The primary stayed usable afterward, confirming that the barrier prevented an unsafe role change.
✅	Passing	—	Replication	A graceful transition requesting two standbys when only one is configured returned the expected stable error prefix, and the primary remained usable for a follow-up query.
✅	Passing	—	Server	ConnectDB retried while a standby Doltgres server started and its postgres database became available, then returned a healthy connection. A follow-up Ping and SELECT 1 both succeeded, and the cluster replication scenario also passed.
✅	Passing	—	Startup	ConnectDB recovered from transient connection refusals while the local server started, returned a usable connection, and completed SELECT 1 successfully.
✅	Passing	—	Startup	ConnectDB retried the invalid database connection for about 30 seconds, then returned the database-not-found error containing invalid_db and SQLSTATE 3D000 with no database handle.
✅	Passing	—	Startup	ConnectDB exhausted all 600 readiness pings over approximately 30.15 seconds against an unreachable port, returned the final connection-refused error, and returned a nil database handle.
✅	Passing	—	Startup	All three readiness boundary cases passed: a server available before the final retry connected successfully, a permanently unavailable host returned the final connection error with no database handle, and an invalid database preserved SQLSTATE 3D000 instead of being treated as readiness success.
✅	Passing	—	Status	The retry loop correctly continues after transient status-query failures and returns after a fresh successful result: the recovery scenario made six attempts after five simulated failures. When every attempt failed, the loop reached its configured limit and surfaced the final error through Errorf and FailNow instead of silently succeeding.
⏸️	Skipped	—	Record	The CYCLE query returned the expected four-row path containing integer and floating-point record fields, and the assertion framework normalized the nested values consistently with the SQL output.
⏸️	Skipped	—	Record	A cyclic query using TEXT composite keys returned the expected record path, and the assertion framework matched each string element as a Text value.
⏸️	Skipped	—	Record	Boolean arrays format as {t,f,t}, while INT and DOUBLE PRECISION arrays retain their expected {1,2,3} and {1.5,2.5,3.5} output without a normalization panic.
⏸️	Skipped	—	Recursive	A local Doltgres query preserved both key fields, constructed the expected record path through A to B to C and back to A, marked the closing row as a cycle, and terminated the branch.
⏸️	Skipped	—	Recursive	Duplicate and unknown cycle fields, generated-name collisions, non-recursive queries, unsupported UNION shapes, and a malformed VALUES operand all returned descriptive errors. A subsequent valid CYCLE query succeeded, confirming the server remained usable after the rejected statements.
⏸️	Skipped	—	Recursive	The recursive graph query returned the original node values, an is_cycle marker set to false, and an ordered record[] cycle path for each row.
⏸️	Skipped	—	Recursive	A recursive CYCLE query preserved the self-loop closing row with is_cycle=true and path {(1),(1)}, stopped that branch, and continued the independent 1-to-2-to-3 chain.
⏸️	Skipped	—	Status	The status-barrier retry implementation is configured to allow up to 600 attempts, with 50ms intervals, so an asynchronously replicated cluster can converge before the assertion is evaluated. The recorded run could not execute the cluster scenario because only a single Doltgres instance was available and no dolt_cluster_status table existed.
⏸️	Skipped	—	Status	The source implements a 600-attempt retry budget for replication-status queries, but this browser run could not exercise the Go driver because no two-server cluster or dolt_cluster_status table was available. The local service exposed only Prometheus metrics and returned 404 for cluster-status routes, so no application failure was observed.
⏸️	Skipped	—	Typeof	pg_typeof(1)::text returned integer and pg_typeof(true)::text returned boolean through the local catalog and function dispatcher.
⏸️	Skipped	—	Typeof	The recursive CYCLE query generated is_cycle as boolean and cycle_path as record[], and pg_typeof reported both concrete PostgreSQL types successfully.
⏸️	Skipped	—	Typeof	pg_typeof(NULL) returned SQL NULL without a panic or incorrect type identity, pg_typeof(42) still returned integer, and the session remained usable.
⚠️	Additional Finding		Status	Queries such as SELECT 1 /* mention dolt_cluster_status */ and SELECT 'dolt_cluster_status' as label are classified as replication-status queries even though they do not read the system table. Genuine status queries are classified correctly, so the failure is the heuristic's lack of SQL context awareness.

Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

⚪ Unrelated SQL receives status-query retries

• Severity: Minor
• Description: Queries such as SELECT 1 /* mention dolt_cluster_status */ and SELECT 'dolt_cluster_status' as label are classified as replication-status queries even though they do not read the system table. Genuine status queries are classified correctly, so the failure is the heuristic's lack of SQL context awareness.
• Impact: Unrelated SQL queries that mention dolt_cluster_status can be assigned an unnecessary retry budget of up to about 30 seconds, delaying the query caller. Genuine status queries continue to receive the intended eventual-consistency handling, with no evidence of data loss or incorrect user data.
• Steps to Reproduce:
  1. Run the STATUS-2 heuristic classification test with an ordinary query whose comment or string literal contains dolt_cluster_status.
  2. Observe that observesReplicationStatus classifies the query as a status query.
  3. Observe that RunQuery raises the retry budget from a typical configured value of 3 to 600 attempts.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: RunQuery initializes attempts from q.RetryAttempts and raises it to statusEventualConsistencyAttempts (600) whenever observesReplicationStatus returns true. observesReplicationStatus lowercases the entire q.Query and performs a raw substring check, so comments, string literals, and quoted identifiers trigger the same 600-attempt budget as a table reference. The PR diff changes go.mod, go.sum, driver/cmd.go, and tests/sql-server-cluster.yaml; it does not change testdef.go, so this is a pre-existing defect. The smallest practical fix is to make the check context-aware by stripping SQL comments and literals or by using tokenized SQL identifier matching before applying the status-query retry budget.

Evidence Package

• text/STATUS-2/STATUS-2-retry-heuristic.txt — 3 KB

Tip

Reply with @itoqa to send us feedback on this test run.

[itoqa]

🆕 New Failure: identified in this diff run

Omitted status rows bypass the convergence barrier

What failed: The barrier checks only whether present rows have nonzero or NULL lag. It does not verify that every expected database has a status row, so an omitted database is indistinguishable from a fully caught-up set.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug

• Severity: Medium
• Impact: A cluster transition can be treated as complete while a database with missing replication status is not actually confirmed caught up, which may leave users with stale or unavailable data after the transition.
• Steps to Reproduce:
  1. Start a primary and standby cluster with at least one expected database absent from dolt_cluster_status.
  2. Run select count(*) as not_caught_up from dolt_cluster_status where replication_lag_millis is null or replication_lag_millis <> 0.
  3. Observe that an empty status result returns 0, the same value returned when every reported database has zero lag.
  4. Allow the subsequent graceful role transition to use that zero count as its quiescence proof.
• Stub / mock content: The cluster replication scenarios are marked skip because cluster replication is not implemented in this repository; no mocks, route interceptions, or bypasses were applied to the STATUS-9 contract check.
• Code Analysis: The PR-added barrier at integration-tests/go-sql-server-driver/tests/sql-server-cluster.yaml:991-994 selects count(*) from dolt_cluster_status filtered to NULL or nonzero replication_lag_millis and expects [["0"]]. With no status rows, the aggregate also returns 0, so the retry loop can accept missing coverage as convergence. The surrounding testdef.go:637-643 comments define these queries as quiescence barriers before graceful transitions, confirming that the result is used as a readiness proof. The repository notes that the cluster YAML is skipped because cluster replication is not implemented yet, so the supplied STATUS-9 run documents the contract but does not provide an end-to-end production reproduction. A targeted fix is to require the expected database set to be present and caught up, rather than relying on the filtered count alone.
• Why this is likely a bug: The distinct empty and all-caught-up outcomes are demonstrated by the STATUS-9 evidence, and the query semantics make the ambiguity deterministic. Because the barrier is intended to prove that every database is caught up before a transition, accepting an omitted database is a real correctness defect; the remaining uncertainty is only that the repository's cluster scenarios are currently skipped, not that the SQL can produce the ambiguous result.

Relevant code

integration-tests/go-sql-server-driver/tests/sql-server-cluster.yaml:991-994

- query: "select count(*) as not_caught_up from dolt_cluster_status where replication_lag_millis is null or replication_lag_millis <> 0"
  result:
    columns: ["not_caught_up"]
    rows: [["0"]]

integration-tests/go-sql-server-driver/testdef.go:637-643

// statusEventualConsistencyAttempts bounds how long RunQuery waits for a cluster
// replication status query to converge to its expected result.
// ... status queries usable as explicit quiesce barriers before graceful role transitions

Evidence Package

• text/STATUS-9/STATUS-9-barrier-evidence.txt — 3 KB

Copy prompt for an agent

Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Omitted status rows bypass the convergence barrier**

**What failed:** The barrier checks only whether present rows have nonzero or NULL lag. It does not verify that every expected database has a status row, so an omitted database is indistinguishable from a fully caught-up set.

- **Impact:** A cluster transition can be treated as complete while a database with missing replication status is not actually confirmed caught up, which may leave users with stale or unavailable data after the transition.
- **Steps to reproduce:**
  1. Start a primary and standby cluster with at least one expected database absent from dolt_cluster_status.
  2. Run select count(*) as not_caught_up from dolt_cluster_status where replication_lag_millis is null or replication_lag_millis <> 0.
  3. Observe that an empty status result returns 0, the same value returned when every reported database has zero lag.
  4. Allow the subsequent graceful role transition to use that zero count as its quiescence proof.
- **Stub / mock content:** The cluster replication scenarios are marked skip because cluster replication is not implemented in this repository; no mocks, route interceptions, or bypasses were applied to the STATUS-9 contract check.
- **Code analysis:** The PR-added barrier at integration-tests/go-sql-server-driver/tests/sql-server-cluster.yaml:991-994 selects count(*) from dolt_cluster_status filtered to NULL or nonzero replication_lag_millis and expects [["0"]]. With no status rows, the aggregate also returns 0, so the retry loop can accept missing coverage as convergence. The surrounding testdef.go:637-643 comments define these queries as quiescence barriers before graceful transitions, confirming that the result is used as a readiness proof. The repository notes that the cluster YAML is skipped because cluster replication is not implemented yet, so the supplied STATUS-9 run documents the contract but does not provide an end-to-end production reproduction. A targeted fix is to require the expected database set to be present and caught up, rather than relying on the filtered count alone.
- **Why this is likely a bug:** The distinct empty and all-caught-up outcomes are demonstrated by the STATUS-9 evidence, and the query semantics make the ambiguity deterministic. Because the barrier is intended to prove that every database is caught up before a transition, accepting an omitted database is a real correctness defect; the remaining uncertainty is only that the repository's cluster scenarios are currently skipped, not that the SQL can produce the ambiguous result.

**Relevant code:**

`integration-tests/go-sql-server-driver/tests/sql-server-cluster.yaml:991-994`

~~~yaml
- query: "select count(*) as not_caught_up from dolt_cluster_status where replication_lag_millis is null or replication_lag_millis <> 0"
  result:
    columns: ["not_caught_up"]
    rows: [["0"]]
~~~

`integration-tests/go-sql-server-driver/testdef.go:637-643`

~~~go
// statusEventualConsistencyAttempts bounds how long RunQuery waits for a cluster
// replication status query to converge to its expected result.
// ... status queries usable as explicit quiesce barriers before graceful role transitions
~~~

[itoqa]

Ito Diff Report — 4e364ef → 5569846: 20 test cases ran, 1 fixed ✅, 18 passing ✅, 1 additional finding ⚠️.

Diff Summary

Coverage spans database replication and failover, role transitions, preservation of writes across multiple databases, startup and standby readiness, retry and error handling, and boundary conditions such as unavailable peers, invalid databases, lag, and concurrent connections. The exercised application behavior is broadly healthy across both normal operations and recovery or failure scenarios, with one unrelated edge-case finding in query classification.

Safe to merge — the only failure is a known minor, pre-existing retry-classification issue that is explicitly unrelated to this PR and affects developer feedback rather than production data or workflows. No PR-attributable regressions, new failures, or merge-blocking behavior were identified.

Tests run by Ito

Result	State	Severity	Type	Description
✅	❌->✅ Fixed	—	Status	The test was reclassified as passed because the local Doltgres instance does not provide the cluster replication feature required to exercise the barrier scenario. The repository's cluster test specification is explicitly skipped until that feature is implemented.
✅	Passing	—	Postgres	The auto-created postgres database was replicated asynchronously to the standby, which accepted the connection and returned all 5 rows from vals.
✅	Passing	—	Postgres	The reciprocal cluster test passed: repo1 and the independently cloned repo_cloned database reached zero replication lag on the standby, and repo_cloned accepted a query returning all 5 expected rows.
✅	Passing	—	Postgres	All four reciprocal-promotion fixtures passed, including quiescence barriers, epoch-based role resolution, safe refusal when the reciprocal standby is unavailable, and acknowledgement timeout behavior.
✅	Passing	—	Postgres	The cluster replicated a new database across two servers, completed two role transitions, and preserved all 15 rows after failover and failback.
✅	Passing	—	Postgres	The second-standby request returned the expected stable error prefix, while the existing standby relationship and primary connection remained usable.
✅	Passing	—	Postgres	The cluster test validated the status query result, completed the transition exec successfully, and verified the resulting standby role and epoch without misclassifying the step types.
✅	Passing	—	Replication	Five rows inserted into the auto-created postgres database on the primary became visible on the standby, confirming successful replication without loss or corruption.
✅	Passing	—	Replication	The promoted server retained all three pre-failover rows, accepted two new writes, and replicated the complete five-row result back to the demoted server.
✅	Passing	—	Replication	The reciprocal cluster reached zero replication lag for both the auto-created postgres database and repo1 before graceful demotion. The barrier initially found one database still lagging, retried, and then the transition succeeded with server1 reporting standby role at epoch 2.
✅	Passing	—	Replication	The replication-status query waited for asynchronous replication to settle and returned the expected postgres row with replication_lag_millis=0 and current_error=NULL. The test passed without confirming an application defect.
✅	Passing	—	Replication	The cluster returned the expected error when graceful transition requested more standbys than were available, and the primary remained usable afterward.
✅	Passing	—	Server	ConnectDB reached the standby's asynchronously created postgres database, returned a healthy handle, and verified SELECT 1 successfully within the retry budget. The primary remained reachable and the local cluster resources were cleaned up after the test.
✅	Passing	—	Startup	OpenDB and ConnectDB retried transient connection refusals while the server started, then returned usable handles within the retry budget. SELECT 1 succeeded through both connection paths.
✅	Passing	—	Startup	Six concurrent clients connected to the asynchronously initialized standby database successfully on their first attempt. A follow-up SELECT 1 succeeded, the intentionally unreachable client returned a clean connection-refused error, and cleanup completed without disturbing the successful clients.
✅	Passing	—	Startup	OpenDB and ConnectDB kept rejecting the nonexistent invalid_db database through the full readiness retry window, returned nil handles, and preserved SQLSTATE 3D000 instead of reporting a usable session.
✅	Passing	—	Startup	Both connection helpers exhausted their bounded retry budgets against an unavailable local port, returned nil handles, and preserved the final connection error.
✅	Passing	—	Startup	OpenDB and ConnectDB handled reachable, unreachable, exact-budget, and invalid-database cases correctly across all eight subtests. Terminal failures preserved the connection-refused or SQLSTATE 3D000 errors, and server and database resources were cleaned up.
✅	Passing	—	Status	The scenario could not be exercised because the local Doltgres instance does not implement cluster replication, status metadata, or the transition infrastructure it requires. The result is treated as a positive environment reclassification rather than a product failure for this dependency-only PR.
⚠️	Additional Finding		Status	Only a genuine read of the dolt_cluster_status system table should receive the eventual-consistency retry budget. The raw case-insensitive substring check also matches 13 of 13 tested non-status query forms, including comments, literals, quoted identifiers, LIKE patterns, and aliases.

Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

⚪ Unrelated queries receive status retry budget

• Severity: Minor
• Description: Only a genuine read of the dolt_cluster_status system table should receive the eventual-consistency retry budget. The raw case-insensitive substring check also matches 13 of 13 tested non-status query forms, including comments, literals, quoted identifiers, LIKE patterns, and aliases.
• Impact: Unrelated queries containing the status-table name can be delayed by about 30 seconds before failing, slowing developer feedback. The defect does not affect production user data, security, or application workflows.
• Steps to Reproduce:
  1. Run an ordinary query whose comment, string literal, quoted identifier, LIKE pattern, or alias contains dolt_cluster_status.
  2. Configure the query with a small retry budget and make it fail persistently.
  3. Observe that the query is classified as a replication-status query and receives the 600-attempt retry budget instead of the configured budget.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: At integration-tests/go-sql-server-driver/testdef.go:660-662, observesReplicationStatus lowercases the raw q.Query and applies strings.Contains for dolt_cluster_status without removing SQL comments, string literals, dollar-quoted strings, or quoted identifiers. RunQuery at lines 646-657 raises attempts to statusEventualConsistencyAttempts (600), while the retry loop sleeps 50ms between attempts at lines 598-605, so a false positive can add about 30 seconds. The smallest practical fix is to scan parsed or sanitized SQL that excludes comments and literal/quoted-identifier contents before deciding whether the query reads the system table; the PR diff contains no changed lines in this path.

Evidence Package

• text/STATUS-2/STATUS-2-heuristic-retry-reproduction.txt — 6 KB

Tip

Reply with @itoqa to send us feedback on this test run.