feat(messages): make append delivery traceable - #1312
Conversation
Why: expose durable append evidence and make failed target delivery safely retryable without resending user messages.
Why: repeated body exposure and rejected stale retries must not corrupt delivery history or leave dead retry controls.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c639fc123
ℹ️ 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".
| if (!current.pendingTargetCats.includes(targetCatId as CatId)) return { next: current }; | ||
| const attempts = ensureTargetAttempts(current); | ||
| const previous = latestTargetAttempt(attempts, targetCatId); | ||
| if (!previous || previous.id !== expectedAttemptId || previous.state !== 'failed') return { next: current }; |
There was a problem hiding this comment.
When an invocation is canceled, QueueProcessor leaves the target in failedByCatIds but records its latest attempt as cancelled with terminalReason: 'invocation_cancelled'. The receipt therefore presents a failed target, yet this fence accepts only previous.state === 'failed' (and the UI similarly hides Retry for cancelled), so a stopped reply can never be retried even though its queue carrier remains pending. Accept invocation-cancelled attempts here while continuing to exclude source_withdrawn cancellations.
Useful? React with 👍 / 👎.
Why: queue/append intent and its durable receipt projection form one user journey; Stop remains independently reviewable.
Why: a stale client preference must not override persisted delivery intent, and a stopped invocation should remain recoverable without reopening author withdrawals.
|
@codex review the latest head; prior inline findings were addressed in subsequent commits. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bab571401d
ℹ️ 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".
| this.deps.log.warn({ err, threadId, entryId, catId }, '[QueueProcessor] retry auto-dispatch failed'); | ||
| }); | ||
| } else { | ||
| void this.processNext(threadId, userId).catch((err) => { |
There was a problem hiding this comment.
When a user-authored queue entry has multiple failed targets, InvocationQueue.retryFailedTarget clears failure state only for the clicked catId, but this call to processNext executes the entry with its entire unchanged targetCats array. The unselected failed targets are therefore run again without a corresponding retry attempt being appended for them, which can trigger unwanted work and leave their attempt history inconsistent with the execution. Dispatch a target-scoped snapshot or otherwise prevent sibling targets from running on a single-target retry.
Useful? React with 👍 / 👎.
Why: retrying one failed recipient must not re-run sibling targets or rewrite their attempt history; receipt assertions now preserve the durable attempt contract.
|
@codex review the latest head; this addresses single-target retry dispatch and preserves the receipt attempt contract. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 526c9f0e1d
ℹ️ 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".
| if (!this.deps.queue.markProcessingById(threadId, entryId)) return false; | ||
|
|
||
| const processing = this.deps.queue.getEntrySnapshot(threadId, userId, entryId); | ||
| return this.startReservedEntry(processing ?? current, slotKey, catId, [catId], true); |
There was a problem hiding this comment.
When the retried carrier is user-authored and another queued user entry has the same intent, owner, and full target set, this target-scoped call still enters executeEntry's user-batching block around lines 3485-3503. That block appends the other entry's content and marks it processing, but the invocation is routed only to [catId]; the unrelated entry is therefore executed without a retry attempt being appended for it, corrupting its receipt-attempt history and potentially settling work the user did not retry. Scoped retry executions should bypass normal user batching.
Useful? React with 👍 / 👎.
| const slotKey = QueueProcessor.slotKey(threadId, catId); | ||
| if (this.processingSlots.has(slotKey) || this.deps.invocationTracker.has(threadId, catId)) return false; | ||
| if (!this.deps.queue.markProcessingById(threadId, entryId)) return false; |
There was a problem hiding this comment.
If another invocation or queue execution owns this target slot when Retry is clicked, the durable retry attempt has already been appended and the failed marker cleared, but this method simply returns false and its fire-and-forget caller ignores the result. The entry remains in the ordinary queue with no target-scoped dispatch owner, so a later generic dequeue or manual continue executes its full targetCats array and can rerun failed sibling targets without corresponding retry attempts. Keep the scoped retry pending until this slot is available, or avoid committing the attempt until its scoped reservation is secured.
Useful? React with 👍 / 👎.
1 participant
What
Closes #1308.
Verification
Known non-blocking