Absorb external-links (doors) list surface; defer create/image/composite (bc3 #12375)#431
Absorb external-links (doors) list surface; defer create/image/composite (bc3 #12375)#431jeremy wants to merge 1 commit into
Conversation
github-actions
Bot
added
typescript
Spec Change Impact
Breaking Changes:
SDK Updates Checklist:All SDKs require regeneration due to the changes.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38e8903459
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "mode": "live", | ||
| "name": "ListRecordings type=Door decodes the external-link (door) shape", | ||
| "description": "DECODING coverage for the absorbed external-links/doors list surface (spec/api-gaps/external-links-doors.md, bc3 #12375). Drives the type=Door recordings query and validates each recording against the Recording schema, including the door-specific url/service/description/position fields. Live-dormant: validates statically until credentials are provisioned. Create/image/composite remain residual gaps (see the entry).", | ||
| "operation": "ListRecordings", |
There was a problem hiding this comment.
When the live canary runs with BASECAMP_LIVE=1, live-runner.test.ts passes every fixture operation to assertDispatchCoverage, but LIVE_OPERATIONS has no ListRecordings entry. The runner therefore aborts in beforeAll with “missing dispatch cases for: ListRecordings” before issuing any request, so this new Door field coverage can never execute; add a dispatch that calls ctx.client.recordings.list("Door", ...).
AGENTS.md reference: AGENTS.md:L310-L313
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR absorbs the external-links (doors) list surface by extending the existing ListRecordings shape: it adds Door to the RecordingType documented enum and extends the shared Recording projection with door-specific fields (position, description, service: DoorService) across all SDKs, plus Go wrapper decoding tests and a live-dormant conformance canary.
Changes:
- Extend the modeled recordings surface to support
type=Doorlistings by addingDoortoRecordingTypeand introducingDoorService. - Regenerate all SDK artifacts (TS/Ruby/Python/Swift/Kotlin/Go) and OpenAPI outputs to include the new fields/models.
- Add Go runtime decode coverage for door recordings and a
ListRecordings type=Doorconformance canary; mark the api-gap aspartial-coverage.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 7 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| typescript/src/generated/services/recordings.ts | Adds Door to the generated RecordingsService.list type union/docs. |
| typescript/src/generated/schema.d.ts | Introduces DoorService and extends Recording with door fields in TS schema types. |
| typescript/src/generated/openapi-stripped.json | Updates stripped OpenAPI schema/descriptions for DoorService + door fields. |
| typescript/src/generated/metadata.ts | Regenerated timestamp metadata. |
| swift/Sources/Basecamp/Generated/Models/Recording.swift | Adds description, position, and service to Swift Recording. |
| swift/Sources/Basecamp/Generated/Models/DoorService.swift | Adds Swift DoorService generated model. |
| spec/basecamp.smithy | Adds Door to RecordingType docs and extends Recording + defines DoorService. |
| spec/api-gaps/README.md | Updates external-links-doors status to partial-coverage. |
| spec/api-gaps/external-links-doors.md | Documents partial absorption scope + adds smithy refs and residual gaps. |
| ruby/lib/basecamp/generated/types.rb | Adds Ruby DoorService type and extends Recording with door fields. |
| ruby/lib/basecamp/generated/services/recordings_service.rb | Updates recordings service param docs to include Door. |
| ruby/lib/basecamp/generated/metadata.json | Regenerated timestamp metadata. |
| python/src/basecamp/generated/types.py | Adds DoorService TypedDict and extends Recording TypedDict with door fields. |
| openapi.json | Updates full OpenAPI schema/descriptions for door modeling. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/recordings.kt | Updates recordings service docs to include Door. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Recording.kt | Extends Kotlin Recording with door fields. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/DoorService.kt | Adds Kotlin DoorService generated model. |
| go/pkg/generated/client.gen.go | Adds generated DoorService, extends generated Recording, and updates docs for type. |
| go/pkg/basecamp/recordings.go | Adds RecordingTypeDoor, extends wrapper Recording, and maps door fields from generated types. |
| go/pkg/basecamp/recordings_test.go | Adds decode tests for Door recordings and for non-door omission behavior. |
| conformance/tests/live-my-surface.json | Adds a live-dormant canary validating ListRecordings?type=Door against the Recording schema. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// The outside address an external link points to. Present only on `Door` | ||
| /// recordings returned by the `type=Door` recordings query (the only endpoint | ||
| /// that returns the full door shape); `Recording.url` already carries the | ||
| /// recording's own URL, while this `description`/`service` pair is | ||
| /// door-specific. See `spec/api-gaps/external-links-doors.md`. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 21 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="go/pkg/basecamp/recordings_test.go">
<violation number="1" location="go/pkg/basecamp/recordings_test.go:327">
P3: TestRecording_UnmarshalDoor includes "example_url" in the service fixture but never asserts on d.Service.ExampleURL. The test comment says it verifies "the full door shape" and other service fields (Name, Code, ValidPatterns, SupportingText) are checked, so omitting ExampleURL means a regression in its decoding wouldn't be caught.</violation>
</file>
<file name="conformance/tests/live-my-surface.json">
<violation number="1" location="conformance/tests/live-my-surface.json:190">
P1: This new live fixture adds `operation: "ListRecordings"`, but the live dispatcher also needs a matching `ListRecordings` dispatch case; otherwise dispatch-coverage checks fail in `beforeAll` and the Door canary never executes when `BASECAMP_LIVE=1`. Adding the corresponding live dispatch entry keeps this fixture runnable in live mode.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "mode": "live", | ||
| "name": "ListRecordings type=Door decodes the external-link (door) shape", | ||
| "description": "DECODING coverage for the absorbed external-links/doors list surface (spec/api-gaps/external-links-doors.md, bc3 #12375). Drives the type=Door recordings query and validates each recording against the Recording schema, including the door-specific url/service/description/position fields. Live-dormant: validates statically until credentials are provisioned. Create/image/composite remain residual gaps (see the entry).", | ||
| "operation": "ListRecordings", |
There was a problem hiding this comment.
P1: This new live fixture adds operation: "ListRecordings", but the live dispatcher also needs a matching ListRecordings dispatch case; otherwise dispatch-coverage checks fail in beforeAll and the Door canary never executes when BASECAMP_LIVE=1. Adding the corresponding live dispatch entry keeps this fixture runnable in live mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At conformance/tests/live-my-surface.json, line 190:
<comment>This new live fixture adds `operation: "ListRecordings"`, but the live dispatcher also needs a matching `ListRecordings` dispatch case; otherwise dispatch-coverage checks fail in `beforeAll` and the Door canary never executes when `BASECAMP_LIVE=1`. Adding the corresponding live dispatch entry keeps this fixture runnable in live mode.</comment>
<file context>
@@ -182,5 +182,19 @@
+ "mode": "live",
+ "name": "ListRecordings type=Door decodes the external-link (door) shape",
+ "description": "DECODING coverage for the absorbed external-links/doors list surface (spec/api-gaps/external-links-doors.md, bc3 #12375). Drives the type=Door recordings query and validates each recording against the Recording schema, including the door-specific url/service/description/position fields. Live-dormant: validates statically until credentials are provisioned. Create/image/composite remain residual gaps (see the entry).",
+ "operation": "ListRecordings",
+ "method": "GET",
+ "path": "/projects/recordings.json",
</file context>
| if len(recordings) != 1 { | ||
| t.Fatalf("expected 1 recording, got %d", len(recordings)) | ||
| } | ||
| d := recordings[0] |
There was a problem hiding this comment.
P3: TestRecording_UnmarshalDoor includes "example_url" in the service fixture but never asserts on d.Service.ExampleURL. The test comment says it verifies "the full door shape" and other service fields (Name, Code, ValidPatterns, SupportingText) are checked, so omitting ExampleURL means a regression in its decoding wouldn't be caught.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At go/pkg/basecamp/recordings_test.go, line 327:
<comment>TestRecording_UnmarshalDoor includes "example_url" in the service fixture but never asserts on d.Service.ExampleURL. The test comment says it verifies "the full door shape" and other service fields (Name, Code, ValidPatterns, SupportingText) are checked, so omitting ExampleURL means a regression in its decoding wouldn't be caught.</comment>
<file context>
@@ -286,6 +287,86 @@ func TestRecordingType_Constants(t *testing.T) {
+ if len(recordings) != 1 {
+ t.Fatalf("expected 1 recording, got %d", len(recordings))
+ }
+ d := recordings[0]
+ if d.Type != "Door" {
+ t.Errorf("expected type Door, got %q", d.Type)
</file context>
277533d to
afebd93
Compare
…mposite (bc3 #12375) bc3 #12375 documents the external-link (historically "door") resource. A pre-implementation spike scoped this to the cleanly-absorbable surface: Absorbed: - Make Door reachable through the EXISTING polymorphic ListRecordings query (GET /projects/recordings.json?type=Door) rather than a new operation: ListRecordings already carries a type query and Recording.url. Add Door to the RecordingType documented enum and optional position/description/service (DoorService struct) to the shared Recording projection. - Go wrapper: RecordingTypeDoor constant, clean DoorService type, door fields on Recording, mapper wiring. Runtime decode tests prove the full door shape (external url + service struct + description + position) decodes through the projection, and that a non-door recording leaves the door fields empty. - Get/rename/trash already reuse GetTool/UpdateTool/DeleteTool (no work). - type=Door ListRecordings canary entry (validates statically; live dormant). Deferred as residual gaps (entry is partial-coverage, not absorbed-in-sdk): - Create (POST .../dock/doors.json) returns 302 + empty body redirecting cross-origin. The spike found the six SDK transports handle this inconsistently — Go/TS/Kotlin/Swift follow the redirect; Python/Ruby do not but face empty-body decode — so modeling "return only the 302/empty" response needs a cross-cutting per-operation transport change, out of scope for an additive absorption. - image thumbnail is multipart-only; unmodeled. - The SPEC §18 create-and-discover composite depends on a shippable create. Additive only (Door enum + optional Recording fields); no operation change.
38e8903 to
6947813
Compare
| smithy_refs: | ||
| - "RecordingType Door enum value (spec/basecamp.smithy:6063)" | ||
| - "Recording.position/description/service (spec/basecamp.smithy:6078)" | ||
| - "DoorService (spec/basecamp.smithy:6142)" |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6947813f47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| /// Metadata about the recognized external service the link's `url` points to | ||
| /// (Figma, Dropbox, GitHub, …). Present only on `Door` recordings. | ||
| service: DoorService |
There was a problem hiding this comment.
When type=Door returns the payload represented by the new TestRecording_UnmarshalDoor fixture—which includes every existing required Recording field except parent—the shared schema still requires parent. Consequently, generated Swift and Kotlin decoders throw for Door rows, while the live schema canary rejects the same response, so listing external links does not work in those SDKs; relax Recording.parent or use a Door-specific response shape. This fixture/schema mismatch is an explicit stop condition.
AGENTS.md reference: AGENTS.md:L124-L127
Useful? React with 👍 / 👎.
Closes #430. Part of the post-#401 BC3 follow-up absorption program (PR-4, gated by a modeling/redirect spike).
Spike outcome (three blockers)
ListRecordingsalready ownsGET /projects/recordings.jsonwith atypequery andRecording.url.Dooris reached by adding it to theRecordingTypedoc-enum + optionalposition/description/service(DoorService) on the sharedRecordingprojection — no new op, no collision.follow_redirects=False) and Ruby (Faraday) don't follow but face empty-body decode. Modeling "return only the 302/empty response" needs a coordinated per-operation transport change across all six generated transports — out of scope for an additive absorption. Create deferred.imagemultipart → unmodeled residual gap (independently blocks a full flip).Absorbed
DoorRecordingTypeenum value + optionalposition/description/service(DoorService) onRecording.url+servicestruct +description+position) decodes through the projection; a non-door recording leaves the door fields empty.GetTool/UpdateTool/DeleteTool.type=DoorListRecordingscanary (validates statically; live dormant).Deferred (residual gaps → entry is
partial-coverage, notabsorbed-in-sdk)CreateExternalLink(302 transport),image(multipart), the SPEC §18 create-and-discover composite. No JSON update path (PUT → 406) by contract.Verification
make generate && make check— clean (24 api-gap entries; additive only, no operation change).Summary by cubic
Adds external-links (doors) list support by adding
DoortoRecordingTypeand extendingRecordingwith door fields soGET /projects/recordings.json?type=Doorreturns the full door shape across SDKs. Create/image/composite are deferred due to redirect and multipart constraints (bc3 #12375).DoortoRecordingType; extendRecordingwith optionalposition,description, andservice(DoorService).urlsemantics in OpenAPI/Smithy and regenerate Go, Kotlin, Swift, Python, Ruby, and TypeScript SDKs.type=DoorListRecordingslive canary and TS dispatcher wiring.DoorinListRecordingstypeparam; reuse existing get/rename/trash; no new endpoints.Written for commit 6947813. Summary will update on new commits.