Repository navigation
Add the Embeds service: create and get an embed - #1019
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-SDK generator changes require human review and Swift CI confirmation, with a Go encoded-ID round-trip issue still unresolved.
3 open issues
What changed in this PR
Adds create/get embed support across all seven SDKs through the Smithy-to-generated-service pipeline, enabling embeds in rich text ahead of the server deployment.
Changes:
- Adds Embeds models, services, client accessors, and operation-specific retry metadata.
- Encodes string path parameters in Ruby, Python, Kotlin, and Swift.
- Adds unit/conformance coverage, shared fixtures, and documentation updates.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
| typescript/tests/services/embeds.test.ts | Tests embed operations and escaping. |
| typescript/src/index.ts | Exports Embeds service and request type. |
| typescript/src/generated/services/index.ts | Exports generated service. |
| typescript/src/generated/services/embeds.ts | Adds create/get methods. |
| typescript/src/generated/schema.d.ts | Adds embed API types. |
| typescript/src/generated/path-mapping.ts | Maps embed routes to operations. |
| typescript/src/generated/openapi-stripped.json | Adds account-relative embed contract. |
| typescript/src/generated/metadata.ts | Adds embed retry policies. |
| typescript/src/client.ts | Wires Embeds accessor. |
| typescript/scripts/generate-services.ts | Registers embed service naming. |
| typescript/README.md | Lists Embeds service. |
| swift/Tests/BasecampTests/EmbedsServiceTests.swift | Tests embed operations and escaping. |
| swift/Sources/BasecampGenerator/Utilities.swift | Generates encoded string parameters. |
| swift/Sources/BasecampGenerator/ServiceGrouper.swift | Registers Embeds grouping. |
| swift/Sources/BasecampGenerator/ServiceEmitter.swift | Identifies string path parameters. |
| swift/Sources/BasecampGenerator/MethodNaming.swift | Registers embed method naming. |
| swift/Sources/Basecamp/Services/BaseService.swift | Adds path-segment encoding helper. |
| swift/Sources/Basecamp/Generated/Services/SchedulesService.swift | Encodes occurrence date parameter. |
| swift/Sources/Basecamp/Generated/Services/EmbedsService.swift | Adds create/get methods. |
| swift/Sources/Basecamp/Generated/Models/Embed.swift | Adds embed response model. |
| swift/Sources/Basecamp/Generated/Models/CreateEmbedRequest.swift | Adds creation request model. |
| swift/Sources/Basecamp/Generated/Metadata.swift | Adds retry and idempotency metadata. |
| swift/Sources/Basecamp/Generated/AccountClient+Services.swift | Adds Embeds accessor. |
| swift/README.md | Updates service inventory. |
| spec/overlays/tags.smithy | Tags embed operations. |
| spec/fixtures/manifest.yaml | Registers embed fixture coverage. |
| spec/fixtures/embeds/pending.json | Adds pending response fixture. |
| spec/fixtures/embeds/active.json | Adds active response fixture. |
| spec/bc3-route-allowlist.yml | Records temporary upstream route exceptions. |
| spec/basecamp.smithy | Defines embed operations and shapes. |
| spec/api-gaps/README.md | Registers absorbed embed contract. |
| spec/api-gaps/embeds.md | Documents contract and deployment dependency. |
| SPEC.md | Updates inventory and conformance documentation. |
| SECURITY.md | Documents embed retry exception. |
| scripts/test-check-operation-assignment-parity.rb | Exercises nested Swift interpolation. |
| scripts/enhance-openapi-go-types.sh | Marks embed image URL nullable. |
| scripts/check-operation-assignment-parity | Recognizes nested Swift interpolation. |
| scripts/check-idempotency-parity | Updates expected operation counts. |
| rust/generator/src/naming.rs | Registers embed method naming. |
| rust/generator/names.toml | Registers Embeds grouping. |
| rust/basecamp-sdk/tests/services.rs | Adds wire and error smoke tests. |
| rust/basecamp-sdk/tests/guarantees.rs | Updates operation-count assertions. |
| rust/basecamp-sdk/tests/embeds.rs | Tests embed contract and escaping. |
| rust/basecamp-sdk/src/generated/types.rs | Adds embed request/response types. |
| rust/basecamp-sdk/src/generated/services/people.rs | Updates retry-exception documentation only. |
| rust/basecamp-sdk/src/generated/services/mod.rs | Exports Embeds module. |
| rust/basecamp-sdk/src/generated/services/embeds.rs | Adds create/get methods. |
| rust/basecamp-sdk/src/generated/routes.rs | Adds embed route definitions. |
| rust/basecamp-sdk/src/generated/mod.rs | Updates operation count. |
| rust/basecamp-sdk/src/generated/metadata.rs | Adds embed retry policies. |
| rust/basecamp-sdk/src/generated/accessors.rs | Adds Embeds accessor. |
| ruby/test/basecamp/services/embeds_service_test.rb | Tests embed operations and escaping. |
| ruby/scripts/generate-services.rb | Registers Embeds and generates escaping. |
| ruby/README.md | Lists Embeds service. |
| ruby/lib/basecamp/generated/types.rb | Adds Embed type. |
| ruby/lib/basecamp/generated/services/schedules_service.rb | Encodes occurrence date parameter. |
| ruby/lib/basecamp/generated/services/embeds_service.rb | Adds create/get methods. |
| ruby/lib/basecamp/generated/services/base_service.rb | Adds path-segment encoding helper. |
| ruby/lib/basecamp/generated/metadata.json | Adds embed retry policies. |
| ruby/lib/basecamp/client.rb | Adds Embeds accessor. |
| python/tests/services/test_embeds_service.py | Tests sync/async embed operations. |
| python/src/basecamp/generated/types.py | Adds embed typed dictionaries. |
| python/src/basecamp/generated/services/schedules.py | Encodes occurrence date parameters. |
| python/src/basecamp/generated/services/people.py | Updates retry-exception documentation only. |
| python/src/basecamp/generated/services/embeds.py | Adds sync/async create/get methods. |
| python/src/basecamp/generated/services/_base.py | Adds synchronous path-encoding helper. |
| python/src/basecamp/generated/services/_async_base.py | Adds asynchronous path-encoding helper. |
| python/src/basecamp/generated/services/__init__.py | Exports Embeds services. |
| python/src/basecamp/generated/metadata.json | Adds embed retry policies. |
| python/src/basecamp/client.py | Adds synchronous Embeds accessor. |
| python/src/basecamp/async_client.py | Adds asynchronous Embeds accessor. |
| python/scripts/generate_services.py | Registers Embeds and generates escaping. |
| python/README.md | Lists Embeds accessor. |
| openapi.json | Adds generated embed contract. |
| kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/EmbedsServiceTest.kt | Tests embed operations and escaping. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/BaseService.kt | Adds path-segment encoding helper. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/Types.kt | Adds creation request body. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/schedules.kt | Encodes occurrence date parameter. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/embeds.kt | Adds create/get methods. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/ServiceAccessors.kt | Adds Embeds accessor. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Embed.kt | Adds serializable embed model. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/Metadata.kt | Adds embed retry policies. |
| kotlin/README.md | Updates service inventory. |
| kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/ServiceEmitter.kt | Generates encoded string parameters. |
| kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/Config.kt | Registers embed service and types. |
| kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt | Dispatches embed conformance operations. |
| go/README.md | Lists Embeds service. |
| go/pkg/generated/client.gen.go | Generates embed wire operations and types. |
| go/pkg/basecamp/url-routes.json | Adds embed URL routing entries. |
| go/pkg/basecamp/embeds.go | Adds public service wrapper and model. |
| go/pkg/basecamp/embeds_test.go | Tests contract, retries, hooks, and escaping. |
| go/pkg/basecamp/client.go | Adds cached Embeds accessor. |
| go/pkg/basecamp/catalog/catalog.json | Adds embed operation catalog entries. |
| go/grouped-client-inventory.yml | Records new ungrouped operations. |
| conformance/tests/embeds.json | Adds shared embed contract cases. |
| conformance/runner/typescript/runner.test.ts | Dispatches embed conformance operations. |
| conformance/runner/swift/Sources/ConformanceRunner/Dispatch.swift | Dispatches embed conformance operations. |
| conformance/runner/rust/src/operations.rs | Dispatches embed conformance operations. |
| conformance/runner/ruby/runner.rb | Dispatches embed conformance operations. |
| conformance/runner/python/runner.py | Dispatches embed conformance operations. |
| conformance/runner/go/main.go | Dispatches embed conformance operations. |
| behavior-model.json | Adds embed retry/idempotency behavior. |
| AGENTS.md | Updates documented operation counts. |
Review effort: Balanced
|
🤖 Pi review summary ( Round 1 (on 1789588)
Round 2 (on ac3217c): no findings. Earlier contract changes, found by reviewing the CLI that consumes this (basecamp/basecamp-cli#927):
|
|
🤖 Pi round 3 (on 8281ed5, the router-decoding commit): no findings. It checked decoding of invalid escapes and |
|
🤖 Converged on 8281ed5: CI is green (Swift included), Copilot reported on this head with no open issues, it merges cleanly, and no threads are open. Pi: 2, 0, 0. Nothing is merged or armed. Waiting on a person:
|
8281ed5 to
f951b82
Compare
|
🤖 Rebased onto main (5bcb445), now at f951b82. The rebase was mechanical: CI on f951b82: 48 pass and 5 fail. All 5 failures are the emSign intermediate-CA advisories published since this branch last ran. Main fails them too:
Every language's test job passes: Go, TypeScript, Ruby (3.2 through head), Python (3.11 through 3.14), Kotlin, Swift, and Rust MSRV and stable tests. #1021 clears all four advisories and is green. Once it merges, a rebase or merge of main should turn this PR green. Reviews: Copilot reviewed f951b82 with no open issues, and there are 0 unresolved threads. Against production (bc3#13962 is deployed): through basecamp-cli #927 built at this head, |
CreateEmbed (POST /embeds.json) and GetEmbed (GET /embeds/{sgid}) in all
seven SDKs. String path parameters are now percent-encoded as one path
segment in the Ruby, Python, Kotlin and Swift generators, since an sgid can
hold a slash.
…nish the Embeds surface bc3 now answers every create with 200 and refuses new embeds past 100 an hour with a 429 that has no Retry-After, so CreateEmbed retries on 503 only. Go hooks see the full operation ids, CreateEmbed and GetEmbed. Adds unit tests in every SDK, the embeds conformance cases and runner dispatch, the shared fixtures, client wiring, README rows, the api-gaps entry and the route waivers that hold until bc3 #13962 merges.
…erpolations in the operation assignment gate The gate read Swift paths with \\([^)]*\), which stops at the first closing paren, so a string path parameter sent as \(pathSegment(sgid)) read as an unknown path.
… and escape every slash in the Ruby sgid test
…eEmbed in SECURITY.md's retry inventory Router.MatchAPI in Go and Route::recognize in Rust returned a captured path segment still percent-encoded, so a matched embed URL's sgid would be escaped a second time on the way back out.
f951b82 to
dda9a5c
Compare

Server: https://github.com/basecamp/bc3/pull/13962 (branch api-create-embeds)
CLI: basecamp/basecamp-cli#927
The operations
A new
Embedsservice in all seven SDKs. An agent can now create an embed for a URL, such as a YouTube video or a Dash chart, and put it in rich text as<bc-attachment sgid="…"></bc-attachment>.CreateEmbedcoversPOST /{accountId}/embeds.jsonwith{"url": …}. It is find-or-create and answers200either way. A new embed comes back withstatus: "pending"; a URL Basecamp already has an embed for comes back with that embed in whatever status it has. (bc3 kept200for new embeds so the editor's own request is answered exactly as before.)GetEmbedcoversGET /{accountId}/embeds/{sgid}. An sgid that names no embed answers404.Embedalways carriesurl,status(pending,activeornot_found) andsgid. An active embed addstype,content_type,image_url(nullable) and, when the provider gave HTML, sanitizedcontent. Those four keys are absent, not null, while the embed is pending or not found.422 {"error": "Basecamp can't embed this URL. Link to it instead."}and maps tovalidationwith the server's message. Past 100 new embeds an hour, creating another answers429 {"error": "Too many new embeds. Try again later."}with noRetry-Afterand maps torate_limit. Asking for an embed Basecamp already has still works.CreateEmbedis@basecampIdempotent(natural: true): a repeated create finds the embed the first one made and creates nothing, so a retry after a lost response or a503is safe. It declaresretryOn: [503], though. Its 429 is a verdict for the rest of the hour, not throttling, so the 429 surfaces on the first attempt. This followsUpdateProjectClientAccess's seat-limit 429. SPEC §7 Gate 3, SECURITY.md and that operation's Smithy doc now name both exceptions. It also matters for basecamp-cli: its resilience hook sets a CLI-wide block on a headerless 429 from an operation that retries on 429, and hitting the embed limit must not block the post the embed was for.Usage: create the embed, then get it about once a second while its status is
pending. When it isactive, put the<bc-attachment>in the rich text. When it isnot_found, keep the link.Go:
client.Embeds()returns the service. Its hooks seeOperationInfo{Service: "Embeds", Operation: "CreateEmbed"/"GetEmbed"}, the full operationId SPEC §12 calls for, sogenerated.GetOperationRetryOn(op.Operation)finds the operation's policy.Embedhas plain string fields, withomitemptyon the four optional ones, the wayPing.ImageURLdoes it. An emptyurlorsgidis a usage error, raised before any request. A 422 or 429 comes back as*basecamp.ErrorwithHTTPStatus,Code(validation/rate_limit) and the server'serrortext inMessage.Escaping string path parameters
An sgid is a Rails signed global id. Its strict base64 can contain
/(about 45% of embed ids in a sample I generated), and the?expires_inin the payload adds more. Go (url.PathEscape), TypeScript (openapi-fetch) and Rust (Route::fill) already percent-encoded path parameters. Ruby, Python, Kotlin and Swift interpolated them raw, so a GET for such an sgid would have gone to/embeds/<part>/<part>. Those four generators now send every string-typed path parameter through apath_segment/_path_segment/pathSegmenthelper on the hand-written base service:CGI.escapeURIComponent,urllib.parse.quote(safe=""), Ktor'sencodeURLPathPart, or an unreserved-onlyCharacterSet. The one other string path parameter,GetScheduleEntryOccurrence'sdate, is regenerated through the helper too. AYYYY-MM-DDdate encodes to itself, so its wire form is unchanged.The URL routers had the opposite gap. Go's
Router.MatchAPIand Rust'sRoute::recognizereturned a captured segment still percent-encoded, so a matched/embeds/a%2Fb.jsongave sgida%2Fb, whichGetthen escaped again. Both now percent-decode each capture after matching. The whole path is never decoded, so%2Fstays inside its segment, and+stays literal. Round-trip tests cover it in both languages.Provenance
bc3#13962 is open and unmerged, so
spec/api-provenance.jsonstays where it is. Nothing is pinned to a branch commit. As with Pings ahead of bc3 #13943, the two routes are carried by temporarysdk_routes_absent_from_bc3_docsentries inspec/bc3-route-allowlist.yml, which cite the branch's routes, views and tests.spec/api-gaps/embeds.mdregisters the contract asabsorbed-in-sdk. The repin past #13962 will find the entries stale, and they get deleted then. Until bc3#13962 deploys, the production API host answers create with 404 and get with 500.Tests
tests/embeds.rsplusservices.rs, KotlinEmbedsServiceTest, SwiftEmbedsServiceTests). They cover a create returning a new pending embed and one returning an existing active embed, the url-only body, absent and null optional fields, a 422 with its message, a 404, and the sgid sent as one percent-encoded segment. A 429 maps torate_limitin every SDK's tests, and Go, TypeScript, Python, Kotlin and Rust also show it arrives on the first attempt with retry enabled. Go also checks the empty-argument usage errors, that a 503 is retried, thatGetOperationRetryOn("CreateEmbed")excludes 429, and that hooks seeCreateEmbed/GetEmbed. I mutation-checked the escape tests in Ruby and Python: with escaping removed they fail.spec/fixtures/embeds/{pending,active}.json, registered in the fixture manifest.Embedis a covered schema.conformance/tests/embeds.json(pending create, existing active create, 422 with message, 429 on one request, get, 404), withCreateEmbed/GetEmbeddispatch in all seven runners.scripts/check-operation-assignment-parityread Swift interpolations with\\\([^)]*\), which stops at the first), so\(pathSegment(sgid))read as an unknown path. It now allows one level of nested parentheses. Its self-test's synthetic Swift renderer now emitspathSegment(…)for the string path params openapi.json declares, the way the generator does, so the positive control fails under the old regex.make checkends "All checks passed" locally on Linux. One local caveat:lint-actionsran with actionlint's shellcheck integration off, because the local shellcheck (0.9.0) flags an SC2002 style nit in.github/workflows/security.yml, a file this PR doesn't touch; zizmor ran normally. CI runs its own. Swift's build, tests and conformance runner are macOS-only and are skipped here (the Swift SDK does not build on Linux:CFAbsoluteTime), so the Swift test and runner dispatch rest on CI. The Swift generator itself ran in aswift:6.2container, andswift-check-driftpassed.Found, not changed: Ruby builds every 429 as
RateLimitErrorwith the fixed message "Rate limit exceeded" (its 403 does the same with "Access denied"), so the server'serrortext is not on the error there. SPEC §6 leaves 429 message parsing[static], so the conformance 429 case asserts code, status and a single request, not the message. The other SDKs' unit tests check the message.The
breakinglabel is there because API Compatibility counts methods added to the generatedClientInterfaceas incompatible, as it did on #1015 and #1016.Not done: the comment ledger at the top of
scripts/check-idempotency-paritygets no new paragraph for this +1/+2. I bumped the counts without adding comment lines. This PR adds no code comments.