feat(outbox): SQLite outbox with write-ahead delivery, recovery worker, and sendFinal-required adapters#29953
feat(outbox): SQLite outbox with write-ahead delivery, recovery worker, and sendFinal-required adapters#29953nohat wants to merge 11 commits intoopenclaw:mainfrom
Conversation
Replace unbounded file-based delivery queue with queryable SQLite message_outbox table. Adds TTL/expiry for stale entries, delivery outcome retention, and one-time legacy file queue import on startup. Closes openclaw#23777, openclaw#16555, openclaw#29128
…pat layer Write-ahead delivery pattern: enqueue outbox entry before sending, ack on success, retry on failure. Continuous outbox worker replaces one-shot recovery. Plugin channels get durable delivery guarantees via v1/v2 adapter compat layer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…low v2 sendFinal-only plugin adapters (Codex P1+P2 openclaw#29148)
…l and separate ackDelivery errors in recovery
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
19 similar comments
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
|
Closing this PR because it looks dirty (too many unrelated or unexpected changes). This usually happens when a branch picks up unrelated commits or a merge went sideways. Please recreate the PR from a clean branch. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f4421ef55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (err) { | ||
| const code = | ||
| err && typeof err === "object" && "code" in err | ||
| ? String((err as { code?: unknown }).code) | ||
| : null; | ||
| if (code !== "ENOENT") { | ||
| throw err; | ||
| } | ||
| // Already removed — no-op. | ||
| logVerbose(`delivery-queue: ackDelivery failed: ${String(err)}`); | ||
| } |
There was a problem hiding this comment.
Surface ackDelivery write failures to recovery loop
When SQLite writes fail during ackDelivery (for example, a transient SQLITE_BUSY from another process), this catch block logs and returns success, so the recovery pass treats the send as recovered while the row stays queued. On the next outbox pass, the same pre-start entry is still eligible and gets sent again, causing duplicate outbound messages until an ack eventually succeeds. The recovery path already tries to handle ack failures separately, but this swallow prevents that path from running.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR implements a SQLite-based outbox pattern with write-ahead delivery, recovery worker, and makes Key Changes
Critical Issue Found
Performance Concerns
Positive Notes
Confidence Score: 2/5
Last reviewed commit: 7f4421e |
| for (const entry of pending) { | ||
| if (entry.dispatchKind && entry.dispatchKind !== "final") { | ||
| try { | ||
| db2 | ||
| .prepare( | ||
| `UPDATE message_outbox | ||
| SET status='failed_terminal', | ||
| error_class='terminal', | ||
| terminal_reason='non_final_recovery_skip', | ||
| completed_at=? | ||
| WHERE id=?`, | ||
| ) | ||
| .run(Date.now(), entry.id); | ||
| } catch { | ||
| // non-fatal | ||
| } | ||
| continue; | ||
| } | ||
| finalOnly.push(entry); | ||
| } |
There was a problem hiding this comment.
dispatchKind is stored in JSON payload, not as a column. recovery loads ALL pending entries, then filters in memory. storing dispatchKind as a column with an index would allow filtering in SQL: WHERE (dispatch_kind IS NULL OR dispatch_kind = 'final')
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/infra/outbound/delivery-queue.ts
Line: 477-496
Comment:
`dispatchKind` is stored in JSON payload, not as a column. recovery loads ALL pending entries, then filters in memory. storing `dispatchKind` as a column with an index would allow filtering in SQL: `WHERE (dispatch_kind IS NULL OR dispatch_kind = 'final')`
How can I resolve this? If you propose a fix, please make it concise.| CREATE TABLE IF NOT EXISTS message_outbox ( | ||
| id TEXT NOT NULL PRIMARY KEY, | ||
| turn_id TEXT, | ||
| channel TEXT NOT NULL, | ||
| account_id TEXT NOT NULL DEFAULT '', | ||
| target TEXT NOT NULL DEFAULT '', | ||
| payload TEXT NOT NULL, | ||
| queued_at INTEGER NOT NULL, | ||
| status TEXT NOT NULL DEFAULT 'queued', | ||
| attempt_count INTEGER NOT NULL DEFAULT 0, | ||
| next_attempt_at INTEGER NOT NULL, | ||
| last_attempt_at INTEGER, | ||
| last_error TEXT, | ||
| error_class TEXT, | ||
| delivered_at INTEGER, | ||
| terminal_reason TEXT, | ||
| completed_at INTEGER, | ||
| idempotency_key TEXT | ||
| ); |
There was a problem hiding this comment.
schema missing dispatch_kind column but the field is used in delivery-queue.ts (stored in JSON payload). consider adding: dispatch_kind TEXT for efficient filtering during recovery
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/infra/message-lifecycle/db.ts
Line: 48-66
Comment:
schema missing `dispatch_kind` column but the field is used in delivery-queue.ts (stored in JSON payload). consider adding: `dispatch_kind TEXT` for efficient filtering during recovery
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/auto-reply/reply/reply-dispatcher.ts
Line: 175-215
Comment:
dispatcher enqueues here, then calls `options.deliver` which eventually calls `deliverOutboundPayloads` (deliver.ts:274-288), which enqueues again unless `skipQueue: true`. this creates duplicate outbox entries for every message sent through the dispatcher
How can I resolve this? If you propose a fix, please make it concise. |
Summary
Combines the scope of old PRs #29147 and #29148 into a single, cleaned-up PR.
Key changes from the old PRs:
outboundContractv1/v2 concept removed entirely —sendFinalis now required on allChannelOutboundAdapterssrc/plugin-sdk/outbound-adapter.tsdeletedsendFinalCarries forward from old PRs:
Closes #22376, #9208, #14827, #29126, #23777, #16555, #29128
Related: #28941
Test plan
pnpm buildpassespnpm testpasses (outbox, delivery, recovery, plugin compat tests)sendFinalrequired🤖 Generated with Claude Code