Repin provenance to current bc3 HEAD; pin the participant_ids contract - #491
Conversation
jeremy
added
bug
github-actions
Bot
added
typescript
Spec Change Impact
Checklist of SDKs needing updates:
|
![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: f60bc7d116
ℹ️ 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".
| @@ -0,0 +1,121 @@ | |||
| --- | |||
| gap: everything-tasks-aggregate | |||
| status: partial-coverage | |||
There was a problem hiding this comment.
Because this entry describes two JSON routes that are already serving in BC3 but remain absent from Smithy, partial-coverage misclassifies an SDK-absorption gap as incomplete upstream coverage. Use the corresponding addressed-in-bc3-pr-N status and bc3_pr reference so the advanced provenance pin accurately records how this unmodeled upstream contract was triaged.
AGENTS.md reference: AGENTS.md:L275-L275
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Repins the SDK provenance to the current basecamp/bc3 HEAD and documents/tests the newly fixed UpdateScheduleEntryInput.participant_ids presence contract (omission preserves participants; explicit [] clears), plus registers the newly observed Everything /tasks aggregate gap for later absorption.
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.
Changes:
- Repin provenance + bump
API_VERSION/OpenAPI versions to2026-07-28across SDKs. - Document
participant_idsupdate semantics in Smithy/OpenAPI and propagate the docs into generated SDK surfaces. - Add conformance coverage for schedule-entry participant presence (plus a Swift mirror), and wire dispatch for the new conformance operation.
Reviewed changes
Copilot reviewed 18 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| typescript/src/generated/services/schedules.ts | Documents participantIds update presence semantics in the TS generated request type. |
| typescript/src/generated/schema.d.ts | Propagates the participant_ids contract into generated TS schema typings. |
| typescript/src/generated/openapi-stripped.json | Bumps OpenAPI version and adds participant_ids description. |
| typescript/src/generated/metadata.ts | Updates TS generation timestamp metadata. |
| typescript/src/client.ts | Bumps TS API_VERSION. |
| swift/Tests/BasecampTests/ScheduleEntryParticipantsTests.swift | Adds Swift mirror tests for omitted-vs-empty participant_ids wire behavior. |
| swift/Sources/Basecamp/BasecampConfig.swift | Bumps Swift apiVersion. |
| spec/basecamp.smithy | Bumps Smithy service version and documents participant_ids update semantics. |
| spec/api-provenance.json | Repins bc3 revision/date. |
| spec/api-gaps/everything-tasks-aggregate.md | Registers the new Everything /tasks aggregate endpoints as an API gap. |
| ruby/lib/basecamp/version.rb | Bumps Ruby API_VERSION. |
| ruby/lib/basecamp/generated/types.rb | Updates Ruby generated timestamp banner. |
| ruby/lib/basecamp/generated/services/schedules_service.rb | Propagates participant_ids semantics into Ruby generated service docs. |
| ruby/lib/basecamp/generated/metadata.json | Updates Ruby generation timestamp metadata. |
| python/src/basecamp/_version.py | Bumps Python API_VERSION. |
| openapi.json | Bumps OpenAPI version and adds participant_ids description. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/BasecampConfig.kt | Bumps Kotlin API_VERSION. |
| kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt | Adds conformance dispatch for UpdateScheduleEntry honoring presence semantics. |
| go/pkg/generated/client.gen.go | Propagates participant_ids contract doc into Go generated request type. |
| go/pkg/basecamp/version.go | Bumps Go APIVersion. |
| go/pkg/basecamp/api-provenance.json | Repins bc3 revision/date for Go package. |
| conformance/tests/schedule_entries_write.json | Adds conformance fixtures asserting absent vs empty participant_ids on update. |
| conformance/runner/typescript/runner.test.ts | Adds TS conformance dispatch for UpdateScheduleEntry with presence-aware spreading. |
| conformance/runner/ruby/runner.rb | Adds Ruby conformance dispatch and key-filtering helper for schedule-entry writes. |
| conformance/runner/python/runner.py | Adds Python conformance dispatch for UpdateScheduleEntry with key filtering. |
| conformance/runner/go/main.go | Adds Go conformance dispatch for UpdateScheduleEntry, setting participant IDs only when fixture key is present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 26 files
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="conformance/runner/go/main.go">
<violation number="1" location="conformance/runner/go/main.go:546">
P2: Conformance cases supplying `description`, `all_day`, or `notify` will succeed against a request that silently drops those fixture fields, masking serialization regressions for this operation. Map the remaining `UpdateScheduleEntryRequest` fields, preserving `all_day` presence with a local bool pointer.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| req := &basecamp.UpdateScheduleEntryRequest{ | ||
| Summary: getStringParam(tc.RequestBody, "summary"), | ||
| StartsAt: getStringParam(tc.RequestBody, "starts_at"), | ||
| EndsAt: getStringParam(tc.RequestBody, "ends_at"), | ||
| } |
There was a problem hiding this comment.
P2: Conformance cases supplying description, all_day, or notify will succeed against a request that silently drops those fixture fields, masking serialization regressions for this operation. Map the remaining UpdateScheduleEntryRequest fields, preserving all_day presence with a local bool pointer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At conformance/runner/go/main.go, line 546:
<comment>Conformance cases supplying `description`, `all_day`, or `notify` will succeed against a request that silently drops those fixture fields, masking serialization regressions for this operation. Map the remaining `UpdateScheduleEntryRequest` fields, preserving `all_day` presence with a local bool pointer.</comment>
<file context>
@@ -538,6 +538,22 @@ func executeOperation(ctx context.Context, account *basecamp.AccountClient, tc T
+ // (BC3 preserves only because the key is missing), an empty non-nil
+ // slice means "remove everyone".
+ entryID := getInt64Param(tc.PathParams, "entryId")
+ req := &basecamp.UpdateScheduleEntryRequest{
+ Summary: getStringParam(tc.RequestBody, "summary"),
+ StartsAt: getStringParam(tc.RequestBody, "starts_at"),
</file context>
| req := &basecamp.UpdateScheduleEntryRequest{ | |
| Summary: getStringParam(tc.RequestBody, "summary"), | |
| StartsAt: getStringParam(tc.RequestBody, "starts_at"), | |
| EndsAt: getStringParam(tc.RequestBody, "ends_at"), | |
| } | |
| req := &basecamp.UpdateScheduleEntryRequest{ | |
| Summary: getStringParam(tc.RequestBody, "summary"), | |
| StartsAt: getStringParam(tc.RequestBody, "starts_at"), | |
| EndsAt: getStringParam(tc.RequestBody, "ends_at"), | |
| Description: getStringParam(tc.RequestBody, "description"), | |
| Notify: getBoolParam(tc.RequestBody, "notify"), | |
| } | |
| if _, ok := tc.RequestBody["all_day"]; ok { | |
| allDay := getBoolParam(tc.RequestBody, "all_day") | |
| req.AllDay = &allDay | |
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fd0a46fa5
ℹ️ 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".
| @@ -0,0 +1,100 @@ | |||
| --- | |||
| gap: everything-tasks-aggregate | |||
There was a problem hiding this comment.
This new entry is absent from spec/api-gaps/README.md: the “Entries (current)” table does not list it, and the latest provenance note still ends at c3086931 (2026-07-26). Consequently, maintainers using the registry during the next drift review can miss this classification even though this file explicitly exists to prevent the routes from being re-flagged. Add the entry to the table and record the new dffa7e11b3 sync range in the registry's provenance history.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="spec/api-gaps/everything-tasks-aggregate.md">
<violation number="1" location="spec/api-gaps/everything-tasks-aggregate.md:5">
P3: `sdk_demand: none` is not a valid value — the schema (`spec/api-gaps/schema.json`) defines the enum as `["high", "medium", "low"]`. Since this field is optional and the entry now carries `confirmed-not-api-resource` status, the cleanest fix is to remove the `sdk_demand` line entirely.</violation>
<violation number="2" location="spec/api-gaps/everything-tasks-aggregate.md:8">
P3: This gap entry now documents the `dffa7e11b3` sync context, but the registry index/provenance docs are not updated alongside it. That can make this classification easy to miss during future drift triage when maintainers rely on `spec/api-gaps/README.md` as the source of truth.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| gap: everything-tasks-aggregate | ||
| status: confirmed-not-api-resource | ||
| detected: 2026-07-28 | ||
| sdk_demand: none |
There was a problem hiding this comment.
P3: sdk_demand: none is not a valid value — the schema (spec/api-gaps/schema.json) defines the enum as ["high", "medium", "low"]. Since this field is optional and the entry now carries confirmed-not-api-resource status, the cleanest fix is to remove the sdk_demand line entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spec/api-gaps/everything-tasks-aggregate.md, line 5:
<comment>`sdk_demand: none` is not a valid value — the schema (`spec/api-gaps/schema.json`) defines the enum as `["high", "medium", "low"]`. Since this field is optional and the entry now carries `confirmed-not-api-resource` status, the cleanest fix is to remove the `sdk_demand` line entirely.</comment>
<file context>
@@ -1,28 +1,31 @@
+status: confirmed-not-api-resource
detected: 2026-07-28
-sdk_demand: medium
+sdk_demand: none
bc3_refs:
introduced_in: five
</file context>
| sdk_demand: none | ||
| bc3_refs: | ||
| introduced_in: five | ||
| bc3_plan_section: "Not in API scope — web-only 'All tasks' page (bc3 e4bf93c3c2 + f697d8604d @ dffa7e11b3); HTML/Turbo Frame controllers, no respond_to :json, no jbuilder, no doc/api section" |
There was a problem hiding this comment.
P3: This gap entry now documents the dffa7e11b3 sync context, but the registry index/provenance docs are not updated alongside it. That can make this classification easy to miss during future drift triage when maintainers rely on spec/api-gaps/README.md as the source of truth.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spec/api-gaps/everything-tasks-aggregate.md, line 8:
<comment>This gap entry now documents the `dffa7e11b3` sync context, but the registry index/provenance docs are not updated alongside it. That can make this classification easy to miss during future drift triage when maintainers rely on `spec/api-gaps/README.md` as the source of truth.</comment>
<file context>
@@ -1,28 +1,31 @@
+sdk_demand: none
bc3_refs:
introduced_in: five
+ bc3_plan_section: "Not in API scope — web-only 'All tasks' page (bc3 e4bf93c3c2 + f697d8604d @ dffa7e11b3); HTML/Turbo Frame controllers, no respond_to :json, no jbuilder, no doc/api section"
routes:
- - "GET /:account_id/tasks.json → everything/assignments#index"
</file context>
21d0724 to
c21e6a8
Compare
…s semantics basecamp/bc3#12425 merged as dffa7e11b3, fixing the schedule-entry participant wipe behind #477. Repin follows the sync-time rule rather than pinning the fix commit: the pin records the upstream HEAD the spec was synced against, which is dffa7e11b3 (2026-07-28). The #12425 merge is an ancestor of it — verified with git merge-base --is-ancestor — and is recorded here as fix evidence, not as the pin value. API_VERSION tracks the pin date, 2026-07-26 to 2026-07-28. Triaged all 116 commits from the old pin c308693171. Six touch the API surface: the #12425 fix itself, the three bubble_up_url commits absorbed by the parent PR, and two that together add a NEW Everything aggregate — e4bf93c3c2 added everything/assignments, then f697d8604d renamed it to everything/tasks before it ever appeared under a pin. That aggregate is unmodeled, so it is registered as spec/api-gaps/everything-tasks-aggregate.md rather than silently carried across the repin. The entry states explicitly that this is NOT a break of my/assignments: the commit subject "Move /assignments to /tasks" reads alarming against an SDK shipping GetMyAssignments, but the moved routes are the Everything ones, and /my/assignments.json is untouched across the whole drift range. UpdateScheduleEntryInput.participant_ids now documents the contract — omission preserves, an empty array clears — with the history of why that guarantee is recent. Only the update input carries it; on create there is nothing to preserve. conformance/tests/schedule_entries_write.json pins both directions and is dispatched by all five executable runners, with a Swift mirror. The SDK's job here is narrow but load-bearing: conformance proves wire presence, and BC3's own request tests prove the server-side preserve/clear. Kotlin initially SKIPped both cases because my mock body lacked required ScheduleEntry members — the same impossible-payload defect being fixed elsewhere in this stack. Fixed rather than left as a skip. Closes #477.
I filed this as partial-coverage with a provisional response shape and an SDK absorption plan. That was wrong, and the shape was invented. At the pinned dffa7e11b3 both controllers are HTML-only: Everything::AssignmentsController#index is an empty action, and the results controller declares layout "turbo_rails/frame" — it exists to serve a Turbo Frame for the filter UI. Every view under app/views/everything/assignments is .html.erb, there is no respond_to, no JSON branch, no jbuilder template, and nothing in doc/api. A Rails route tolerating a .json suffix does not create a JSON representation. So the correct status is confirmed-not-api-resource. The entry now records a NEGATIVE result — its purpose is to stop these routes being re-flagged at the next provenance sync, since sitting inside scope module: :everything makes them look like siblings of the modeled JSON aggregates. Dropped the suggested API shape and the absorption plan rather than leaving a sketch of a payload that does not exist. Kept the note that this is not a break of my/assignments, which remains the useful part.
1fd0a46 to
d54f414
Compare
|
You're right that the At the pinned Now Noted on Rebased onto |
Closes #477. Stacked on #489 → #488 → #487 →
main.basecamp/bc3#12425 merged as
dffa7e11b3, fixing the schedule-entry participant wipe server-side. This is the SDK half.The repin follows the sync-time rule, not the fix commit
Per repository policy the pin records the upstream HEAD the spec was synced against, not the SHA that happened to contain the fix:
bc3 origin/master→dffa7e11b3337b17454d9a82301be3e94226a858(2026-07-28).git merge-base --is-ancestorpasses. (Here they coincide, since the merge is currently HEAD; the check matters because that will stop being true the moment anything else lands.)spec/api-provenance.jsonto that HEAD + date;make provenance-syncpropagated it.API_VERSIONtracked2026-07-26→2026-07-28.Drift triage — 116 commits, six touch the API
git log c308693171..dffa7e11b3 -- app/views/api config/routes.rb doc/api:dffa7e11b3the #12425 fix012ae895d1,0e4fe32dcd,21b3718af9bubble_up_urle4bf93c3c2+f697d8604dThe one genuinely new gap
e4bf93c3c2addedeverything/assignments;f697d8604dthen renamed it toeverything/tasksbefore it ever appeared under a pin. Net: a new Everything aggregate atthat the SDK does not model. Registered as
spec/api-gaps/everything-tasks-aggregate.md(partial-coverage) rather than carried silently across the repin — absorbing two new list endpoints is Phase-4-sized work, not repin work.It is not a break of
my/assignments, and the entry says so explicitly. "Move /assignments to /tasks" reads alarming against an SDK shippingGetMyAssignments, but the moved routes are the ones insidescope module: :everything. The SDK's assignment operations use/:account_id/my/assignments.json, untouched across the entire drift range —git diff c308693171..dffa7e11b3 -- config/routes.rbcontains only the twotasksadditions.Semantics, documented where they're consumed
UpdateScheduleEntryInput.participant_idsnow carries the contract — omission preserves, an empty array clears — along with why that guarantee is only days old. Only the update input gets it; on create there is nothing to preserve.Conformance
New
conformance/tests/schedule_entries_write.json, dispatched by all five executable runners plus a Swift mirror:participant_ids→ one PUT,requestBodyAbsent. This is the load-bearing one: BC3 preserves participants only because the key is missing, so a compactor emittingnull, or a default emitting[], would clear the list on the server.participant_ids: []→ one PUT with the empty array present. Ruby'scompact_paramsand Python's_compactstrip nil/None but not empty collections, which is what keeps "not addressed" distinct from "addressed and cleared".Kotlin initially SKIPped both, because my mock body lacked required
ScheduleEntrymembers (parent,bucket,creator,description_attachments) — the same impossible-payload defect being fixed elsewhere in this stack, and a good argument for Kotlin's strict decoding. Fixed rather than accepted as a skip; a skip is not a pass.Division of proof
Conformance proves SDK wire presence only. The server-side guarantee — that omission preserves and
[]clears — is proven by the BC3 request tests in #12425, where the preserve test fails against the unpatched controller with exactly the wipe signature (Expected: [525460207, 950737456], Actual: []).Full
make checkgreen.Summary by cubic
Repinned API provenance to BC3 HEAD (2026-07-28) and documented the
UpdateScheduleEntryparticipant_idscontract: omission preserves participants,[]clears. Marked Everything “tasks” as web-only and added conformance plus SDK guards to only sendparticipant_idswhen present.Bug Fixes
participant_idsis presence-bearing. SDKs now omit the key unless provided; an explicit[]is sent to clear.conformance/tests/schedule_entries_write.jsonand wired into Go, Python, Ruby, TypeScript, Kotlin; added a Swift mirror test.Refactors
dffa7e11b3and bumped API version to2026-07-28across Go, Kotlin, Python, Ruby, Swift, and TypeScript.spec/api-gaps/everything-tasks-aggregate.md) to prevent re-flagging.Written for commit d54f414. Summary will update on new commits.