Add SQLite outbox pattern support (issue #149)#151
Merged
Conversation
… Validate Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ath normalization Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ric path normalization Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…SqlServer shape Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lerAsync Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds RelationalSendMessageCommand fork check inside Handle() and a new HandleExternalTransaction private method that reuses the caller's SQLiteTransaction/SQLiteConnection without committing or disposing them. Mirrors the PG/SqlServer pattern (PG line 106). Uses fully-qualified System.Data.SQLite.SQLiteTransaction to avoid collision with the DotNetWorkQueue.Transport.SQLite.Basic.SQLiteTransaction wrapper. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ueInit Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…it tests 6 tests: 3 base-behavior preservation checks + .db3 round-trip, :memory: mode, and mismatch-with-normalized-values for SQLite-specific normalization. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
8 tests covering null-transaction guards, SQLiteTransaction cast guard, validator-fires-before-guard ordering, batch validator-once-per-batch, and type-hierarchy assertions. Guard-pass happy path deferred to Phase 3 integration tests (SQLiteConnection is sealed/unmockable). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ive-path flip) The corresponding delete of SqliteProducerDoesNotImplementRelationalTests.cs landed earlier in commit 58cb3e0 (PLAN-1.1 scope leak — the extractor-tests commit included the unrelated delete). This commit closes PLAN-1.4 by adding the positive-assertion replacement file. End state matches CONTEXT-2 D2. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wave 1 base class for SQLite outbox integration tests. Mirrors PostgreSqlOutboxIntegrationTestBase shape with SQLite-specific adaptations: TEXT column type, Convert.ToInt64 for INTEGER affinity, explicit-interface caveat for EnableHoldTransactionUntilMessageCommitted (hardwired false on SQLite), BEGIN EXCLUSIVE serialization documented in XML doc, WSL interop-lock workaround documented in XML doc. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PLAN-2.1. 4 test methods × [DataRow(true)] [DataRow(false)] = 8 cases. Covers DB-name mismatch, .db3 path-normalization, null-tx, and guard-pass happy path (closes Phase 2 deferral). Local test runs fail with libdl.so missing on WSL host — pre-existing environment issue affecting all SQLite integration tests; Jenkins Docker agents have libdl and will execute the suite. If Jenkins surfaces a real validator path-comparison defect, fix forward in a follow-up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PLAN-2.2. 4 sync methods × [DataRow(true)] [DataRow(false)] = 8 cases: Commit_SingleSend, Rollback_SingleSend, Commit_Batch, Rollback_Batch. Real SQLiteConnection + BeginTransaction round-trip. Local libdl.so WSL issue applies; Jenkins is the source of truth. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Phase 3 PLAN-2.5 review surfaced that SQLite's RetryCommandHandlerOutput- Decorator and Async variant lack the `if (command is IRetrySkippable && SkipRetry) return _decorated.Handle(command);` guard that PG and SqlServer have (line 54/55 in their respective files). Without it, outbox-pattern Send(...) with caller-managed transactions still goes through the Polly retry loop on SQLite, defeating the IRetrySkippable contract that RelationalSendMessageCommand carries. Mirrors SqlServer/PG shape exactly: adds `using DotNetWorkQueue.Transport. RelationalDatabase;` and the early-return guard at the top of Handle / HandleAsync. Closes a Phase 1 gap surfaced by Phase 3 integration testing — exactly the kind of finding integration tests should catch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ts update Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All anchor targets resolve, no Tx abbreviation, identifier capitalization correct. No content edits required. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Phase 4 review caught a Critical accuracy defect in the SQLite Quick Start note: the example referenced `SqliteConnection`/`SqliteTransaction`, which are the Microsoft.Data.Sqlite provider's type names. This transport is built on System.Data.SQLite, whose types are `SQLiteConnection` and `SQLiteTransaction` (note all-caps `SQLite`). A reader copy-pasting the wrong names would compile against the wrong provider package and hit `InvalidOperationException` at the first Send call from GuardSQLiteTransaction. Three corrections: - `#### SQLite note` lines 108-109: rename to `SQLiteConnection`/ `SQLiteTransaction` and explicitly disambiguate "from the System.Data.SQLite provider — not Microsoft.Data.Sqlite" - Capability-cast comment line 62: add SQLite to the supported-transports list (was SqlServer + PostgreSQL only) - `busy_timeout` bullet line 286-289: replace internal `IntegrationConnectionInfo` reference with caller-facing guidance "On the open SQLiteConnection before calling BeginTransaction()" Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tone Adds 4 tactical lessons to CLAUDE.md ## Lessons Learned section, distilled from the SQLite Outbox Pattern Support milestone (issue #149): - Decorator-chain parity audit when porting features across transports (caught the IRetrySkippable gap in commit 66f7e86) - System.Data.SQLite vs Microsoft.Data.Sqlite provider name distinction (caught the doc accuracy defect in commit 447250f) - SQLiteConnectionStringBuilder.DataSource returns empty for FullUri= (latent risk, not yet observed in CI) - Team-mode Agent dispatch shares parent task list with teammates (Phase 1 Wave 2 autopilot incident; 3 commits reverted) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds caller-transaction outbox support for SQLite: makes base validator extensible, adds SQLite extractor/validator, implements a SQLite relational producer and handler forks that reuse caller transactions, wires retry-bypass, adds unit/integration tests, DI wiring, and documents SQLite-specific concurrency caveats. ChangesSQLite Outbox Pattern Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxIntegrationTestBase.cs (1)
109-110: ⚡ Quick winReplace
hold-txwithhold-transactionin comments.Use the full word for consistency with repository naming rules.
As per coding guidelines, "Do not use the
Txabbreviation for 'transaction' — use the full word 'transaction' in all identifiers and documentation."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxIntegrationTestBase.cs` around lines 109 - 110, Update the comment text in the SqliteOutboxIntegrationTestBase class to use the full word "hold-transaction" instead of the abbreviation "hold-tx"; search for occurrences in the inline comment shown and the class XML doc above (and any other comments in that file) and replace "hold-tx" with "hold-transaction" to comply with the repository naming guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxIntegrationTestBase.cs`:
- Around line 147-155: Add an ActivityListener before any sends in the
SqliteOutboxIntegrationTestBase so trace decorators are invoked (register an
ActivityListener via ActivitySource.AddActivityListener at test setup or inside
CreateRelationalProducer before using the producer pipeline); harden disposal in
QueueScope.Dispose and ProducerScope.Dispose by adding an atomic guard (use an
Interlocked flag/boolean to ensure Dispose is executed only once) to prevent
double-dispose races; and update the inline comment that uses the abbreviation
"hold-tx" to read "hold transaction" for clarity. Ensure you reference
CreateRelationalProducer when adding the ActivityListener and modify
QueueScope.Dispose and ProducerScope.Dispose implementations to use Interlocked
operations and adjust the comment text accordingly.
- Around line 73-85: QueueScope.Dispose and ProducerScope.Dispose need to be
made atomic/idempotent: add a private int _disposed field to each of the nested
classes and at the start of Dispose use Interlocked.Exchange(ref _disposed, 1)
(or Interlocked.CompareExchange) to run the cleanup body only when the previous
value was 0; keep the existing cleanup calls (OCreation?.RemoveQueue(),
OCreation?.Dispose(), Scope?.Dispose(), QueueCreator?.Dispose() in QueueScope
and the corresponding disposals in ProducerScope) but wrap them so they execute
only on the first successful exchange to ensure Dispose is thread-safe and only
runs once.
In
`@Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxRetryBypassTests.cs`:
- Around line 67-77: The test currently catches any Exception into the variable
thrown after calling producer.RelationalProducer.Send(new FakeMessage(), new
AdditionalMessageData(), completedTransaction) which can mask unrelated fast
failures; change the assertion to verify the specific expected exception type
and, if applicable, the expected exception message (e.g., assert thrown is the
expected exception type and thrown.Message equals the expected text) for this
completed-transaction path, and apply the same tighter assertion to the other
similar block (the one at lines 85-87); reference the Send call, the thrown
variable, FakeMessage/AdditionalMessageData and completedTransaction when making
the replacement.
In
`@Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxValidationTests.cs`:
- Around line 134-136: The test currently only checks DB row visibility but
doesn't assert the API result from the happy-path call; update the test in
SqliteOutboxValidationTests to capture the return value of
producer.RelationalProducer.Send(FakeMessage, AdditionalMessageData,
transaction) (or any overload used) and assert it's a success/empty-error per
the Send contract (e.g., not null/false/no exception or matches expected result
type), ensuring the call to Send(...) explicitly asserts the expected success
result in addition to the existing DB assertions.
In
`@Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.cs`:
- Around line 253-255: The handler currently downcasts
relCommand.ExternalTransaction to System.Data.SQLite types in
SendMessageCommandHandler and SendMessageCommandHandlerAsync; replace those
sealed casts by treating relCommand.ExternalTransaction as IDbTransaction and
obtaining the connection via transaction.Connection cast to IDbConnection so the
code relies on IDbTransaction/IDbConnection abstractions (use
relCommand.ExternalTransaction as IDbTransaction, var transaction =
(IDbTransaction)relCommand.ExternalTransaction; use transaction.Connection as
IDbConnection for command/connection access), updating any variable names
(sqliteTransaction/sqliteConn) and usages to the IDb* types to remove the
provider coupling.
In
`@Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs`:
- Around line 255-256: Replace provider-specific SQLite casts in
HandleExternalTransactionAsync: stop downcasting
RelationalSendMessageCommand.ExternalTransaction to
SQLiteTransaction/SQLiteConnection and instead use IDbTransaction and
IDbConnection locals (e.g., var dbTx =
(IDbTransaction)relCommand.ExternalTransaction; var dbConn =
(IDbConnection)dbTx.Connection). Update call sites that pass
sqliteConn/sqliteTransaction into DoesJobExistQuery<IDbConnection,
IDbTransaction> and SetJobLastKnownEventCommand<IDbConnection, IDbTransaction>
to pass dbConn/dbTx. Make the same change in the sync handler
SendMessageCommandHandler.cs so both handlers are provider-agnostic.
---
Nitpick comments:
In
`@Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxIntegrationTestBase.cs`:
- Around line 109-110: Update the comment text in the
SqliteOutboxIntegrationTestBase class to use the full word "hold-transaction"
instead of the abbreviation "hold-tx"; search for occurrences in the inline
comment shown and the class XML doc above (and any other comments in that file)
and replace "hold-tx" with "hold-transaction" to comply with the repository
naming guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed9adb6b-0ce2-4775-bb71-b9107778d7a8
📒 Files selected for processing (22)
CLAUDE.mdSource/DotNetWorkQueue.Transport.RelationalDatabase/Basic/ExternalTransactionValidator.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxAdditionalDataTests.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxIntegrationTestBase.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxRetryBypassTests.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxSendAsyncTests.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxSendTests.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxValidationTests.csSource/DotNetWorkQueue.Transport.SQLite.Tests/Basic/SqLiteExternalTransactionValidatorTests.csSource/DotNetWorkQueue.Transport.SQLite.Tests/Basic/SqliteExternalDbNameExtractorTests.csSource/DotNetWorkQueue.Transport.SQLite.Tests/Basic/SqliteProducerDoesNotImplementRelationalTests.csSource/DotNetWorkQueue.Transport.SQLite.Tests/Basic/SqliteProducerImplementsRelationalTests.csSource/DotNetWorkQueue.Transport.SQLite.Tests/Basic/SqliteRelationalProducerQueueTests.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqLiteExternalDbNameExtractor.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqLiteExternalTransactionValidator.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqLiteMessageQueueInit.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqliteRelationalProducerQueue.csSource/DotNetWorkQueue.Transport.SQLite/Decorator/RetryCommandHandlerOutputDecorator.csSource/DotNetWorkQueue.Transport.SQLite/Decorator/RetryCommandHandlerOutputDecoratorAsync.csdocs/outbox-pattern.md
💤 Files with no reviewable changes (1)
- Source/DotNetWorkQueue.Transport.SQLite.Tests/Basic/SqliteProducerDoesNotImplementRelationalTests.cs
Comment on lines
+255
to
+256
| var sqliteTransaction = (SQLiteTransaction)relCommand.ExternalTransaction; | ||
| var sqliteConn = (SQLiteConnection)sqliteTransaction.Connection; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo_root="$(pwd)"
file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs"
echo "== File exists? =="
ls -la "$file"
echo "== Lines 230-360 excerpt =="
nl -ba "$file" | sed -n '230,360p'
echo "== Find all provider-specific downcasts in the async handler =="
rg -n "SQLite(Transaction|Connection)|\b(SQLiteTransaction|SQLiteConnection)\b" "$file"
echo "== Check for IDbConnection/IDbTransaction patterns in async handler =="
rg -n "\bIDb(Transaction|Connection)\b" "$file"
echo "== Look for sync-path counterpart and inspect for similar casts =="
sync_file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.cs"
if [ -f "$sync_file" ]; then
echo "== Sync file exists: $sync_file =="
nl -ba "$sync_file" | sed -n '200,360p'
rg -n "SQLite(Transaction|Connection)|\b(SQLiteTransaction|SQLiteConnection)\b" "$sync_file"
rg -n "\bIDb(Transaction|Connection)\b" "$sync_file"
else
echo "== Sync counterpart not found at expected path: $sync_file =="
fi
echo "== Search repository for other SendMessageCommandHandler*.cs in SQLite transport =="
fd -t f "SendMessageCommandHandler*.cs" Source/DotNetWorkQueue.Transport.SQLite || trueRepository: blehnen/DotNetWorkQueue
Length of output: 321
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs"
sync_file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.cs"
echo "== Grep excerpt lines 230-360 (with line numbers via awk) =="
awk 'NR>=230 && NR<=360 {printf "%6d | %s\n", NR, $0}' "$file"
echo "== All provider-specific downcasts in async handler =="
rg -n "SQLiteTransaction|SQLiteConnection" "$file" || true
echo "== Any IDbTransaction/IDbConnection usage in async handler =="
rg -n "\bIDbTransaction\b|\bIDbConnection\b" "$file" || true
if [ -f "$sync_file" ]; then
echo "== Grep excerpt from sync counterpart lines 230-360 =="
awk 'NR>=230 && NR<=360 {printf "%6d | %s\n", NR, $0}' "$sync_file"
echo "== Downcasts in sync handler =="
rg -n "SQLiteTransaction|SQLiteConnection" "$sync_file" || true
echo "== IDbConnection/IDbTransaction usage in sync handler =="
rg -n "\bIDbTransaction\b|\bIDbConnection\b" "$sync_file" || true
else
echo "== Sync counterpart not found: $sync_file =="
fi
echo "== Find other SendMessageCommandHandler*.cs in SQLite transport =="
fd -t f "SendMessageCommandHandler*.cs" Source/DotNetWorkQueue.Transport.SQLite || trueRepository: blehnen/DotNetWorkQueue
Length of output: 18793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo_root="$(pwd)"
async_file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs"
sync_file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.cs"
echo "== Async handler top usings (first 80 lines) =="
sed -n '1,90p' "$async_file"
echo "== Sync handler top usings (first 90 lines) =="
sed -n '1,90p' "$sync_file"
echo "== Where does SQLiteTransaction / SQLiteConnection resolve in async file? (grep for type declarations/usings) =="
rg -n "\bSQLiteTransaction\b|\bSQLiteConnection\b" "$async_file" | head -n 50
echo "== Find definitions of SQLiteTransaction in SQLite transport =="
rg -n "class SQLiteTransaction|interface .*SQLiteTransaction|struct SQLiteTransaction" Source/DotNetWorkQueue.Transport.SQLite --glob='*.cs' || true
echo "== Find definitions of SQLiteConnection in SQLite transport =="
rg -n "class SQLiteConnection|interface .*SQLiteConnection|struct SQLiteConnection" Source/DotNetWorkQueue.Transport.SQLite --glob='*.cs' || true
echo "== Find GuardSQLiteTransaction implementation =="
rg -n "GuardSQLiteTransaction" Source/DotNetWorkQueue.Transport.SQLite --glob='*.cs' || trueRepository: blehnen/DotNetWorkQueue
Length of output: 12006
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Files of interest
rel_cmd_files=$(fd -t f "RelationalSendMessageCommand*.cs" Source/DotNetWorkQueue.Transport --hidden || true)
echo "== RelationalSendMessageCommand files =="
echo "$rel_cmd_files"
# Dump the definition (small file only)
for f in $rel_cmd_files; do
echo "== Inspect: $f (first 220 lines) =="
wc -l "$f"
sed -n '1,220p' "$f"
done
echo "== ISQLiteTransactionWrapper =="
sqlite_trans_file="Source/DotNetWorkQueue.Transport.SQLite/ISQLiteTransaction.cs"
if [ -f "$sqlite_trans_file" ]; then
wc -l "$sqlite_trans_file"
sed -n '1,220p' "$sqlite_trans_file"
else
echo "missing $sqlite_trans_file"
fi
echo "== SqliteRelationalProducerQueue excerpts around GuardSQLiteTransaction =="
producer_file="Source/DotNetWorkQueue.Transport.SQLite/Basic/SqliteRelationalProducerQueue.cs"
wc -l "$producer_file"
sed -n '150,240p' "$producer_file"
echo "== Search for ExternalTransaction assignment in SqliteRelationalProducerQueue =="
rg -n "ExternalTransaction|ISQLiteTransactionWrapper|GuardSQLiteTransaction" "$producer_file"Repository: blehnen/DotNetWorkQueue
Length of output: 5647
🏁 Script executed:
#!/bin/bash
set -euo pipefail
async_file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs"
sync_file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.cs"
echo "== Locate SendMessage.GetMainCommand signature =="
rg -n "GetMainCommand\\(" Source/DotNetWorkQueue --glob='*.cs'
echo "== Locate SendMessage.CreateMetaDataRecord signature =="
rg -n "CreateMetaDataRecord\\(" Source/DotNetWorkQueue --glob='*.cs'
echo "== Locate RelationalSendMessageCommand definition =="
rg -n "class RelationalSendMessageCommand|struct RelationalSendMessageCommand|record RelationalSendMessageCommand" Source/DotNetWorkQueue --glob='*.cs' || true
# If found, print the relevant section around ExternalTransaction
rel_cmd_file="$(rg -l "RelationalSendMessageCommand" Source/DotNetWorkQueue --glob='*.cs' | head -n 20 | tr '\n' ' ')"
echo "== Candidate files containing RelationalSendMessageCommand (top 20) =="
echo "$rel_cmd_file"
# Better: direct search for ExternalTransaction property usage
echo "== Search for ExternalTransaction property/type =="
rg -n "ExternalTransaction" Source/DotNetWorkQueue --glob='*.cs' | head -n 50Repository: blehnen/DotNetWorkQueue
Length of output: 117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find SendMessage helper type files =="
fd -t f "SendMessage*.cs" Source || true
fd -t f "*SendMessage*.cs" Source/DotNetWorkQueue.Transport.SQLite || true
fd -t f "*SendMessage*.cs" Source/DotNetWorkQueue.Transport.RelationalDatabase || true
fd -t f "SendMessage.cs" Source || true
echo "== Search for GetMainCommand/CreateMetaDataRecord without assuming filenames =="
rg -n "GetMainCommand|CreateMetaDataRecord" Source --glob='*.cs' || true
echo "== Find RelationalSendMessageCommand definition =="
rg -n "RelationalSendMessageCommand" Source --glob='*.cs' | head -n 50
rel_cmd_path="$(rg -l "RelationalSendMessageCommand" Source --glob='*.cs' | head -n 1 || true)"
echo "== Candidate RelationalSendMessageCommand file =="
echo "$rel_cmd_path"
if [ -n "${rel_cmd_path:-}" ] && [ -f "$rel_cmd_path" ]; then
sed -n '1,220p' "$rel_cmd_path"
rg -n "ExternalTransaction" "$rel_cmd_path" || true
fiRepository: blehnen/DotNetWorkQueue
Length of output: 27844
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rel_cmd="Source/DotNetWorkQueue.Transport.RelationalDatabase/Basic/Command/RelationalSendMessageCommand.cs"
if [ -f "$rel_cmd" ]; then
echo "== RelationalSendMessageCommand.cs snippet (first 140 lines) =="
sed -n '1,160p' "$rel_cmd"
echo "== ExternalTransaction property/type =="
rg -n "ExternalTransaction" "$rel_cmd" || true
echo "== ExternalTransaction declaration context =="
sed -n '1,220p' "$rel_cmd" | sed -n '1,220p'
else
echo "Missing: $rel_cmd"
# Fallback: search for the class file
f="$(rg -l "class RelationalSendMessageCommand" Source --glob='*.cs' | head -n 1 || true)"
echo "Found candidate: $f"
[ -n "$f" ] && sed -n '1,200p' "$f"
fiRepository: blehnen/DotNetWorkQueue
Length of output: 8274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
send_message_file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessage.cs"
wc -l "$send_message_file"
sed -n '1,120p' "$send_message_file"
sed -n '120,260p' "$send_message_file"
rg -n "GetMainCommand\\(|CreateMetaDataRecord" "$send_message_file"Repository: blehnen/DotNetWorkQueue
Length of output: 10854
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs"
echo "== Occurrences of provider-specific SQLite casts in async handler =="
rg -n "\(\s*SQLite(Transaction|Connection)\s*\)" "$file" || true
echo "== Occurrences of any SQLite* identifiers in async handler (casts/usage) =="
rg -n "SQLite(Transaction|Connection|Command|DataReader|Parameter)" "$file" || trueRepository: blehnen/DotNetWorkQueue
Length of output: 825
Remove provider-specific SQLite downcasts in HandleExternalTransactionAsync (use IDbTransaction/IDbConnection only).
RelationalSendMessageCommand.ExternalTransaction is a DbTransaction, but SendMessageCommandHandlerAsync.HandleExternalTransactionAsync downcasts it to SQLiteTransaction/SQLiteConnection (lines 255-256). Replace those with IDbTransaction/IDbConnection-typed locals to keep the handler provider-agnostic, including at the call sites where sqliteConn/sqliteTransaction are passed into DoesJobExistQuery<IDbConnection, IDbTransaction> and SetJobLastKnownEventCommand<IDbConnection, IDbTransaction> (lines 274-275, 329-330).
The sync handler in the same folder still contains the same sealed casts (lines 253-254 in SendMessageCommandHandler.cs), so apply the same abstraction fix there for parity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs`
around lines 255 - 256, Replace provider-specific SQLite casts in
HandleExternalTransactionAsync: stop downcasting
RelationalSendMessageCommand.ExternalTransaction to
SQLiteTransaction/SQLiteConnection and instead use IDbTransaction and
IDbConnection locals (e.g., var dbTx =
(IDbTransaction)relCommand.ExternalTransaction; var dbConn =
(IDbConnection)dbTx.Connection). Update call sites that pass
sqliteConn/sqliteTransaction into DoesJobExistQuery<IDbConnection,
IDbTransaction> and SetJobLastKnownEventCommand<IDbConnection, IDbTransaction>
to pass dbConn/dbTx. Make the same change in the sync handler
SendMessageCommandHandler.cs so both handlers are provider-agnostic.
…xtractor The outbox validator failed every FullUri=file:NAME?mode=memory&cache=shared integration test (Jenkins, issue #149 / PR #151) with: Caller-supplied transaction's connection points to database 'file:Iabc?mode=memory&cache=shared' but the queue is configured for database ''. Root cause is asymmetric connection-string parsing. SQLiteConnectionStringBuilder populates FullUri (not DataSource) for the FullUri= form, so the queue side (SqliteConnectionInformation.Container -> DbDataSource.DataSource -> builder.DataSource) returns empty. The connection side reads conn.DataSource directly, which the opened FullUri connection reports as the raw URI. The Ordinal compare always fails for in-memory shared databases. The fix routes both sides through SqLiteExternalDbNameExtractor.Canonicalize (new internal static), which parses connection strings via SQLiteConnectionStringBuilder and selects FullUri ?? DataSource before Path.GetFileNameWithoutExtension. Identical normalization on both sides guarantees symmetry across all supported connection-string forms: Data Source=, FullUri=file:..., and :memory:. DbDataSource and SqliteConnectionInformation.Container are unchanged so the fix has zero blast radius outside the outbox path. Unit tests rewritten to mock conn.ConnectionString (was conn.DataSource) and extended with FullUri symmetry cases; 171/171 SQLite unit tests pass.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #151 +/- ##
==========================================
- Coverage 89.78% 89.72% -0.07%
==========================================
Files 1002 1005 +3
Lines 29703 29872 +169
Branches 2405 2426 +21
==========================================
+ Hits 26670 26803 +133
- Misses 2368 2395 +27
- Partials 665 674 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodeRabbit review on PR #151 surfaced 6 actionable findings; all addressed. PRODUCTION (Major: sealed-cast coupling per CLAUDE.md lesson): - SendMessageCommandHandler.HandleExternalTransaction and the async fork no longer downcast relCommand.ExternalTransaction to System.Data.SQLite's sealed SQLiteTransaction/SQLiteConnection. Both handlers now operate on IDbTransaction/IDbConnection only — the producer's GuardSQLiteTransaction still enforces the concrete type at the API boundary. Removing the casts: * matches the PG/SS transport discipline (sealed-cast lesson, CLAUDE.md) * makes the outbox-fork path unit-testable with NSubstitute on the IDb* interfaces (the sealed types cannot be mocked at all) TESTS (Minor + Major fixes): - SqliteOutboxIntegrationTestBase: added EnsureActivityListenerRegistered helper + ClassInitialize hooks across all 5 outbox suites, so trace decorators are exercised (CLAUDE.md trace-decorator coverage lesson; mirrors PG/SS inbox base). - QueueScope/ProducerScope Dispose() now guard with Interlocked.Exchange for idempotent double-dispose safety. - SqliteOutboxValidationTests.Validation_GuardPass_HappyPath_SingleSend now asserts result.HasError == false in addition to row visibility. - SqliteOutboxRetryBypassTests.RetryBypass_TransientError_SingleAttempt uses Assert.ThrowsExactly<InvalidOperationException> instead of catch-all + null check — false-positive resistance. - Comment cleanup: "hold-tx" → "hold transaction" (matches no-Tx-abbreviation preference and CodeRabbit ask). LESSON CAPTURE: - CLAUDE.md: new SQLite FullUri/SQLiteConnectionStringBuilder symmetry lesson anchored to commit d111b0b — documents the asymmetric-builder bug shape for future SQLite work. 171/171 SQLite unit tests pass; both prod and IT projects build clean.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.cs (1)
21-21: 💤 Low valueUnused
using System.Data.SQLite;directive.The handler now operates exclusively on
IDbTransaction/IDbConnectionabstractions. TheSystem.Data.SQLitenamespace import is no longer referenced anywhere in this file and can be removed.♻️ Suggested fix
using System; using System.Data; -using System.Data.SQLite; using System.Diagnostics.CodeAnalysis;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.cs` at line 21, Remove the unused System.Data.SQLite using directive from SendMessageCommandHandler.cs; the handler now uses IDbTransaction/IDbConnection abstractions (e.g., the SendMessageCommandHandler class and its methods) so delete the top-level "using System.Data.SQLite;" import and verify there are no remaining references to types from that namespace in the SendMessageCommandHandler class.Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs (1)
21-21: 💤 Low valueUnused
using System.Data.SQLite;directive.Same as the sync handler — the async handler now operates on
IDbTransaction/IDbConnectionabstractions only. This import is no longer referenced.♻️ Suggested fix
using System; using System.Data; -using System.Data.SQLite; using System.Diagnostics.CodeAnalysis;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs` at line 21, Remove the unused System.Data.SQLite import in SendMessageCommandHandlerAsync.cs: delete the `using System.Data.SQLite;` directive at the top of the file (the async handler uses IDbTransaction/IDbConnection abstractions only, similar to the sync handler), ensuring the class SendMessageCommandHandlerAsync compiles without that redundant using.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Line 182: Update the Lessons Learned paragraph to consistently state that the
FullUri comparison bug was fixed (commit d111b0b5) and remove or reword the
sentence claiming the issue is still latent in CI; reference the implemented fix
by name and mechanism (SqLiteExternalDbNameExtractor.Canonicalize which uses
SQLiteConnectionStringBuilder and selects FullUri ?? DataSource, and the
validator now using Canonicalize(_connectionInfo.ConnectionString)) and drop the
contradictory statement about Container/DbDataSource.DataSource vs
builder.DataSource to reflect the current resolved state.
---
Nitpick comments:
In
`@Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.cs`:
- Line 21: Remove the unused System.Data.SQLite using directive from
SendMessageCommandHandler.cs; the handler now uses IDbTransaction/IDbConnection
abstractions (e.g., the SendMessageCommandHandler class and its methods) so
delete the top-level "using System.Data.SQLite;" import and verify there are no
remaining references to types from that namespace in the
SendMessageCommandHandler class.
In
`@Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs`:
- Line 21: Remove the unused System.Data.SQLite import in
SendMessageCommandHandlerAsync.cs: delete the `using System.Data.SQLite;`
directive at the top of the file (the async handler uses
IDbTransaction/IDbConnection abstractions only, similar to the sync handler),
ensuring the class SendMessageCommandHandlerAsync compiles without that
redundant using.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d850ab6-ec37-4694-98e5-98f9ca59aea7
📒 Files selected for processing (9)
CLAUDE.mdSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxAdditionalDataTests.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxIntegrationTestBase.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxRetryBypassTests.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxSendAsyncTests.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxSendTests.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/SqliteOutboxValidationTests.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandler.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessageCommandHandlerAsync.cs
| - **Plan code shapes can drift from the actual API surface.** Outbox PR #138 / REVIEW-1.2 caught a missing `using DotNetWorkQueue.Configuration;` in the `docs/outbox-pattern.md` tutorial code block — copy-paste would have failed CS0246 because `QueueConnection` lives in `DotNetWorkQueue.Configuration` not `DotNetWorkQueue` root. Pattern: reviewers for doc/code-example plans should TRACE the example against the current API surface (mentally compile it), not just verify it matches the plan's code shape. | ||
| - **Phase scope reframing — RESEARCH should validate the phase isn't already done.** ROADMAP Phase 7 framed as "add XML doc comments to phases 2-4 public types." RESEARCH §1 surfaced that the builders had already added docs as they went. Phase 7 reframed to a VERIFICATION pass + csproj gate fixes (net8 `<DocumentationFile>` gap + ISSUE-032 NU1902 closure on Transport.SQLite). Pattern: when a phase title implies authoring, researcher should explicitly confirm the work was not already incidentally done before architect plans new authoring work. | ||
| - **`<WarningsNotAsErrors>` pattern for accepted advisory carry-forward.** ISSUE-032 (OpenTelemetry NU1902 advisory) blocked the strictest `dotnet build -c Release -p:CI=true` build path on `Transport.SQLite`. Phase 7 PLAN-1.1 / commit `88ff8996` added `<WarningsNotAsErrors>NU1902</WarningsNotAsErrors>` to all 3 Release PropertyGroup blocks (`Release|net10.0`, `Release|net8.0`, `Release|AnyCPU`). The advisory still surfaces as a visible warning on every Release build (long-term remediation isn't forgotten) but the build is no longer blocked. Reusable pattern when a ship-blocking advisory is out of scope for the current milestone. | ||
| - **`SQLiteConnectionStringBuilder.DataSource` is empty when `FullUri=` is used — both sides of any compare MUST go through the builder symmetrically.** Outbox milestone PR #151 shipped with an asymmetric validator (`Container` side via builder, connection side via `conn.DataSource`) that passed local on-disk integration but failed every shared-in-memory FullUri test on Jenkins with `'' != 'file:Iabc?mode=memory&cache=shared'`. Pre-fix: extractor read `conn.DataSource` (raw URI on opened FullUri connection); queue side read `Container` → `DbDataSource.DataSource` → `builder.DataSource` (empty). Fix (commit `d111b0b5`): added `SqLiteExternalDbNameExtractor.Canonicalize(string)` that parses via `SQLiteConnectionStringBuilder` and selects `FullUri ?? DataSource` before `Path.GetFileNameWithoutExtension`; validator now uses `Canonicalize(_connectionInfo.ConnectionString)` for `expected`. Pattern: any new "compare two SQLite database identities" code must route both sides through the same builder + selection, never one side via the builder and the other via the open connection's `DataSource` property. Same root shape as the SqlServer `ToUpperInvariant` symmetric-normalization lesson from PR #138. |
There was a problem hiding this comment.
Resolve contradictory SQLite FullUri guidance in Lessons Learned.
Line 182 says the FullUri comparison bug was fixed, but Line 186 still describes it as latent and not yet seen in CI. Please reconcile these so the doc has one current state.
🧰 Tools
🪛 LanguageTool
[style] ~182-~182: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ontainer→DbDataSource.DataSource→builder.DataSource(empty). Fix (commitd111b0b`): adde...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~182-~182: Ensure spelling is correct
Context: ...ourceproperty. Same root shape as the SqlServerToUpperInvariant` symmetric-normalizat...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` at line 182, Update the Lessons Learned paragraph to consistently
state that the FullUri comparison bug was fixed (commit d111b0b5) and remove or
reword the sentence claiming the issue is still latent in CI; reference the
implemented fix by name and mechanism
(SqLiteExternalDbNameExtractor.Canonicalize which uses
SQLiteConnectionStringBuilder and selects FullUri ?? DataSource, and the
validator now using Canonicalize(_connectionInfo.ConnectionString)) and drop the
contradictory statement about Container/DbDataSource.DataSource vs
builder.DataSource to reflect the current resolved state.
This was referenced May 26, 2026
blehnen
added a commit
that referenced
this pull request
May 26, 2026
Revert SQLite outbox pattern support (#151)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds transactional outbox-pattern producer support to the SQLite transport (issue #149), mirroring the existing SqlServer and PostgreSQL outbox shape. Inbox / hold-transaction support remains a permanent non-goal — SQLite's
BEGIN EXCLUSIVE-on-write semantics make it structurally non-viable (PR #143 confirmed this across 14 Jenkins runs).4 phases, 23 commits, ADVISORY security audit (0 critical / 0 important).
What's included
SqliteRelationalProducerQueue<T>,SqLiteExternalDbNameExtractor(with symmetric.db3-stripping),SqLiteExternalTransactionValidator(subclass overridingValidatewithPath.GetFileNameWithoutExtensionon both sides beforeStringComparison.Ordinalcompare), outbox-branch added to sync + async send handlers, DI wiring inSqLiteMessageQueueInitIRetrySkippableguard added to SQLite retry decorators (66f7e864) — caught by Phase 3 integration test review. Without it, outboxSkipRetry=truewould have silently re-entered Polly retry on SQLite.Source/DotNetWorkQueue.Transport.SQLite.Tests/Basic/— extractor, validator subclass, producer queue, capability-cast positive-flip (replaces deleted negative-guard file). 25 unit tests, all pass locally (166 total in the SQLite Tests project pass).Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Outbox/(Base + Validation + Send + SendAsync + AdditionalData + RetryBypass). 28 test cases via[DataRow(true)] [DataRow(false)]parameterization (:memory:+ on-disk). Jenkins is the runtime source of truth (local WSLlibdl.soenv block on the dev host).docs/outbox-pattern.mdadds#### SQLite note,### SQLite Concurrency Caveat, DB-Name table row, and restructures Supported Transports to 3 tiers (Supported / Supported with caveats / Not supported). Provider-name accuracy fix (447250fc): note usesSQLiteConnection/SQLiteTransactionfromSystem.Data.SQLite— explicitly disambiguated fromMicrosoft.Data.Sqlite.Test plan
dotnet test Source/DotNetWorkQueue.Transport.SQLite.Tests/...— 166/166 PASS (4 m 27 s)dotnet test Source/DotNetWorkQueue.Transport.RelationalDatabase.Tests/...— 221/221 PASS (495 ms)Notable
producer is IRelationalProducerQueue<T>now succeeds for SQLite producers, matching PG/SS.git grep IRelationalWorkerNotification Source/DotNetWorkQueue.Transport.SQLite/returns zero hits. Permanent non-goal documented indocs/outbox-pattern.md§ SQLite Concurrency Caveat with link to issue Add outbox pattern support for SQLite (inbox not workable — single-writer lock) #149.PackageReferenceadditions in the diff.stage('SQLite')invocation.Deferred to follow-up (low priority)
.shipyard/NOTES.md(method rename, weak Windows-path assertion, duplicate WHY comments, duplicate connection-info boilerplate, deadconn.Databasestub)ConnectionString.cs:25literal"\\\\"path separator on Linux — latent risk for on-disk IT on Jenkins/Docker; not introduced by this milestoneCloses #149.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Behavior Changes / Bug Fixes
Documentation
Tests