HT-6: conversations API v1 spec - #2
Conversation
Wire-compatible with the REST subset the existing Resonant IQ consumer speaks (cutover = base URL + auth header), Helpthread-native Bearer auth, full TypeScript shapes, server-enforced limits, and six flagged OPEN QUESTIONs (thread-type semantics, internal-note filtering, attachment reject-vs-truncate, attachment size/mime limits, sort generalization, create idempotency). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughChangesThe pull request adds a Conversations API v1 specification covering six customer-facing operations, shared JSON schemas, authentication and error conventions, validation semantics, authorization expectations, open questions, and explicit non-goals. Conversations API v1
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/conversations-v1.md`:
- Around line 65-82: Clarify the list-conversations API contract for invalid
pageSize, sortField, sortOrder, and embed query values: specify which values are
accepted and whether invalid inputs return 400 validation_failed, are clamped,
or are ignored. Update the “List conversations by customer email” parameter
notes and pagination/sorting behavior so clients have deterministic validation
and response semantics.
- Around line 211-221: Define customer-facing conversation reads, including GET
/conversations/{id}?embed=threads, as excluding all internal-note and other
agent-only threads; remove the open question and document this as a mandatory
contract invariant. Add an acceptance fixture asserting that note/agent-only
content is absent from the embedded threads response.
- Around line 122-126: Update the v1 attachment contract and its
request-validation rules to define per-file and aggregate decoded-byte limits,
reject malformed base64, and validate MIME types and filenames; document that
any violation returns HTTP 400 with error code validation_failed, covering both
the Attachment interface and the related request fields.
- Around line 193-201: The Thread schema conflates thread kind with author
identity, so agent- and assistant-authored messages cannot be distinguished.
Update the Thread interface to add an explicit actorType field with customer,
agent, and assistant values, while retaining ThreadType for
customer/message/note classification; document the field and ensure the
corresponding schema examples or definitions around the additional referenced
section are updated consistently.
- Around line 241-246: Define the create idempotency contract in the v1
conversation API specification before implementation, including Idempotency-Key
scope, retention period, behavior when the same key is reused with a different
payload, and whether successful or failed responses are replayed. Update the
create endpoint requirements and referenced consumer behavior accordingly.
🪄 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: e8464fb0-a765-41b2-90c3-c3f09714fcfb
📒 Files selected for processing (1)
specs/api/conversations-v1.md
| - **Pagination:** v1 ships **first-page-only**. `pageSize` is accepted but | ||
| capped at 50 (the consumer's only observed value) with no cursor/offset for | ||
| a second page — see §5. Sort is newest-first by default and, in v1, the only | ||
| supported combination. | ||
|
|
||
| ## 3. The six operations | ||
|
|
||
| ### a. List conversations by customer email | ||
|
|
||
| `GET /api/v1/conversations?customerEmail={email}&pageSize=50&sortField=createdAt&sortOrder=desc&embed=threads` | ||
|
|
||
| | param | type | required | notes | | ||
| |---|---|---|---| | ||
| | `customerEmail` | string | yes | matched case-insensitively (client always lowercases before sending) | | ||
| | `pageSize` | number | no | default 50; v1 hard cap 50 | | ||
| | `sortField` | string | no | default `createdAt`; **v1 only supports this value** — see OPEN QUESTION below | | ||
| | `sortOrder` | string | no | default `desc`; **v1 only supports this value** | | ||
| | `embed` | string | no | `threads` embeds each conversation's threads inline, for list-view previews without an N+1 fetch | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define behavior for unsupported query values.
Specify whether invalid pageSize, sortField, sortOrder, and embed values return 400 validation_failed, are clamped, or are ignored. Without this, clients cannot rely on deterministic pagination or sorting behavior.
🤖 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/conversations-v1.md` around lines 65 - 82, Clarify the
list-conversations API contract for invalid pageSize, sortField, sortOrder, and
embed query values: specify which values are accepted and whether invalid inputs
return 400 validation_failed, are clamped, or are ignored. Update the “List
conversations by customer email” parameter notes and pagination/sorting behavior
so clients have deterministic validation and response semantics.
| interface Attachment { | ||
| fileName: string; | ||
| mimeType: string; | ||
| data: string; // base64 | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Define attachment size and content validation before freezing v1.
A limit of ten attachments does not bound the decoded bytes or total request size. Define per-file and aggregate byte limits, reject malformed base64, and specify MIME and filename validation with 400 validation_failed; otherwise this endpoint can be used for oversized payloads and unsafe content.
Also applies to: 230-240
🤖 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/conversations-v1.md` around lines 122 - 126, Update the v1
attachment contract and its request-validation rules to define per-file and
aggregate decoded-byte limits, reject malformed base64, and validate MIME types
and filenames; document that any violation returns HTTP 400 with error code
validation_failed, covering both the Attachment interface and the related
request fields.
| interface Thread { | ||
| id: number; | ||
| type: ThreadType; | ||
| text: string; | ||
| createdAt: string; | ||
| createdBy?: { id: number; firstName?: string; lastName?: string }; | ||
| } | ||
|
|
||
| type ThreadType = 'customer' | 'message' | 'note'; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Separate thread kind from actor identity.
The charter requires every thread to record whether it was authored by a customer, agent, or assistant, but this schema only distinguishes customer | message | note. An agent-authored message and an assistant-authored message become indistinguishable. Add an explicit actor field, such as actorType: 'customer' | 'agent' | 'assistant', or revise Thread.type and document the mapping.
Also applies to: 211-217
🤖 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/conversations-v1.md` around lines 193 - 201, The Thread schema
conflates thread kind with author identity, so agent- and assistant-authored
messages cannot be distinguished. Update the Thread interface to add an explicit
actorType field with customer, agent, and assistant values, while retaining
ThreadType for customer/message/note classification; document the field and
ensure the corresponding schema examples or definitions around the additional
referenced section are updated consistently.
| `type` values on `Thread`: `customer` (customer-authored, the only type the | ||
| consumer ever constructs, via create and reply) and `message`/`note` are | ||
| included as the actor-facing types implied by the charter's actor vocabulary | ||
| (agent-authored reply and internal note, respectively) — **OPEN QUESTION**: | ||
| the consumer code read for this spec never parses or renders a thread by | ||
| `type`, so the customer/message/note split above is inferred from field | ||
| naming and the charter's actor model, not observed. Also **OPEN QUESTION**: | ||
| whether `note` threads must ever be filtered out of a customer-facing read | ||
| response — an internal note leaking to `GET /conversations/{id}` would be a | ||
| real information leak on the agent's behalf, and nothing in the consumer code | ||
| proves FreeScout (or should Helpthread) filters this server-side. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make internal-note filtering a hard contract invariant.
GET /conversations/{id}?embed=threads is customer-facing, yet the schema allows note threads and the spec leaves filtering undecided. Define that internal notes and other agent-only content are excluded from every customer-facing read, then add an acceptance fixture proving the behavior.
🤖 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/conversations-v1.md` around lines 211 - 221, Define customer-facing
conversation reads, including GET /conversations/{id}?embed=threads, as
excluding all internal-note and other agent-only threads; remove the open
question and document this as a mandatory contract invariant. Add an acceptance
fixture asserting that note/agent-only content is absent from the embedded
threads response.
| - **Idempotency for create is unaddressed by the consumer** — it calls create | ||
| exactly once per user submit with no client-generated idempotency key or | ||
| dedupe logic visible in `actions.ts`. **OPEN QUESTION**: does v1 need an | ||
| idempotency-key mechanism (e.g., an `Idempotency-Key` header) to protect | ||
| against retry-on-timeout double-creating a conversation? Flagged for design, | ||
| not decided here. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Specify create idempotency before implementation.
A request can time out after the conversation is persisted, and a retry can create a duplicate. Define Idempotency-Key semantics, including scope, retention, same-key/different-payload behavior, and replayed responses.
🤖 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/conversations-v1.md` around lines 241 - 246, Define the create
idempotency contract in the v1 conversation API specification before
implementation, including Idempotency-Key scope, retention period, behavior when
the same key is reused with a different payload, and whether successful or
failed responses are replayed. Update the create endpoint requirements and
referenced consumer behavior accordingly.
The founding six-operation customer-side contract, derived from what Resonant IQ's production support integration actually consumes (its client code is the de facto contract). Wire shapes compatible with the REST subset it already parses — cutover becomes a base-URL + auth-header swap. Helpthread-native Bearer auth, error envelope, server-enforced validation limits, shared TypeScript shapes.
Six OPEN QUESTIONs flagged for design rather than guessed:
Proof pass: verified the client's lowercasing and the Conversation.number field against source; fixed create-error semantics (bad mailboxId = 400, not 500); trimmed two defensive asides.
Jira: https://resonantiq.atlassian.net/browse/HT-6
🤖 Generated with Claude Code
https://claude.ai/code/session_01TqG66PPZreBrj17VbAqe3b
Summary by CodeRabbit