docs(api): close the GA under-specification gaps in the /v1 contract - #784
Merged
Conversation
Pre-GA precision pass. Every change below documents or locks behavior that is already real and enforced; none of it changes what the server does. The spec is emitted from the handlers, so all of it lands as Go annotations plus `make spec`. contact.due was the only delivered event with no schema pointer anywhere. Stable payloads map through `x-e2a-event-data-schemas`; even the beta `agent.suppression_added` had an `x-e2a-beta-event-data-schemas` entry. contact.due had neither — its shape lived only in a docs/events.md sentence, and its `data` was a hand-built map[string]any, the one published payload outside the golden-fixture regime. It is now the typed `eventpayload.ContactDueData` (+ `ContactDueContact`), published as a beta component schema, mapped from the beta envelope extension, and locked by `contact.due.json` / `contact.due.min.json` like every other event. The typing is byte-neutral: the struct's field order is the map's key order, and `TestEventForDueIsByteIdenticalToTheLegacyMap` compares the new builder against the old one verbatim across four input shapes. `eventpayload.BetaEvents` now catalogs the published-but-unfrozen payloads the way StableEvents catalogs the frozen ones, so registration, the standalone anchors, the stability markers, and the fixture gates are all data-driven instead of hardcoding one schema name. Also documented, each verified against the enforcement code: - SMTP mailbox octet limits (local part <= 64, addr-spec <= 254, counted in UTF-8 bytes) on send/reply/forward to/cc/bcc (400 invalid_recipient) and reply_to (400 invalid_request). Enforced by outbound.ValidateMailboxAddress since #750; mentioned by neither the spec nor docs/api.md, so a client generating long plus-addressed local parts got an unexplained rejection. - Location on createContact/upsertEngagement 201: the impl uses url.PathEscape, which legally leaves `@` RAW. Both forms are valid and neither was promised, so the convention is now pinned — percent-decode the final path segment, then compare. - ETag on getContact/getEngagement/updateContact/upsertEngagement: strong quoted 32-hex validators, documented as opaque and replay-verbatim. Both headers now resolve to shared `components.headers` entries, so one wording covers every response that emits them. - Contact metadata bounds: maxProperties and propertyNames.maxLength are published on the four request schemas that route through validateContactMetadata, with the value/total/scalar rules in prose. They ride in Extensions rather than struct tags on purpose — a huma tag would move an over-limit request from the handler's 400 to a 422 at the edge, which would be a behavior change. - UserExport's Message gained scheduled_at with no description and no beta marker, unlike its MessageView siblings; it now carries both. It keeps omitting thread_id, deliberately: that is a server-owned projection of our reply topology, not the account's data, and the same reasoning already keeps it out of stored events and webhook payloads. Recorded in docs and pinned by a test. - deleteEngagement was the only one of the four outreach operations whose description omitted the agent-scoped-credentials sentence, though all four route through resolveOutreachAgent/requireAgentAccess. oasdiff reports zero breaking changes. SDK bases regenerated (descriptions plus the two new beta payload models; no type changed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
||
| from __future__ import annotations | ||
| import pprint | ||
| import re # noqa: F401 |
|
|
||
| from __future__ import annotations | ||
| import pprint | ||
| import re # noqa: F401 |
| * Do not edit the class manually. | ||
| */ | ||
|
|
||
| import { HttpFile } from '../http/http.js'; |
| */ | ||
|
|
||
| import { ContactDueContact } from '../models/ContactDueContact.js'; | ||
| import { HttpFile } from '../http/http.js'; |
| import { Attachment } from '../models/Attachment.js'; | ||
| import { AttachmentMetaView } from '../models/AttachmentMetaView.js'; | ||
| import { AttachmentView } from '../models/AttachmentView.js'; | ||
| import { ContactDueContact } from '../models/ContactDueContact.js'; |
| import { AttachmentMetaView } from '../models/AttachmentMetaView.js'; | ||
| import { AttachmentView } from '../models/AttachmentView.js'; | ||
| import { ContactDueContact } from '../models/ContactDueContact.js'; | ||
| import { ContactDueData } from '../models/ContactDueData.js'; |
| import { Attachment } from '../models/Attachment.js'; | ||
| import { AttachmentMetaView } from '../models/AttachmentMetaView.js'; | ||
| import { AttachmentView } from '../models/AttachmentView.js'; | ||
| import { ContactDueContact } from '../models/ContactDueContact.js'; |
| import { AttachmentMetaView } from '../models/AttachmentMetaView.js'; | ||
| import { AttachmentView } from '../models/AttachmentView.js'; | ||
| import { ContactDueContact } from '../models/ContactDueContact.js'; | ||
| import { ContactDueData } from '../models/ContactDueData.js'; |
| import { Attachment } from '../models/Attachment.js'; | ||
| import { AttachmentMetaView } from '../models/AttachmentMetaView.js'; | ||
| import { AttachmentView } from '../models/AttachmentView.js'; | ||
| import { ContactDueContact } from '../models/ContactDueContact.js'; |
| import { AttachmentMetaView } from '../models/AttachmentMetaView.js'; | ||
| import { AttachmentView } from '../models/AttachmentView.js'; | ||
| import { ContactDueContact } from '../models/ContactDueContact.js'; | ||
| import { ContactDueData } from '../models/ContactDueData.js'; |
Resolves conflicts with the #780/#776/#781/#782/#783/#772 batch: - contacts_import.go: keep #780's removal of the per-row maxLength schema tags (over-long rows must fail per-row, not 422 the batch); keep this branch's Metadata doc rewording. - outbound.go: combine both edits to the 400 description — this branch's SMTP octet limits on invalid_request/invalid_recipient plus #780's filename/CRLF cases on invalid_attachment. - api/openapi.yaml + both SDK generated files: regenerated from the resolved sources (make spec, make generate-sdk). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Pre-GA precision pass on the
/v1contract. The v1.5.0 audit found the wirecontract backward-compatible but under-specified — behavior that is real and
enforced, where the spec says nothing. Silence is the dangerous category for a
GA freeze: a client cannot tell an intentional guarantee from an accident.
Every change here documents or locks behavior that already exists. No wire
behavior changed.
api/openapi.yamlis emitted from the handlers, so all of itlands as Go annotations plus
make spec.Priority 1 —
contact.duehad no schema pointer anywhereWhat the code did. Stable event payloads map through
x-e2a-event-data-schemas; the betaagent.suppression_addedhad anx-e2a-beta-event-data-schemasentry.contact.duewas in neither. Itsdatawas a hand-builtmap[string]anyininternal/contactdue/publisher.go(pre-change lines 79-101), and its shape existed only as prose in
docs/events.md:173— "— (untyped, built ininternal/contactdue)". It wasthe one delivered event outside the golden-fixture regime that locks every other
payload.
What landed.
eventpayload.ContactDueData+ContactDueContact(
internal/eventpayload/payloads.go:248-297), fully doc-annotated.internal/contactdue/publisher.go:69-92builds the typed struct.x-stability-level: beta, mappedfrom
x-e2a-beta-event-data-schemas, anchored inx-e2a-standalone-schema-refs(internal/httpapi/eventpayload_schemas.go).internal/eventpayload/testdata/contact.due{,.min}.json,generated by
internal/eventpayload/contact_due_golden_test.goand assertedby the real builder via
goldenassert.Data.docs/events.md:173now namesContactDueDataand states the presence rules.Wire-neutrality — verified, not asserted.
TestEventForDueIsByteIdenticalToTheLegacyMap(
internal/contactdue/publisher_test.go:88-135) keeps the old map builderverbatim and requires
json.Marshalof both to be byte-identical across fourinput shapes (full, minimal, no-last-outbound, nil-metadata). The struct's field
order is deliberately the alphabetical order
encoding/jsonused for the map, soeven key ordering is unchanged. The comment says to delete the legacy builder
once
contact.dueis declared stable.Prose vs implementation: they agreed. The builder emits
last_outbound_atand
last_conversation_idconditionally, anddocs/events.mdalready listedboth as optional. Nothing had to be reconciled.
Structural note. Rather than hardcode a second beta schema name in four
places,
eventpayload.BetaEventsnow catalogs published-but-unfrozen payloadsthe way
StableEventscatalogs frozen ones.catalog_test.goenforces thatevery beta entry is also in
webhookpub.ExperimentalEventTypesand never inStableEvents, so promoting an event is one coordinated move.StableEventwasrenamed
EventPayloadContract(one file; only the values were referencedelsewhere).
Priority 2 — enforced but unspecified
(a) SMTP mailbox octet limits
Code.
outbound.ValidateMailboxAddress(
internal/outbound/sender.go:566-582): local part ≤ 64 octets, addr-spec ≤ 254octets, counted in bytes. Reached from
agent.validateRecipients(
internal/agent/api.go:1944) →400 invalid_recipient, and fromvalidateReplyTo(internal/httpapi/outbound.go:774) →400 invalid_request.Neither the spec nor
docs/api.mdmentioned it, so a client generating longplus-addressed local parts got an unexplained rejection — and the
maxLength: 320the schema did advertise counts code points, a different unit on a different
string.
Documented. The
to/cc/bcc/reply_todescriptions onSendEmailRequest/ReplyRequest/ForwardRequest, the400responsedescription on the three send operations, and a new Email-address limits
bullet in
docs/api.md. The two error codes are stated separately because thecall sites genuinely differ.
TestDocumentedOctetLimitsMatchTheEnforcerties thenumbers in the prose to the function that rejects (64/65 boundary, a 17-emoji
local part, the 254/255 addr-spec boundary).
(b)
LocationencodingCode.
url.PathEscapeatinternal/httpapi/contacts.go:399andinternal/httpapi/engagements.go:453. Verified empirically: it leaves@ + & = : $raw and escapes' / ; , ?and non-ASCII — so/v1/contacts/a.partner@fund.vcround-trips unescaped. Our own conformance suitehad to decode-then-compare because neither form was promised.
Documented. A shared
components.headers/ResourceLocationentry pins theconvention: percent-encoded per segment, sub-delimiters may appear unescaped,
percent-decode the final path segment before comparing, never string-compare
against a locally built URL.
TestLocationEncodingDescriptionMatchesTheImplementationasserts the claimagainst
url.PathEscapeitself rather than trusting the prose.(c)
ETagformatCode.
contactETag/engagementETagemit strong quoted 32-hex validators(
internal/httpapi/contacts.go:67-73,internal/httpapi/engagements.go:93-102).Only
upsertEngagement's 201 header had any description ("Opaque revisiontoken"); the
getContact/getEngagement/updateContact200 ETags were baretype: string.Documented. A shared
components.headers/ETagentry: opaque, strong,currently a quoted 32-hex token, replay verbatim in
If-Match, moves onevery accepted write, stale ⇒
412 precondition_failed. All six declarations now$refit, includingupsertEngagement's hand-declared 201.(d) Contact metadata bounds
Code.
validateContactMetadata(internal/httpapi/contacts.go:255-290)enforces exactly: ≤ 50 keys, key ≤ 128 bytes, string value ≤ 4096 bytes, encoded
object ≤ 16384 bytes, scalars only. The schema said
additionalProperties: {}and the prose said "see the field bounds in the API docs".
Encoded on the four request schemas that route through it
(
CreateContactRequest,UpdateContactRequest,UpsertEngagementRequest,ContactImportRow):maxProperties: 50— identical to the key-count check.propertyNames.maxLength: 128— the server counts bytes, JSON Schemacounts code points, so the published keyword is never stricter: 129 code
points is ≥ 129 bytes, which the server also rejects. It cannot reject anything
the server currently accepts.
Value-size / total-size / scalars-only stay in prose (now stated concretely
instead of pointing elsewhere): expressing them needs constraints on
additionalProperties, which would change the generated SDK's metadata type froma plain map into a union — a much larger change than documenting a bound.
Why Extensions, not struct tags. huma's
maxPropertiestag also makes huma'sown request validator enforce it, moving an over-limit request from the handler's
400 invalid_requestto a422at the edge. That is a behavior change.TestContactMetadataBoundsDoNotChangeRuntimeRejectionpins both halves: thehandler still returns 400, and
huma.Schema.MaxPropertiesstays nil.(e)
UserExport'sMessagescheduled_atarrived with no description and no beta marker, unlikeMessageView/MessageSummaryView. It now carries the same doc string andx-stability-level: beta(internal/identity/store.go:370,internal/httpapi/stability.go:268-273).thread_id: deliberately still absent.identity.Message.ThreadIDisjson:"-"with an existing comment stating that account export must not acquireit through
Message's JSON representation. That reasoning holds, and this PRmakes it explicit rather than reversing it:
thread_idis a server-ownedprojection of e2a's mailbox-local reply topology, not a fact about the user's
data, and
docs/events.md:150-153already excludes it from stored events,webhook payloads, and WS notifications for exactly that reason. A GDPR Art. 15
dump should not export our internal graph.
scheduled_atis the opposite case —a scheduled-but-unsent message is the account's own pending data. Recorded in
docs/api.mdand pinned byTestExportMessageOmitsThreadID.(f)
deleteEngagementdescriptionAll four outreach operations route through
resolveOutreachAgent→requireAgentAccess(internal/httpapi/engagements.go:240,459), but onlydeleteEngagementomitted the agent-scoped-credentials sentence, which read asif un-enrolling were account-only. Added, and
TestOutreachOperationsAllStateAgentScopenow requires all four to say it.Discrepancies found (reported, not silently changed)
contact.dueprose vs builder — none. The builder anddocs/events.mdagreed on all nine fields, including the two optional ones.
validateContactAddress(
internal/httpapi/contacts.go:233-251) parses withnet/mailbut doesnot call
ValidateMailboxAddress, socreateContact/importContactsaccept an address a later send will reject with
400 invalid_recipient. Thedocs state precisely where the limits are enforced and make no claim about
contacts. Closing the asymmetry is validation logic and a behavior change, so
it is out of scope here — flagging it as a possible follow-up.
upsertEngagement's hand-declared 201 describedETagdifferently fromits own auto-derived 200 (which had no description at all). Both now resolve
to the same component.
Verification
make spec/make spec-check— clean.make openapi-compat-check(oasdiff v1.23.0,--fail-on WARN,--stability-level stable) → "No breaking changes to report".make generate-sdk-check— regenerated, then clean. The SDK diff isdescriptions plus two new beta models (
ContactDueData,ContactDueContact);no existing type changed —
metadatais still a plain string-keyed map andMessage.scheduledAtis stillDate.go test ./internal/httpapi/ ./internal/eventpayload/... ./internal/contactdue/...— pass.go test -short ./internal/identity/— pass.New tests:
contact_due_golden_test.go,contact_metadata_schema_test.go,contract_headers_addressed_test.go,export_message_schema_test.go,recipient_limits_doc_test.go, plus the byte-identity and beta-catalog tests.Deliberately not done: no TS/Python SDK payload test against the
contact.duefixture. The SDKs carry no stable-event guard for a beta event, and adding one
would widen a documentation PR into SDK-parity work.
🤖 Generated with Claude Code