feat(api): agent-inbox write paths — reply + status (HT-18) - #15
Merged
Conversation
Closes the dogfoodable loop: mail in → thread → store → GET → POST reply
(mints token, sends) → sent. Framework-agnostic, on top of HT-17.
- POST /api/v1/conversations/{id}/replies — Agent sends { text, html? };
server derives to/from/Re:-subject and the In-Reply-To/References chain
from the conversation, then sendReply mints + persists + sends. 201 with
the created ThreadView; reopens a closed conversation; 404 missing/deleted;
400 bad body.
- PATCH /api/v1/conversations/{id} — close/reopen via a new store
setConversationStatus (excludes deleted).
- Router + InboxApiDeps extended (sender/keyring/mailDomain/supportAddress).
Codex adversarial review (reply/token path, standing rule) → all fixed:
- HIGH double-send hole: a send that SUCCEEDED but failed to record 'sent'
was reported as 502 "not delivered", which would make a client resend.
sendReply now returns typed outcomes instead of an ambiguous throw — a
send-success-then-mark-failure resolves to ok:true (the message WAS
delivered), never a failure. Regression tests at unit + API level.
- send-failed is now a distinct result carrying persistedStatus
(failed|pending), so the 502 message claims only what's always true ("could
not be delivered"), never a false "saved as failed".
- reply mints the token from the CANONICAL conversation.id (the fetched row),
not the raw path segment, so an upper-cased path id can't embed a
non-canonical id in the outbound Message-ID.
227 tests pass; typecheck + Biome clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds reply creation and conversation status update endpoints, extends routing and dependency injection, adds outbound delivery result handling, persists status changes, and expands API, store, router, mail, and specification coverage. ChangesAgent Inbox write paths
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant InboxApi
participant ConversationStore
participant sendReply
participant EmailSender
Client->>InboxApi: POST conversation reply
InboxApi->>ConversationStore: Read conversation and threads
InboxApi->>sendReply: Send derived reply
sendReply->>EmailSender: Deliver outbound email
EmailSender-->>sendReply: Delivery result
sendReply-->>InboxApi: Return delivery outcome
InboxApi->>ConversationStore: Read created thread
InboxApi-->>Client: ThreadView or mapped error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
… a guaranteed 'failed' persist (HT-18) Codex confirm residual: the handleReply JSDoc still said the reply is durably persisted as delivery_status='failed' on a 502. That over-claims — sendReply returns a send-failed result (no throw) and the row may be 'failed' OR stuck 'pending'. Doc now matches the runtime + the user-safe message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/api/index.test.ts (1)
596-612: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCover the
pendingpersistence outcome too.
sendReplycan return502while leaving the outbound threadpendingif marking itfailedthrows. Add an end-to-end test for that branch, not only the successfulfailedtransition.🤖 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 `@src/api/index.test.ts` around lines 596 - 612, Extend the reply-sending tests around sendReply and the existing throwing EmailSender case to cover when persisting the failed delivery status also throws. Configure the store or persistence layer so the outbound thread remains pending, assert the API still returns 502 with the send_failed error shape, then reload the conversation and verify the outbound thread’s deliveryStatus is pending.
🤖 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 `@specs/api/agent-inbox-v1.md`:
- Around line 139-140: Update the 404 behavior description in the sendReply
specification to remove the claim that nothing is minted; state that no message
is sent and that any token minted before append resolution is discarded when the
conversation is missing or deleted, matching sendReply.
- Around line 123-124: Fix the Markdown code span in the subject description:
change the formatting so `Re:` excludes the trailing space, while explicitly
stating that the prefix is followed by a space and retaining the
case-insensitive, no-double-prefix behavior.
- Around line 142-146: Clarify the `send_failed` contract in the agent-inbox
response documentation: state that the persisted reply may have `delivery_status
= 'failed'`, or remain `pending` if updating it to failed also errors. If the
error-envelope examples are exhaustive, include a `send_failed` example with the
502 response.
---
Nitpick comments:
In `@src/api/index.test.ts`:
- Around line 596-612: Extend the reply-sending tests around sendReply and the
existing throwing EmailSender case to cover when persisting the failed delivery
status also throws. Configure the store or persistence layer so the outbound
thread remains pending, assert the API still returns 502 with the send_failed
error shape, then reload the conversation and verify the outbound thread’s
deliveryStatus is pending.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b98421d9-325f-4c1a-966a-a780a0407a7a
📒 Files selected for processing (10)
specs/api/agent-inbox-v1.mdsrc/api/conversations.tssrc/api/index.test.tssrc/api/index.tssrc/api/router.test.tssrc/api/router.tssrc/mail/send.test.tssrc/mail/send.tssrc/store/conversations.test.tssrc/store/conversations.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
specs/api/agent-inbox-v1.md (1)
123-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrailing space inside code span (MD038).
`Re: `has a trailing space inside the backticks, flagged by markdownlint (MD038).📝 Proposed fix
-- **`subject`** = the conversation's `subject`, prefixed `Re: ` if it isn't already - (case-insensitive check — never double-prefix `Re: Re:`). +- **`subject`** = the conversation's `subject`, prefixed with `"Re: "` if it isn't already + (case-insensitive check — never double-prefix `Re: Re:`).🤖 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 `@specs/api/agent-inbox-v1.md` around lines 123 - 124, Remove the trailing space from the inline code span in the `subject` description and rephrase the surrounding text as needed to preserve the intended `Re:` prefix behavior without placing whitespace inside backticks.Source: Linters/SAST tools
🤖 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 `@specs/api/agent-inbox-v1.md`:
- Around line 142-147: Update the `502 send_failed` section to avoid
guaranteeing `delivery_status = 'failed'` after an `EmailSender` failure. State
that the reply may be persisted with status `'failed'` or remain `'pending'` if
the status update fails, and describe only that delivery failed without
promising it is saved for retry.
---
Nitpick comments:
In `@specs/api/agent-inbox-v1.md`:
- Around line 123-124: Remove the trailing space from the inline code span in
the `subject` description and rephrase the surrounding text as needed to
preserve the intended `Re:` prefix behavior without placing whitespace inside
backticks.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2097aefa-9720-4168-a5a5-0dd4ab33eeb8
📒 Files selected for processing (10)
specs/api/agent-inbox-v1.mdsrc/api/conversations.tssrc/api/index.test.tssrc/api/index.tssrc/api/router.test.tssrc/api/router.tssrc/mail/send.test.tssrc/mail/send.tssrc/store/conversations.test.tssrc/store/conversations.ts
CodeRabbit on #15 (3 Minor): the spec still described the OLD reply-failure behavior after the code was fixed to typed results. - 502 send_failed: provider-reject → thread 'failed' OR stuck 'pending'; message says only 'could not be delivered'; note the sent-but-mark-fails → 201 asymmetry. (was: over-claimed 'saved as failed, will be retried') - 404: drop the 'nothing is minted' self-contradiction (a token IS minted pre-append, then discarded). - error-envelope code enum made exhaustive (add method_not_allowed, send_failed); fix the 'Re: ' code-span MD038 whitespace. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
This was referenced Jul 11, 2026
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.
Closes HT-18 — and closes the dogfoodable loop: mail in → thread → store →
GET(read) →POSTreply (mints the token, actually sends) → sent → the customer's reply threads back. Framework-agnostic, on top of HT-17's read layer.What's here
POST /api/v1/conversations/{id}/replies— the Agent sends{ text, html? }; the server derivesto/from/Re:-subject and theIn-Reply-To/Referenceschain from the conversation, thensendReplymints the reply token, persists the outbound thread, and sends via the injectedEmailSender.201with the createdThreadView; reopens aclosedconversation;404missing/deleted;400bad body;502if the provider rejects.PATCH /api/v1/conversations/{id}— close/reopen via a newsetConversationStatus(excludes deleted).InboxApiDepsextended (sender/keyring/mailDomain/supportAddress).Codex adversarial review (reply/token path — standing rule)
Returned DON'T-SHIP with 3 findings; all fixed with tests:
sentwas reported as502 "not delivered", which would make a client resend.sendReplynow returns typed outcomes instead of an ambiguous throw — a send-success-then-mark-failure resolves took:true(the message was delivered), never a failure. Regression tests at unit and API level.send-failedis now a distinct result carryingpersistedStatus(failed|pending), so the502message claims only what's always true ("could not be delivered"), never a false "saved as failed" when the row is actually stuckpending.conversation.id(the fetched row), not the raw path segment — an upper-cased path id can't smuggle a non-canonical id into the outboundMessage-ID.Codex confirmed the header derivation, validation, routing, auth-before-routing, and
no-storeguarantees are correct.Testing
Real PGlite + fake/throwing
EmailSender: reply happy-path (derived headers + verbatim Message-ID), no-double-Re:, reopen-on-reply, missing/deleted/non-UUID → 404, bad body → 400, provider-reject → 502 (+persistedfailed), sent-but-mark-fails → 201 (the double-send regression), PATCH open/closed/404/400, method-routing 405s, auth,no-store. 227 tests pass; typecheck + Biome clean.A Codex confirm pass on the redesigned
sendReplycontract is running; I'll note the verdict.🤖 Generated with Claude Code
Summary by CodeRabbit
failed/pendingstate without throwing.400/404responses.405and502cases).