fix: only check zero affected rows on first provisioning query

[c1-dev-bot]

Summary

• Fix multi-query revoke transactions being silently rolled back by the rowsAffected == 0 check in RunProvisioningQueries
• Only apply the zero-rows check to the first query in a provisioning block, allowing secondary/conditional queries to legitimately affect 0 rows
• The first query represents the primary operation (e.g., DELETE role assignment); zero rows there correctly signals "already granted/revoked"

Problem

The rowsAffected == 0 check (introduced in v0.5.2, commit d80deff) applies to all queries in a multi-query provisioning block. When a revoke config has a conditional second query (e.g., UPDATE user deactivation that only fires on last role removal), the second query may legitimately affect 0 rows. This triggers ErrQueryAffectedZeroRows, which:

1. Rolls back the entire transaction (including the successful first DELETE)
2. Is caught by Revoke() as GrantAlreadyRevoked
3. Returns nil error — task reports success
4. Database is unchanged

Fix

Change the loop in RunProvisioningQueries from for _, q := range queries to for i, q := range queries, and gate the zero-rows check with i == 0. This preserves the useful "already granted/revoked" detection for the primary query while allowing secondary queries to affect 0 rows.

Test plan

• Verify single-query grant/revoke still returns ErrQueryAffectedZeroRows when 0 rows affected
• Verify multi-query revoke commits successfully when first query affects 1 row and second affects 0 rows
• Verify multi-query revoke still rolls back when first query affects 0 rows
• Verify rowsAffected > 1 check still applies to all queries

Fixes: CXH-1341

---

Automated PR Notice

This PR was automatically created by c1-dev-bot as a potential implementation.

This code requires:

• Human review of the implementation approach
• Manual testing to verify correctness
• Approval from the appropriate team before merging

[linear]

CXH-1341 baton-sql v0.5.2: multi-query revoke transactions silently rolled back by zero-rows check

[github-actions]

Claude finished @c1-dev-bot[bot]'s task in 1m 22s —— View job

---

PR Review: fix: only check zero affected rows on first provisioning query

• Read changed file and surrounding context
• Trace error handling through callers (Grant/Revoke)
• Check for existing tests
• Post review feedback

---

Summary

Clean, minimal, well-targeted fix. The change is correct and the PR description explains the problem clearly.

Review

The fix is sound. The i == 0 guard correctly scopes the zero-rows check to only the primary query. The key invariant — "if the primary operation (DELETE/INSERT) affected 0 rows, the grant/revoke was already applied" — is preserved, while secondary conditional queries (e.g., deactivation UPDATEs) are allowed to be no-ops.

Observations:

1. Single-query blocks are unaffected — i == 0 is always true when there's exactly one query, so existing behavior is preserved.

2. rowsAffected > 1 still guards all queries (line 392) — good safety net, this is correct.

3. Convention assumption: This fix assumes the first query is always the "primary" operation. That's a reasonable convention for baton-sql configs, but worth noting — if someone ever structures a multi-query block with the primary operation second, this check wouldn't catch it. Consider documenting this expectation in the config schema or provisioning docs.

4. Pre-existing issue (not introduced by this PR): When result.RowsAffected() errors at line 388, the code logs but continues with rowsAffected == 0. For the first query this would incorrectly trigger ErrQueryAffectedZeroRows. For subsequent queries, it's now silently ignored. Not a regression, just something to be aware of.

5. No tests exist for RunProvisioningQueries. The test plan in the PR description is solid — it would be valuable to add tests covering the scenarios listed there, particularly:

   • Multi-query where first query affects 1 row and second affects 0 → should commit
   • Multi-query where first query affects 0 rows → should roll back
   • Single-query affecting 0 rows → should return ErrQueryAffectedZeroRows

6. Left an inline suggestion for a debug log on zero-row secondary queries — not blocking, just helpful for operator visibility.

Verdict: Approve with minor suggestions. The core logic change is correct and well-scoped.

[github-actions]

nit: Consider adding a debug log when a secondary query affects 0 rows, so operators have visibility into when this happens during troubleshooting. Something like:

Suggested change

	if i == 0 && rowsAffected == 0 {
	return ErrQueryAffectedZeroRows
	}
	if i == 0 && rowsAffected == 0 {
	return ErrQueryAffectedZeroRows
	}
	if rowsAffected == 0 {
	l.Debug("secondary query affected 0 rows", zap.Int("query_index", i), zap.String("query", q))
	}

Not blocking — the current behavior is correct. But silently continuing on 0-row secondary queries could make debugging harder if a config mistake causes a secondary query to be a no-op when it shouldn't be.