Skip to content

chore(github): PR template with multi-surface checklist#176

Merged
jiashuoz merged 1 commit into
mainfrom
chore/pr-template
May 28, 2026
Merged

chore(github): PR template with multi-surface checklist#176
jiashuoz merged 1 commit into
mainfrom
chore/pr-template

Conversation

@jiashuoz
Copy link
Copy Markdown
Member

Summary

Adds `.github/pull_request_template.md` as a forcing function against the parity gap we hit on PR #171 — Forward shipped to Go/TS/CLI/MCP but missed the Python SDK, requiring PR #175 to close two PRs later. The template surfaces the full client matrix at PR-author time so the gap is visible at review.

What it does

When someone opens a PR via `gh pr create` or the web "New PR" form, GitHub auto-fills the body with the template. It includes:

  • Client surface checklist — explicit rows for Go, TS SDK, Python SDK (sync + async), CLI, MCP, plus migration safety, regenerated types, and tests per surface.
  • A note that rows can be deleted if the PR genuinely doesn't touch that surface.
  • A "why skipped" field so reviewers can see intentional omissions instead of guessing.
  • Standard sections for summary, operational risk, and test plan.

What it doesn't do

This is a forcing function for authors and reviewers — it doesn't fail CI. The next step (deferred) is the automated surface-coverage check that parses `openapi.yaml` and the SDK source to assert every endpoint has a method in every SDK. That's a one-PR-sized chunk on its own.

Test plan

  • `gh pr create` against this branch picks up the template body (this PR's body should reflect the template's structure)
  • After merge, open a sample PR and confirm the GitHub web UI auto-fills it

Adds .github/pull_request_template.md as a forcing function against
the parity gap we hit on #171 (Forward shipped to Go/TS/CLI/MCP but
missed Python; needed #175 to close). The template surfaces the full
client matrix (Go, TS SDK, Python SDK sync+async, CLI, MCP) plus
migration safety, generated-types refresh, and a per-surface tests
row.

GitHub auto-fills this on `gh pr create` and the web "New PR" form,
so it's free to author and free to enforce. Rows are individually
deletable for PRs that don't touch the API or any client.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jiashuoz jiashuoz merged commit 99fe972 into main May 28, 2026
10 checks passed
jiashuoz added a commit that referenced this pull request May 29, 2026
* feat(api): conversations — thin read layer over conversation_id (Go)

Slice A of the Conversations API: backend end-to-end. Read-only.
Surfaces conversations (groups of messages sharing conversation_id)
as a first-class resource without adding any persistence — every
field is computed at read time over the existing messages table.

Adds:
- Migration 022: idx_messages_agent_conv_created (agent_id,
  conversation_id, created_at DESC) WHERE conversation_id <> ''.
  CONCURRENTLY-built behind the e2a:no-transaction directive
  (mirrors migration 021's pattern) so prod deploys don't block
  writes on the multi-million-row messages table. Partial index
  excludes the empty-conversation rows — keeps it small.
- identity.ListConversationsByAgent: groups by conversation_id,
  aggregates last/first_message_at, message/inbound/outbound
  counts, has_unread, latest subject + sender. Sorts by most-
  recent activity. Hard-capped at 100 per response; since/until
  brackets the window via HAVING MAX(created_at).
- identity.GetConversationByID: returns the aggregate + every
  member message (oldest-first) + computed participants union
  (sender + recipient + to + cc + bcc) + labels union. Cross-
  agent / missing returns ErrMessageNotFound — indistinguishable
  to avoid leaking existence.
- GET /api/v1/agents/{email}/conversations + .../{id} handlers.
  Both auth + rate-limited under the existing pollLimit.
- ConversationSummary, ConversationDetail, ListConversationsResponse
  in api_docs.go.

Tests (20):
- internal/identity/conversations_test.go (9): groups by id,
  sort by recent-activity DESC, limit clamp, since/until window,
  aggregate field correctness, chronological ordering on detail,
  participants + labels union, NotFound, cross-agent isolation.
- internal/agent/conversations_api_test.go (11): unauthorized,
  agent-not-found, summary shape + groups, invalid since/until
  (3 subtests), empty-conversation_id exclusion, detail
  unauthorized, detail not-found, detail shape (labels + participants
  + chronological + always-non-null labels per message), cross-agent
  404.

Deferred for slice 1:
- Pagination beyond 100 (cursor-based, when needed)
- ?label=, ?status= filters
- DELETE / PATCH on conversations
- Cross-agent / user-scoped /conversations endpoint

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(clients): expose Conversations API on TS/Python SDKs, CLI, MCP

Slice B of Conversations: wires GET /conversations + /conversations/{id}
through every client surface. Matches the parity checklist from the
new PR template (#176).

TS SDK:
- api.listConversations / api.getConversation (raw).
- client.listConversations / client.getConversation (high-level,
  agent-email resolved from constructor or opts).

Python SDK (sync + async):
- E2AApi.list_conversations + .get_conversation.
- AsyncE2AApi mirrors.
- E2AClient / AsyncE2AClient high-level wrappers that take the
  agent_email kwarg with the usual fallback.

CLI:
- `e2a conversations list [--limit N] [--since ts] [--until ts]`
  with a paginated table (id / msgs / unread / latest subject /
  last activity).
- `e2a conversations show <id>` printing the aggregate + member
  messages in a compact table.

MCP:
- `list_conversations` and `get_conversation` tools. Descriptions
  steer the LLM toward conversations for thread-level questions and
  messages for single-message reads. Tool registry assertions
  (stdio + http) updated; +2 positive plumbing tests.

Tests:
- Python: 134 pass (+8 — raw list + detail with shape assertions,
  high-level sync + async smoke).
- TS SDK: 85 pass (no new tests; existing harness covers types).
- CLI: 103 pass (no new tests; conversations is a thin print layer
  over the SDK).
- MCP: 92 pass (+2 new plumbing tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(conversations): review followups — shared serializer, path-param cap, drift guard

Review followups for PR #177:

1. MEDIUM — extract shared messageSummary serializer.
   The detail handler was hand-rolling a map[string]interface{} per
   member message, omitting hitl_status / webhook_status /
   webhook_error / size_bytes and at risk of drifting from the list
   endpoint's MessageSummary contract on any future field add. Hoist
   messageSummary + new messageSummaryFromIdentity helper to package
   scope; refactor both handleGetMessages and handleGetConversation
   to share them. Adding a field is now a one-place edit.

2. MEDIUM — length cap + validateConversationID on the path param.
   handleGetConversation read the path param straight into the DB
   query. Caps at maxFilterStr (200, hoisted to package scope so the
   list filter and the path param share the constant) and rejects
   CRLF — matches the list endpoint's ?conversation_id= validator.

3. LOW — document has_unread semantics on the OpenAPI annotation.
   The field's "only inbound + unread counts; outbound never
   contributes" rule was in the handler doc but not the wire schema.
   Regen'd OpenAPI + TS/Python types pick it up.

Tests (+3):
- TestGetConversation_RejectsOversizedID: 250-char path → 400.
- TestGetConversation_RejectsCRLFInID: %0D%0A in path → 400.
- TestGetConversation_MessageRowHasAllSummaryFields: contract
  regression — the JSON keys on a member message in the detail
  response must be a superset of the list endpoint's row for the
  same message. Catches future MessageSummary additions that one
  endpoint plumbs and the other forgets.

Deferred from review:
- Python SDK _encode_email rename (cosmetic; touches many files).
- Live e2e against running binary (separate exercise).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant