fix(outbound): keep channel send durable when transcript mirror fails (#89626)#89812
Conversation
The post-delivery transcript mirror in deliverOutboundPayloads runs after the
channel payloads are already sent. Its append was unguarded, so a transient
session-lock failure ("session file changed while embedded prompt lock was
released") propagated out and the whole send was reported as failed. Callers
then retried and re-delivered the already-sent message.
For sub-agent completion announcements this produced up to 3 duplicate messages
before the announce gave up (issue #89626).
Treat the mirror as best-effort (matching the cron delivery path,
mirrorDirectDeliveryIntoTranscript): catch throws and not-ok results, log a
warning, and let the already-successful channel delivery stand. The append
still carries its idempotency key for later reconciliation.
Fixes #89626
Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
Codex review: passed. Reviewed June 3, 2026, 9:20 AM ET / 13:20 UTC. Summary PR surface: Source +16, Tests +61. Total +77 across 2 files. Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTranscript throw or return ok:false after a channel adapter send; current main has no guard at src/infra/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the bounded outbound mirror guard after required checks so platform sends stay durable while transcript mirror misses remain warning-only diagnostics. Do we have a high-confidence way to reproduce the issue? Yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTranscript throw or return ok:false after a channel adapter send; current main has no guard at src/infra/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Is this the best way to solve the issue? Yes. Guarding the transcript mirror in the outbound substrate is the narrowest maintainable fix because the channel send has already succeeded, and the cron direct-delivery sibling already treats the same mirror append as best-effort. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 446a2b24c359. Label changesLabel justifications:
Evidence reviewedPR surface: Source +16, Tests +61. Total +77 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
🦞✅ Source: What merged:
Automerge notes:
The automerge loop is complete. Automerge progress:
|
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
…e failure The "session file changed while embedded prompt lock was released" error is a concurrent-session race: the parent transcript is being modified by turn-maintenance while the subagent completion tries to inject. Retrying does not help because each retry independently triggers a duplicate outbound send (3x duplicate Telegram messages reported in openclaw#91527). Add the error message pattern to PERMANENT_ANNOUNCE_DELIVERY_ERROR_PATTERNS so it stops triggering retries, mirroring the approach used in PR openclaw#89812 for the general outbound delivery path. Fixes openclaw#91527 Co-Authored-By: Claude Fable 5 <[email protected]>
… failure with send evidence When a required-mode batch send fails mid-batch after an earlier payload already succeeded, the wrapper catch in deliverOutboundPayloadsWithQueueCleanup called failDelivery. failDelivery only bumps retryCount/lastError; it does not advance recoveryState, so the entry stayed in send_attempt_started (set earlier by markDeliveryPlatformSendAttemptStarted via onPlatformSendStart). On the next Telegram reconnect, drainQueuedEntry sees send_attempt_started and calls reconcileUnknownQueuedDelivery. When adapter reconciliation misreports not_sent (the message was actually sent, per the outbound send ok / messageId evidence), the entry is replayed and the user receives a duplicate. Fix: when the error carries send evidence (OutboundDeliveryError with sentBeforeError === true and platformSendStarted === true), call markQueuedPlatformOutcomeUnknown instead of failDelivery. This advances the entry to unknown_after_send, which drain already routes through reconcileUnknownQueuedDelivery, preserving the entry for adapter reconciliation rather than leaving it in send_attempt_started for replay. When there is no send evidence (sentBeforeError === false), failDelivery remains correct: nothing reached the channel, so retrying is safe. This is a third duplicate path distinct from openclaw#89812 (mirror best-effort) and openclaw#92274 (subagent-announce-delivery retry); it is the outbound/deliver wrapper catch, which neither prior fix covers. Tests: - regression: two payloads, first succeeds, second throws; asserts markDeliveryPlatformOutcomeUnknown called, failDelivery/ackDelivery not. - guard: no send evidence; failDelivery still called.
… failure with send evidence When a required-mode batch send fails mid-batch after an earlier payload already succeeded, the wrapper catch in deliverOutboundPayloadsWithQueueCleanup called failDelivery. failDelivery only bumps retryCount/lastError; it does not advance recoveryState, so the entry stayed in send_attempt_started (set earlier by markDeliveryPlatformSendAttemptStarted via onPlatformSendStart). On the next Telegram reconnect, drainQueuedEntry sees send_attempt_started and calls reconcileUnknownQueuedDelivery. When adapter reconciliation misreports not_sent (the message was actually sent, per the outbound send ok / messageId evidence), the entry is replayed and the user receives a duplicate. Fix: when the error carries send evidence (OutboundDeliveryError with sentBeforeError === true and platformSendStarted === true), call markQueuedPlatformOutcomeUnknown instead of failDelivery. This advances the entry to unknown_after_send, which drain already routes through reconcileUnknownQueuedDelivery, preserving the entry for adapter reconciliation rather than leaving it in send_attempt_started for replay. When there is no send evidence (sentBeforeError === false), failDelivery remains correct: nothing reached the channel, so retrying is safe. This is a third duplicate path distinct from #89812 (mirror best-effort) and #92274 (subagent-announce-delivery retry); it is the outbound/deliver wrapper catch, which neither prior fix covers. Tests: - regression: two payloads, first succeeds, second throws; asserts markDeliveryPlatformOutcomeUnknown called, failDelivery/ackDelivery not. - guard: no send evidence; failDelivery still called.
… failure with send evidence When a required-mode batch send fails mid-batch after an earlier payload already succeeded, the wrapper catch in deliverOutboundPayloadsWithQueueCleanup called failDelivery. failDelivery only bumps retryCount/lastError; it does not advance recoveryState, so the entry stayed in send_attempt_started (set earlier by markDeliveryPlatformSendAttemptStarted via onPlatformSendStart). On the next Telegram reconnect, drainQueuedEntry sees send_attempt_started and calls reconcileUnknownQueuedDelivery. When adapter reconciliation misreports not_sent (the message was actually sent, per the outbound send ok / messageId evidence), the entry is replayed and the user receives a duplicate. Fix: when the error carries send evidence (OutboundDeliveryError with sentBeforeError === true and platformSendStarted === true), call markQueuedPlatformOutcomeUnknown instead of failDelivery. This advances the entry to unknown_after_send, which drain already routes through reconcileUnknownQueuedDelivery, preserving the entry for adapter reconciliation rather than leaving it in send_attempt_started for replay. When there is no send evidence (sentBeforeError === false), failDelivery remains correct: nothing reached the channel, so retrying is safe. This is a third duplicate path distinct from openclaw#89812 (mirror best-effort) and openclaw#92274 (subagent-announce-delivery retry); it is the outbound/deliver wrapper catch, which neither prior fix covers. Tests: - regression: two payloads, first succeeds, second throws; asserts markDeliveryPlatformOutcomeUnknown called, failDelivery/ackDelivery not. - guard: no send evidence; failDelivery still called.
… failure with send evidence When a required-mode batch send fails mid-batch after an earlier payload already succeeded, the wrapper catch in deliverOutboundPayloadsWithQueueCleanup called failDelivery. failDelivery only bumps retryCount/lastError; it does not advance recoveryState, so the entry stayed in send_attempt_started (set earlier by markDeliveryPlatformSendAttemptStarted via onPlatformSendStart). On the next Telegram reconnect, drainQueuedEntry sees send_attempt_started and calls reconcileUnknownQueuedDelivery. When adapter reconciliation misreports not_sent (the message was actually sent, per the outbound send ok / messageId evidence), the entry is replayed and the user receives a duplicate. Fix: when the error carries send evidence (OutboundDeliveryError with sentBeforeError === true and platformSendStarted === true), call markQueuedPlatformOutcomeUnknown instead of failDelivery. This advances the entry to unknown_after_send, which drain already routes through reconcileUnknownQueuedDelivery, preserving the entry for adapter reconciliation rather than leaving it in send_attempt_started for replay. When there is no send evidence (sentBeforeError === false), failDelivery remains correct: nothing reached the channel, so retrying is safe. This is a third duplicate path distinct from openclaw#89812 (mirror best-effort) and openclaw#92274 (subagent-announce-delivery retry); it is the outbound/deliver wrapper catch, which neither prior fix covers. Tests: - regression: two payloads, first succeeds, second throws; asserts markDeliveryPlatformOutcomeUnknown called, failDelivery/ackDelivery not. - guard: no send evidence; failDelivery still called.
…e failure The "session file changed while embedded prompt lock was released" error is a concurrent-session race: the parent transcript is being modified by turn-maintenance while the subagent completion tries to inject. Retrying does not help because each retry independently triggers a duplicate outbound send (3x duplicate Telegram messages reported in openclaw#91527). Add the error message pattern to PERMANENT_ANNOUNCE_DELIVERY_ERROR_PATTERNS so it stops triggering retries, mirroring the approach used in PR openclaw#89812 for the general outbound delivery path. Fixes openclaw#91527 Co-Authored-By: Claude Fable 5 <[email protected]>
…openclaw#89626) (openclaw#89812) Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: openclaw#89812 (comment) Co-authored-by: harjoth <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>
… failure with send evidence When a required-mode batch send fails mid-batch after an earlier payload already succeeded, the wrapper catch in deliverOutboundPayloadsWithQueueCleanup called failDelivery. failDelivery only bumps retryCount/lastError; it does not advance recoveryState, so the entry stayed in send_attempt_started (set earlier by markDeliveryPlatformSendAttemptStarted via onPlatformSendStart). On the next Telegram reconnect, drainQueuedEntry sees send_attempt_started and calls reconcileUnknownQueuedDelivery. When adapter reconciliation misreports not_sent (the message was actually sent, per the outbound send ok / messageId evidence), the entry is replayed and the user receives a duplicate. Fix: when the error carries send evidence (OutboundDeliveryError with sentBeforeError === true and platformSendStarted === true), call markQueuedPlatformOutcomeUnknown instead of failDelivery. This advances the entry to unknown_after_send, which drain already routes through reconcileUnknownQueuedDelivery, preserving the entry for adapter reconciliation rather than leaving it in send_attempt_started for replay. When there is no send evidence (sentBeforeError === false), failDelivery remains correct: nothing reached the channel, so retrying is safe. This is a third duplicate path distinct from openclaw#89812 (mirror best-effort) and openclaw#92274 (subagent-announce-delivery retry); it is the outbound/deliver wrapper catch, which neither prior fix covers. Tests: - regression: two payloads, first succeeds, second throws; asserts markDeliveryPlatformOutcomeUnknown called, failDelivery/ackDelivery not. - guard: no send evidence; failDelivery still called. (cherry picked from commit 71422a9)
Makes #89640 merge-ready for the ClawSweeper automerge loop.
The edit pass should inspect the live PR diff, review comments, and failing checks; rebase if needed; keep the contributor branch credited; and stop only when validation is green or an external blocker is proven.
Known failing checks:
ClawSweeper 🐠 replacement reef notes:
Inherited issue-closing references from the source PR:
Fixes #89626
Co-author credit kept:
fish notes: model gpt-5.5, reasoning high; reviewed against dfe0fd7.