BC5 readiness: forward-compat spec, briefs, parity lint#293
Merged
Conversation
github-actions
Bot
added
documentation
Spec Change Impact
Checklist of SDKs needing updates:
|
github-actions
Bot
added
enhancement
There was a problem hiding this comment.
Pull request overview
Foundation work to prepare the SDK repo for BC5 readiness: additive Smithy/OpenAPI model updates, new repo-level spec/brief validation tooling, and regenerated multi-language SDK artifacts pinned to the updated API version.
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:
- Add BC5 forward-compat fields to the Smithy spec (notifications bubble-up fields, todo steps, todoset counts/URLs, person tagline) and regenerate OpenAPI + SDK model artifacts.
- Introduce repo lints/utilities: bucket↔flat parity check, API gap brief validator, and branch-aware BC3 drift reporting wired into
make check/make sync-status. - Add and validate API gap briefs + allowlist schema/docs, and add
make generateas a full regeneration aggregator.
Reviewed changes
Copilot reviewed 34 out of 50 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| typescript/src/generated/schema.d.ts | Generated TS schema typings updated for new BC5 fields + descriptions. |
| typescript/src/generated/openapi-stripped.json | Generated stripped OpenAPI updated (version + new schema fields/docs). |
| typescript/src/generated/metadata.json | Regeneration timestamp update. |
| typescript/src/client.ts | Bump TS SDK API_VERSION to 2026-05-01. |
| swift/Sources/Basecamp/Generated/Models/Todoset.swift | Generated Swift model updated with new todoset fields. |
| swift/Sources/Basecamp/Generated/Models/Todo.swift | Generated Swift model updated with embedded steps. |
| swift/Sources/Basecamp/Generated/Models/Person.swift | Generated Swift model updated with tagline. |
| swift/Sources/Basecamp/Generated/Models/Notification.swift | Generated Swift model updated with bubble-up fields. |
| swift/Sources/Basecamp/Generated/Models/GetMyNotificationsResponseContent.swift | Generated Swift response content updated with bubble-up collections. |
| swift/Sources/Basecamp/BasecampConfig.swift | Bump Swift SDK apiVersion to 2026-05-01. |
| spec/bucket-scoped-allowlist.txt | New allowlist for bucket-scoped list ops without flat counterparts. |
| spec/basecamp.smithy | Smithy service version bump + additive BC5 model fields. |
| spec/api-provenance.json | Provenance structure updated to track active branch + compatibility baseline. |
| scripts/validate-briefs.sh | Shell entrypoint for briefs validation (Ruby). |
| scripts/validate-briefs.rb | Ruby validator for brief frontmatter/body sections + allowlist schema checks. |
| scripts/report-bc3-drift.sh | New helper to report drift for an arbitrary BC3 baseline/branch/label. |
| scripts/check-bucket-flat-parity.sh | New lint to enforce bucket↔flat list-operation parity (with allowlist). |
| ruby/lib/basecamp/version.rb | Bump Ruby SDK API_VERSION to 2026-05-01. |
| ruby/lib/basecamp/generated/types.rb | Regenerated Ruby types with BC5 additive fields. |
| ruby/lib/basecamp/generated/metadata.json | Regeneration timestamp update. |
| python/src/basecamp/generated/types.py | Regenerated Python TypedDicts with BC5 additive fields. |
| python/src/basecamp/_version.py | Bump Python SDK API_VERSION to 2026-05-01. |
| openapi.json | Generated OpenAPI updated (version + new schema fields/docs). |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Todoset.kt | Regenerated Kotlin model updated with new todoset fields. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Todo.kt | Regenerated Kotlin model updated with embedded steps. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Person.kt | Regenerated Kotlin model updated with tagline. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/BasecampConfig.kt | Bump Kotlin SDK API_VERSION to 2026-05-01. |
| go/pkg/generated/client.gen.go | Regenerated Go client models with BC5 additive fields. |
| go/pkg/basecamp/version.go | Bump Go SDK APIVersion to 2026-05-01. |
| go/pkg/basecamp/api-provenance.json | Go copy of provenance updated to new structure. |
| briefs/api-gaps/todoset-completed-list-visibility.md | New API gap brief. |
| briefs/api-gaps/step-top-level.md | New API gap brief. |
| briefs/api-gaps/stack-doc-and-smithy.md | New API gap brief. |
| briefs/api-gaps/search-filter-additions.md | New API gap brief. |
| briefs/api-gaps/scratchpad.md | New API gap brief. |
| briefs/api-gaps/schema.json | New JSON schema for brief frontmatter. |
| briefs/api-gaps/rich-text-project-attachable.md | New API gap brief. |
| briefs/api-gaps/recording-bubbleupable-field.md | New API gap brief. |
| briefs/api-gaps/recordable-subtypes-doc.md | New API gap brief. |
| briefs/api-gaps/everything-aggregates.md | New API gap brief. |
| briefs/api-gaps/calendar.md | New API gap brief. |
| briefs/api-gaps/allowlist.yml | New allowlist for detector-classified routes that don’t require briefs. |
| briefs/api-gaps/allowlist-schema.json | JSON schema for the briefs allowlist format. |
| briefs/api-gaps/activity-timeline.md | New API gap brief. |
| briefs/api-gaps/README.md | Overview doc + table of briefs + validation instructions. |
| briefs/COORDINATION.md | Cross-team coordination doc for BC3 parity + SDK absorption lifecycle. |
| behavior-model.json | Behavior model updated for new Person.tagline JSON path. |
| Makefile | Add generate aggregator; wire new lints/validators into check; improve sync-status. |
| CONTRIBUTING.md | Document make generate, parity lint, and briefs workflow. |
| .gitignore | Ignore Kotlin .kotlin/ build artifacts. |
💡 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}
jeremy
force-pushed
the
bc5-readiness-spec-foundations
branch
from
May 1, 2026 22:21
1e1c949 to
5bd407c
Compare
github-actions
Bot
added
the
documentation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 50 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
10 tasks
jeremy
force-pushed
the
bc5-readiness-spec-foundations
branch
from
May 1, 2026 22:49
5bd407c to
63205bf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 50 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 63205bfebc
ℹ️ 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".
jeremy
force-pushed
the
bc5-readiness-spec-foundations
branch
from
May 1, 2026 23:03
63205bf to
7e9c434
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 50 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 839b69037f
ℹ️ 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".
…sh advisories Three high-severity advisories published since main's last green security run (2026-07-20) trip npm Audit on the TypeScript SDK: - fast-uri GHSA-4c8g-83qw-93j6 (host confusion via failed IDN canonicalization): override >=3.1.2 -> >=3.1.3, resolves 4.1.1 - brace-expansion GHSA-3jxr-9vmj-r5cp (DoS via exponential expansion): override >=5.0.6 -> >=5.0.7 - js-yaml GHSA-52cp-r559-cp3m (quadratic CPU via merge-key chains): new override ^4.3.0, pinned to the 4.x patched line since js-yaml 5 is a major bump for openapi-typescript's transitive use npm audit clean; ts-generate + ts-generate-services + ts-check verified green under the new resolutions.
github-actions
Bot
removed
the
documentation
jeremy
added a commit
that referenced
this pull request
Jul 22, 2026
Surface the BC5 forward-compat fields filed in go/BRIEF-bc5-forward-compat-wrappers.md through the hand-written wrapper layer. PR 1 (#293) added these on the generated client; this commit closes the wrapper gap so basecamp-cli Phase 1 can read them. - Todo: + Steps []CardStep (BC5 jbuilder reuses the steps/step partial, matching the existing CardStep shape; reuse rather than introduce a new type) - Todoset: + TodosCount, CompletedLooseTodosCount, TodosURL, AppTodosURL - Person: + Tagline (alias of Bio; surfaced as a separate wrapper field per the spec note so consumers can distinguish presence) - Notification: + BubbleUpURL, BubbleUpAt (time.Time, following the existing ReadAt/UnreadAt zero-value pattern) - NotificationsResult: + BubbleUps, ScheduledBubbleUps These wrappers all decode through the existing FromGenerated paths (Todo, Todoset, Person) or direct JSON unmarshal (Notification, NotificationsResult) with struct tags; no new conversion pattern is needed. Refs: go/BRIEF-bc5-forward-compat-wrappers.md
- sync-status: pass BC3_REPO through to report-bc3-drift.sh in both the
primary and compatibility invocations — make variables aren't exported
to recipe environments, so 'make sync-status BC3_REPO=org/fork' was
silently querying the default repo
- parity lint: use [[:space:]] instead of the non-POSIX \s escape in the
allowlist filter; accept {projectId} alongside {bucketId} in the
bucket-path pattern (and strip logic) for forward-compat with the
documented Smithy path convention; use # as the sed delimiter to keep
the ERE alternation unambiguous across GNU/BSD sed
Candidate count and lint verdict unchanged (verified against openapi.json
before/after).
github-actions
Bot
added
the
documentation
- parity lint: accept literal filter segments (e.g. /todos/completed.json)
in addition to path-parameter variants — the header always promised
(/<filter>)? coverage but the regex only matched /{param}. Candidate
set unchanged today (verified: single-resource GETs carry no .json
suffix in this spec, so the anchor keeps the match tight).
- sync-status: guard on jq alongside the existing gh guards.
github-actions
Bot
removed
the
documentation
14 tasks
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66f9dfa926
ℹ️ 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".
jeremy
added a commit
that referenced
this pull request
Jul 22, 2026
Surface the BC5 forward-compat fields filed in go/BRIEF-bc5-forward-compat-wrappers.md through the hand-written wrapper layer. PR 1 (#293) added these on the generated client; this commit closes the wrapper gap so basecamp-cli Phase 1 can read them. - Todo: + Steps []CardStep (BC5 jbuilder reuses the steps/step partial, matching the existing CardStep shape; reuse rather than introduce a new type) - Todoset: + TodosCount, CompletedLooseTodosCount, TodosURL, AppTodosURL - Person: + Tagline (alias of Bio; surfaced as a separate wrapper field per the spec note so consumers can distinguish presence) - Notification: + BubbleUpURL, BubbleUpAt (time.Time, following the existing ReadAt/UnreadAt zero-value pattern) - NotificationsResult: + BubbleUps, ScheduledBubbleUps These wrappers all decode through the existing FromGenerated paths (Todo, Todoset, Person) or direct JSON unmarshal (Notification, NotificationsResult) with struct tags; no new conversion pattern is needed. Refs: go/BRIEF-bc5-forward-compat-wrappers.md
jeremy
added a commit
that referenced
this pull request
Jul 22, 2026
…eck (#309) * wrappers: standardize nested Person on personFromGenerated Every *FromGenerated function that built a nested *Person value (Creator, Completer, Approver, Booster, Participants, top-level Person) previously inlined a 6-field shallow copy that silently dropped Bio, Location, Title, PersonableType, AttachableSGID, Client, Employee, TimeZone, CanPing, CanAccessHillCharts, CanAccessTimesheet, CanManageProjects, CanManagePeople, Company, CreatedAt, and UpdatedAt on every nested context. Replace each shallow copy with a personFromGenerated() call. Pure refactor; zero field additions on the wrapper struct side. The change is forward-compat additive — fields that were dropped before are now populated when present in the generated payload; no removals. This makes every subsequent additive Person field (e.g. Tagline) flow through every nested context for free, instead of requiring per-context re-adds. * wrappers: cross-cutting Recording-shaped fields Add the pervasive Category B fields (BookmarkURL, BoostsCount, BoostsURL, CommentsCount, CommentsURL, SubscriptionURL, InheritsStatus, VisibleToClients, Title) — and Content where applicable — to the wrappers that already share the Recording shape but were missing one or more. Affected wrappers: - Recording: + Content, CommentsCount, CommentsURL, SubscriptionURL - Comment: + VisibleToClients, Title, InheritsStatus, BookmarkURL, BoostsCount, BoostsURL - Message: + VisibleToClients, Title, InheritsStatus, BookmarkURL, BoostsURL, CommentsCount, CommentsURL, SubscriptionURL - MessageBoard: + VisibleToClients, InheritsStatus, BookmarkURL - Forward: + VisibleToClients, Title, InheritsStatus, BookmarkURL, SubscriptionURL - ForwardReply: + VisibleToClients, Title, InheritsStatus, BookmarkURL, BoostsCount, BoostsURL - Inbox: + VisibleToClients, InheritsStatus, BookmarkURL - CampfireLine: + BookmarkURL, BoostsURL - ScheduleEntry: + BoostsCount, BoostsURL - QuestionAnswer: + BoostsCount, BoostsURL - Todolist: + BoostsCount, BoostsURL - TimesheetEntry: + Status, VisibleToClients, Title, InheritsStatus, Type, URL, AppURL, BookmarkURL Forward-compat additive — fields default to zero/empty when absent. Wrapper JSON serialization uses omitempty on newly-added fields to match the upstream serialization behavior. * wrappers: BC5 forward-compat fields Surface the BC5 forward-compat fields filed in go/BRIEF-bc5-forward-compat-wrappers.md through the hand-written wrapper layer. PR 1 (#293) added these on the generated client; this commit closes the wrapper gap so basecamp-cli Phase 1 can read them. - Todo: + Steps []CardStep (BC5 jbuilder reuses the steps/step partial, matching the existing CardStep shape; reuse rather than introduce a new type) - Todoset: + TodosCount, CompletedLooseTodosCount, TodosURL, AppTodosURL - Person: + Tagline (alias of Bio; surfaced as a separate wrapper field per the spec note so consumers can distinguish presence) - Notification: + BubbleUpURL, BubbleUpAt (time.Time, following the existing ReadAt/UnreadAt zero-value pattern) - NotificationsResult: + BubbleUps, ScheduledBubbleUps These wrappers all decode through the existing FromGenerated paths (Todo, Todoset, Person) or direct JSON unmarshal (Notification, NotificationsResult) with struct tags; no new conversion pattern is needed. Refs: go/BRIEF-bc5-forward-compat-wrappers.md * wrappers: one-off structural fields Close the remaining Category B one-off structural gaps identified in the field-level wrapper audit (predates BC5, surfaced during the BC5 readiness sweep). - Campfire: + Topic, Position, BookmarkURL, SubscriptionURL - MessageBoard: + AppMessagesURL, Position - Template: + URL, AppURL, Dock []DockItem (reuses the existing DockItem type from projects.go; conversion shared via dockItemFromGenerated helper) - Person: + CanAccessHillCharts, CanAccessTimesheet; also wire CanPing into personFromGenerated (struct field existed; propagation did not) - Notification: + Creator, Participants, PreviewableAttachments (with new companion PreviewableAttachment type mirroring generated.PreviewableAttachment) - Gauge: + Creator, Bucket - GaugeNeedle: + Creator, Bucket, Parent - Inbox: + Position, ForwardsCount, ForwardsURL - Forward: + RepliesCount, RepliesURL The Notification sentinel-creator test fixture was updated to include `personable_type: "LocalPerson"` (the actual BC3 wire shape for system actors) so normalizeJSON correctly coerces the string "basecamp" id to 0 and preserves it as system_label; the test now asserts the wrapper exposes the normalized creator end-to-end rather than silently dropping it. * wrappers: close remaining structural gaps surfaced by drift audit The field-level audit (formalized as the new check-wrapper-drift script in the next commit) surfaced these additional pre-existing structural gaps that the explicit lists in the prior commits did not cover. - Card: + BoostsURL (sibling of existing BoostsCount) - CardColumn: + CommentsCount (alongside the existing oddly-named CommentCount whose json tag was `comment_count`; the canonical `comments_count` tag now also flows through) - CardStep: + CompletionURL - Document: + BoostsURL - Event: + BoostsCount, BoostsURL - Todo: + VisibleToClients, SubscriptionURL, BoostsURL, CommentsCount, CommentsURL, CompletionURL, CompletionSubscribers []Person - Upload: + BoostsURL All additions follow the same forward-compat additive pattern: zero/empty when absent on the wire, populated through the existing *FromGenerated conversion paths. * scripts: field-level wrapper drift check Adds a Go AST-based field-level drift check, sibling of the existing operation-level scripts/check-service-drift.sh. The existing check confirms that every generated operation has a hand-written service wrapper; this new check confirms that every generated *struct field* is propagated through the hand-written wrapper layer (or explicitly marked as intentionally-omitted). Discovery: walks (wrapper, generated) pairs in two ways — 1. signature reading of every `xFromGenerated(g generated.Y) X` function declaration in go/pkg/basecamp/*.go (non-test). 2. an explicit `directDecodePairs` map covering wrappers that decode straight from JSON bytes (Notification, NotificationsResult, MyAssignment, Gauge, GaugeNeedle) without a *FromGenerated function. Matching is keyed on JSON tag, not Go field name, so shape-equivalent collisions (`URL string \`json:"url"\`` vs `Url string \`json:"url"\``) are handled correctly. Opt-out marker format // intentionally-omitted: <tag> - <reason> (ASCII hyphen + space + reason, validated by regex) can appear anywhere inside the wrapper struct body. Wired into: - new `make go-check-wrapper-drift` target - existing `make check` aggregate (next to `go-check-drift`) - go.work `use` block, so `go run ./scripts/check-wrapper-drift/` works without extra setup. Self-tests in main_test.go exercise the parser/extractor helpers, the marker regex (reason-required), the excluded-FromGenerated exception (webhookPersonFromGenerated → WebhookEventPerson), and a happy/drift end-to-end pair. Result on the fully-fixed tree: 51 (wrapper, generated) pairs walked, 842 generated fields verified, zero drift, zero intentionally-omitted markers required. * wrappers: tests for new field propagation Adds wrapper_propagation_test.go with three test patterns: 1. Test<Type>FromGenerated_PropagatesNewFields — one per wrapper file that gained fields. Asserts only the new fields against a fully-populated generated.X fixture; the existing *FromGenerated_FullPopulated tests still cover the full surface. 2. TestPerson_NewFields_NestedPropagation — end-to-end check that the personFromGenerated standardization causes a fully-populated nested generated.Person (Bio, Tagline, Location, Title, AttachableSGID, PersonableType, Client, Employee, TimeZone, CanPing, CanAccessHillCharts, CanAccessTimesheet, CanManageProjects, CanManagePeople, Company, CreatedAt, UpdatedAt) to flow through every nested context (Recording, Comment, Message, Todo, ScheduleEntry.Creator + Participants, Card.Creator + Completer + Assignees). Pins commit 1's standardization end-to-end. 3. Test<Type>_DirectDecode_PropagatesNewFields — JSON-fixture tests for the §C-2 direct-decode wrappers (Notification, NotificationsResult, MyAssignment, Gauge, GaugeNeedle) that decode via json.Unmarshal on the (sometimes pre-normalized) raw body. Also commits the original brief at go/BRIEF-bc5-forward-compat-wrappers.md that triggered this PR — filed by the basecamp-cli dev agent against SDK commit 7e9c434. Preserving it in-tree keeps the audit trail of why Category A field additions look the way they do, alongside the wider audit (Categories B + C) that the brief flagged would be welcome. 21 new tests in total; all pass on the current tree. * wrappers: gofmt long-tag struct alignment `gofmt -s -w` rewrites the column-aligned tag layouts in Notification, NotificationsResult, Person, Todo, and Todoset where the newly-added fields pushed the existing alignment past gofmt's preferred width. No behavior change; closes `make fmt-check` (and thus `make check`). * scripts: wrapper drift check verifies field population, not just tags The field-level wrapper drift check previously confirmed only that each generated JSON tag had a matching tag on the wrapper struct. A wrapper field could carry the right tag yet never be assigned by its *FromGenerated conversion function, leaving it zero-valued on the wire while the check passed. Extend the check with a population pass: for every *FromGenerated-backed pair it AST-walks the conversion body and confirms each tagged wrapper field is actually assigned (via the wrapper's composite literal or a selector-target assignment). A tag-present-but-never-assigned field is now reported as drift. Direct-decode wrappers are exempt — json.Unmarshal populates them straight from the tags, so tag presence is population. The package doc spells out the check's reachability semantics and limits. Also: - Remove the dead exclusion loop in run(): it keyed off the wrapper struct name (k+"FromGenerated") but collectFromGeneratedPairs already filters by function name, so the guard never matched. The inner filter is the only effective one. - Inject the direct-decode pair map into run() so the end-to-end tests drive the real run() entry point over their own fixtures instead of reimplementing the pipeline. Replaces the partial TestEndToEnd with run()-driven cases covering in-sync, missing-tag, tag-present-but- unassigned (the population regression), both assignment forms, and stale omit-marker detection. - Drop the unquoted cd $(CURDIR) from the go-check-wrapper-drift target so it is safe on paths with spaces; go run with a relative path needs no cd. * wrappers: stop omitempty hiding false bools and zero timestamps omitempty on a bool drops a legitimate false, and omitempty on a value time.Time does not suppress the zero time at all — it marshals as 0001-01-01T00:00:00Z instead of being omitted. - Drop omitempty from the visible_to_clients and inherits_status bool response fields on TimesheetEntry, Inbox, Forward, and ForwardReply so a false value stays on the wire. This matches the generated structs, which carry these tags without omitempty. - Change Notification.BubbleUpAt from time.Time to *time.Time so an unset scheduled resurfacing time omits cleanly (nil) rather than emitting the zero-time sentinel. Mirrors the existing *time.Time convention for Card.CompletedAt / Todo.CompletedAt. * wrappers: tighten propagation test assertions to exact values The propagation tests checked several mapped fields only for non-empty, so a swapped field mapping would still pass. - Assert exact URLs for the Message wrapper (BookmarkURL, BoostsURL, CommentsURL, SubscriptionURL) instead of just non-empty. - Assert nested Person CreatedAt/UpdatedAt against the expected RFC3339 formatted timestamps rather than non-empty strings, catching a CreatedAt/UpdatedAt source swap. - Assert BubbleUpAt against its exact expected value (and nil for the unscheduled entry) now that it is a *time.Time, replacing the IsZero checks the pointer change broke. - Drop the unused types import and the var _ = types.Date{} that kept it alive; the test no longer references the package. * scripts: scope wrapper-drift population walk to the wrapper instance The population walk counted any `x.Field = ...` selector write as a wrapper-field write, so writes to helper locals (a nested Person, a WebhookDelivery, a WebhookCopy) could be miscounted as populating the wrapper and mask genuine drift when the helper local shared a field name with the wrapper. Identify the wrapper instance up front (named result, the local bound to the wrapper composite literal, and the returned identifier) and only count selector writes whose base is that instance. Also extend directDecodePairs to the nested direct-decode structs reachable from the top-level raw-body wrappers (PreviewableAttachment, and the MyAssignmentAssignee/Bucket/Parent companions). These have generated counterparts but no *FromGenerated, so the parent check previously stopped at the parent field and would miss future generated-field drift inside them; listing them carries the tag-presence check into the nested structs. Rework the pair-extraction test so the unqualified-parameter exclusion is observable: the excluded function now returns a distinct type whose absence from the pair map is a meaningful assertion. Add soundness regressions proving a helper-local selector write is not attributed to the wrapper and that run() reports drift when a tag is only assigned via such a local. * wrappers: preserve flexible notification person ids on decode Exposing Notification.Creator/Participants (this branch's change) routes notification people through Person.ID, a plain int64. BC3 serializes some notification person ids as JSON strings, and those person objects often omit personable_type, so the existing normalizePersonIds pass (keyed on personable_type) skipped them and the decode failed with a string-into-int64 unmarshal error. Add normalizeNotificationsJSON for the /my/readings.json path: it runs the generic person normalization plus a creator/participants pass that coerces string ids by their structural position regardless of personable_type, reusing the shared id rule (numeric string to json.Number, non-numeric sentinel to 0 with the label preserved as system_label, overflow left for the decoder to reject). Add a decode test covering a string-id creator and participants without personable_type, including the sentinel case. * wrappers: preserve flexible gauge person ids; share embedded-person normalize Gauge and GaugeNeedle gained Creator *Person on this branch but decode raw via json.Unmarshal, so a string-valued creator.id (BC3 wire format) failed the whole decode against Person.ID (a plain int64) — the same regression just fixed for notifications. Generalize the notification-specific pass into a shared one: normalizeNotificationPeople becomes normalizeEmbeddedPersonIds and normalizeNotificationsJSON becomes normalizeEmbeddedPeopleJSON, coercing string ids on embedded creator/participants people regardless of personable_type. Route the gauge service decode sites through a decodeGaugePayload helper that normalizes first, alongside the existing notifications caller. Add a gauge decode test for a string creator id with and without personable_type. * wrappers: drop omitempty from required visible_to_clients/inherits_status These booleans are required (no omitempty) on generated.Todo, Comment, Message, and MessageBoard, but the wrapper fields added on this branch carried omitempty, which drops a legitimate false value when the public wrapper is marshaled. Match the generated shape so an explicit false stays on the wire — the same fix already applied to TimesheetEntry, Inbox, Forward, and ForwardReply. Todo.inherits_status already lacked omitempty, so only visible_to_clients changes there. (Gauge/GaugeNeedle keep omitempty on these tags because their generated counterparts also declare them with omitempty.) * wrappers: test required-bool marshaling; fix stale normalize name in comment Add TestRequiredBools_FalseMarshalsExplicitly covering the omitempty removal: Todo, Comment, Message, and MessageBoard must serialize visible_to_clients / inherits_status as an explicit false rather than omitting the key. The test fails if omitempty is reintroduced on either tag. Also fix a stale reference in the notification decode test comment (normalizeNotificationsJSON was renamed to normalizeEmbeddedPeopleJSON). * wrappers: clear golangci-lint findings in normalize.go The Lint CI job (golangci-lint on go/) flagged two issues the wrapper person-id normalization introduced: an else-after-return in coercePersonID (revive indent-error-flow) and the now-orphaned normalizeJSON (superseded by normalizeEmbeddedPeopleJSON, zero call sites). Lift the err decl out of the if-init and drop the else; remove the dead function and update the two comments that referenced it. golangci-lint reports 0 issues; go build/vet and go test ./pkg/basecamp/... pass. * scripts: enumerate complete direct-decode pair set Adds 9 (wrapper, generated) pairs to directDecodePairs so the field-level drift check covers every public wrapper struct that is populated by raw json.Unmarshal rather than through a *FromGenerated function: Account -> Account AccountLogo -> AccountLogo (nested in Account.logo) AccountLimits -> AccountLimits (nested in Account.limits) AccountSettings -> AccountSettings (nested in Account.settings) AccountSubscription -> AccountSubscription (nested in Account.subscription) Preferences -> Preferences OutOfOffice -> OutOfOffice OutOfOfficePerson -> OutOfOfficePerson (nested in OutOfOffice.person) MyAssignmentsResult -> GetMyAssignmentsResponseContent Pair count moves from 55 to 64, generated fields verified from 859 to 903. Records the derivation recipe in the directDecodePairs declaration so the next contributor can rederive the set when new endpoints land: grep raw-decode call sites, identify the ones that target a hand- written wrapper (rather than a generated.X local routed through a *FromGenerated function), check the generated package for a matching counterpart, and add it plus any nested wrapper structs decoded with their parent. Webhook-flavored parallel types and non-spec endpoints (launchpad authorization, SDK provenance, error envelopes) are explicitly out of scope and listed as such. Adds TestRun_DirectDecodeRenamedPair driving run() end-to-end on the renamed-pair shape (MyAssignmentsResult vs GetMyAssignmentsResponseContent) so the runtime behavior of the broader-named pair entries is pinned. * wrappers: route subscription subscribers through personFromGenerated subscriptionFromGenerated still inlined a 6-field shallow Person copy when building Subscription.Subscribers, so the BC5 Tagline plus the Bio / Location / Title / PersonableType / AttachableSGID / access flags / Company / timestamps that the rest of the wrapper layer standardized on personFromGenerated never reached subscription responses. Routes the conversion through personFromGenerated to match every other nested-Person context, and extends TestPerson_NewFields_NestedPropagation with a subscription.Subscribers subtest so the propagation is pinned the same way it is for recording/comment/message/todo/schedule_entry/card. * docs(brief): align omit-marker syntax + BubbleUpAt pointer choice Two edits keep the brief consistent with the shipped wrapper API and drift-check implementation: * The omit-marker bullet documented a free-form 'intentionally not surfaced because <reason>' phrasing that the drift check does not recognize. Rewritten to spell out the only syntax the check honors - 'intentionally-omitted: <tag> - <reason>' with <tag> the JSON tag on the generated struct - and pointing readers at scripts/check- wrapper-drift's markerRe for the canonical regex. * The Notification.BubbleUpAt proposal still said time.Time and pointed at ReadAt/UnreadAt for the zero-value pattern. The shipped wrapper (commit 4ec355e) is *time.Time so an absent scheduled bubble-up omits the wire key cleanly instead of marshaling as 0001-01-01T00:00:00Z (Go's omitempty does not suppress zero-valued time.Time). Brief now documents the *time.Time choice, names the Card.CompletedAt / Todo.CompletedAt precedent, and tells consumers to nil-check rather than IsZero. * scripts: extend wrapper-drift map with inline-converted tier The directDecodePairs map already covered wrappers populated via raw json.Unmarshal (Notification, MyAssignment, ...). It did not cover a third category: wrappers with a generated counterpart and no *FromGenerated of their own that are populated inline via composite literal inside a parent *FromGenerated body or a service method. Codex named CampfireLineAttachment (built in campfireLineFromGenerated) and EventDetails (built in eventFromGenerated). A wider scan of go/pkg/basecamp/*.go intersected against go/pkg/generated/client.gen.go turned up 12 such pairs. If BC5 adds a field to one of these generated structs, the wrapper would silently drop the new data while the drift check would pass — exactly the regression the check exists to catch. Reorganize the map into three explicitly labeled tiers in one tag- presence check so future contributors see the coverage as a single surface: - Tier 1: *FromGenerated-backed (auto-discovered, both tag-presence and population checks) - Tier 2: direct-decode via json.Unmarshal (tag-presence only; the decoder populates from tags) - Tier 3: inline-converted via composite literal (tag-presence only; population enforced by review of the composite at the call site) Rewrite the leading comment block to document the tier model, restate the derivation recipe (re-scan all wrappers, intersect with generated, subtract tier-1 and design exclusions, classify what is left), and call out the out-of-scope list (webhook-flavored parallel types, non-spec envelopes, outgoing request wrappers with name-clash to generated). Extend the e2e test to cover a tier-3 pair end-to-end via TestRun_InlineConvertedPair: confirms the pair is walked via the injected map and the tag-presence check fires when the inline-converted wrapper drops a tag the generated struct still emits. Total pairs walked: 76 (was 64). Conformance-go: 68/0/0 unchanged. * scripts: extend wrapper-drift walker to population-check tier 3 Tier-3 wrappers (inline-converted via composite literal — 12 of the 30 directDecodePairs entries) previously got the tag-presence check but not the population check: the walker treated everything in directDecodePairs as 'no body to walk' and gave the tier a reviewer-enforced floor. A future generated field added with a matching wrapper tag but a missed composite-literal update would have passed drift while the SDK silently returned zero. Add a composite-literal collector that scans every non-test file in go/pkg/basecamp/ for literals of any tier3Wrappers type and feeds the result into the existing population check the same way *FromGenerated bodies feed for tier 1: - Bare and pointer literals (Wrapper{...}, &Wrapper{...}) — both forms contribute the literal's KeyValueExpr keys. - Local-bound writes (resp := Wrapper{...}; resp.X = ...) and selector-chain bindings (q.Schedule = &Wrapper{...}; q.Schedule.X = ...) — subsequent selector writes against any path the literal is bound to are attributed to the wrapper. The selector-chain binding is what makes the QuestionSchedule shape (where six fields are set in the literal and three more by conditional q.Schedule.X writes) pass the population check end-to-end. tier3Wrappers is a sentinel set listing the 12 tier-3 names (kept in sync with the tier-3 section of directDecodePairs). The population-check gate flips from 'skip if directDecode' to 'skip only if tier 2', so the existing tier-2 path (raw json.Unmarshal — tag presence is population) is unchanged. run() gains a tier3 parameter so tests can drive end-to-end fixtures without dragging in the production tier-3 set, mirroring the existing directDecode injection. Pair count stays 76; field-assignments-verified jumps from 757 to 818 (61 new tier-3 assignments — exactly the gap the prior reviewer-floor left open). * scripts: regression tests for tier-3 composite-literal walker Covers the two construction forms (pointer Field: &Wrapper{...} and bare Wrapper{...}) and the two binding shapes (bare local resp.X = ... and selector chain q.Schedule.X = ...) that the tier-3 corpus uses, plus the dropped-assignment teeth proof for each literal form. Unit tests for the new helpers (collectCompositeLiteralFields, exprToPath, pathPrefixAndField) cover identifier roots, deep selector chains, and non-identifier-rooted expressions (calls, index expressions) that the walker must safely ignore. * scripts: run wrapper-drift tests as part of the make target The go-check-wrapper-drift target only invoked `go run` on the checker, so `scripts/check-wrapper-drift/main_test.go` (incl. the new tier-3 walker regression tests) was never exercised by `make check` or CI's gate — the walker could silently regress while drift kept reporting clean. Run `go test ./scripts/check-wrapper-drift/` as the first step of the target so test failures surface before the drift check itself runs. * wrappers: extend required-bool serialization test to Inbox/Forward/ForwardReply/TimesheetEntry The PR dropped omitempty from non-omitempty bool response fields on these four wrappers alongside Todo/Comment/Message/MessageBoard, but the regression test only pinned the latter four. Symmetric coverage: add the four missing cases so a future accidental re-introduction of omitempty on any of these wrappers fails the test instead of silently dropping false on the wire again. * wrappers: delegate ProjectConstruction.Project to projectFromGenerated The inline `&Project{...}` literal in projectConstructionFromGenerated predated the audit and silently dropped StartDate, EndDate, ClientsEnabled, BookmarkURL, Dock, Bookmarked, ClientCompany, and Clientside — fields the helper already converts. Composite-literal population checking only runs for tier-3 wrappers, so a literal for a type that also has a tier-1 helper escaped the walker. Delegating to the helper closes the gap the same way nested Person sites were standardized on personFromGenerated; a scan for other inline wrapper literals outside their helpers found no other sites.
This was referenced Jul 22, 2026
jeremy
added a commit
to robzolkos/basecamp-sdk
that referenced
this pull request
Jul 22, 2026
…reate-bucket-scope-20260601145434 * origin/main: (28 commits) Upstream sync: repin provenance to bc3 ba105ba7 (2026-07-22), register post-train contracts, codify pin semantics (basecamp#390) ci: pin conformance uv sync to the matrix Python; parse the fan-in gate (basecamp#388) BC5 readiness: pairwise BC4↔BC5 + check-bc5-compat orchestrator + scheduled CI (basecamp#308) CI: fold conformance suites into their language test jobs (basecamp#387) Fix TypeScript SDK Content-Type on GET requests (basecamp#384) BC5 launch reconciliation: registry refresh for the shipped BC5 API train (basecamp#325) BC5 readiness: cross-language wire-replay decoders (basecamp#301) BC5 readiness: Go wrapper forward-compat audit + field-level drift check (basecamp#309) BC5 readiness: TypeScript live wire-capture canary (basecamp#294) BC5 readiness: forward-compat spec, briefs, parity lint (basecamp#293) deps(ts): bump brace-expansion from 5.0.2 to 5.0.7 in /typescript (basecamp#386) typescript: bump fast-uri, brace-expansion, js-yaml overrides for fresh advisories (basecamp#385) deps(actions): bump the actions group with 6 updates (basecamp#382) deps(ruby): bump the bundler-dependencies group in /ruby with 2 updates (basecamp#379) deps(kotlin): bump the gradle-dependencies group (basecamp#383) deps(go): bump github.com/oapi-codegen/oapi-codegen/v2 in /go (basecamp#381) deps(ts): bump the npm-dependencies group in /typescript with 2 updates (basecamp#380) deps(smithy): bump the smithy-gradle-dependencies group (basecamp#378) deps(go): bump github.com/oapi-codegen/runtime (basecamp#377) Todos Update/Edit/Replace: merge-safe writes across all 6 SDKs (basecamp#375) ... # Conflicts: # go/pkg/basecamp/api-provenance.json # go/pkg/basecamp/version.go # go/pkg/generated/client.gen.go # kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/BasecampConfig.kt # openapi.json # python/src/basecamp/_version.py # ruby/lib/basecamp/generated/metadata.json # ruby/lib/basecamp/generated/types.rb # ruby/lib/basecamp/version.rb # spec/api-provenance.json # spec/basecamp.smithy # swift/Sources/Basecamp/BasecampConfig.swift # typescript/src/client.ts # typescript/src/generated/metadata.ts # typescript/src/generated/openapi-stripped.json
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.
Summary
Foundation PR for BC5 release readiness. Additive across the board — no behavior changes, no breaking renames.
Notificationgainsbubble_up_url,bubble_up_atGetMyNotificationsOutputgainsbubble_ups,scheduled_bubble_upsPersongainstagline(alias ofbio)Todogainssteps(reuses existingCardStepshape)Todosetgainstodos_count,completed_loose_todos_count,todos_url,app_todos_urlmemoriesmember documented as observed-current behavior pending BC team's back-compat decisionspec/api-provenance.jsonrestructured for parallel branch tracking. Post-launch topology (BC5 launched 2026-05-27;fivemerged intomaster, BC4 moved to thefourbranch):.bc3tracks BC5master(pinned ate52453ff, 2026-07-17 — main's Todos Update/Edit/Replace: merge-safe writes across all 6 SDKs #375 sync baseline, kept through the rebase; still pre-train, so it claims no coverage of the BC5 API train resources) and.compatibility.bc3-fourkeeps the BC4 baseline (four@9d73959a). Existing tooling reads the same.bc3keys and works unchanged;sync-statusiterates.compatibilitykeys generically.make generateaggregator runs Smithy build, behavior model, URL routes, provenance sync, and per-language SDKs in dependency order.make check-bucket-flat-parityflags bucket-scoped GET list operations without flat counterparts. Initial allowlist seeded withwebhooks(legitimately per-project).make sync-statusreports drift for the active branch and the BC4 compatibility branch as two clearly-labeled blocks.spec/api-gaps/— 11 entries aligned with the BC3 parity plan's Phase 3 deliverables, plusCOORDINATION.mddescribing the SDK ↔ BC3 plan relationship and the absorption lifecycle.make validate-api-gapsvalidates frontmatter and required body sections (Ruby stdlib only, no gem deps).The api-gap registry is durable: each entry transitions through a documented lifecycle (
no-json-contract→addressed-in-bc3-pr-N→absorbed-in-sdk) with status changes flowing through git history. Living next tobasecamp.smithyinspec/makes the spec-ecosystem mental model explicit — entries describe expected future spec state and link back to Smithy structures on absorption.Commit structure
spec(bc5)— Smithy additions + provenance restructure + parity-lint allowlist seedbuild(make)— generate aggregator + parity lint + branch-aware sync-status + validator make-targetdocs(api-gaps)— API gap registry + COORDINATION.md + validator + CONTRIBUTING updates4-9. One commit per SDK (
ts,ruby,python,kotlin,swift,go) for regenerated artifactsCross-team contract decisions (called out in COORDINATION.md)
Two items the SDK canary will surface that need BC team resolution before release:
memories: []on BC5 — back-compat or accept-and-document.app_todoslists_urltypo — server fix, alias period, or SDK accommodation.Test plan
make smithy-check,make behavior-model-check,make provenance-check,make sync-spec-version-check,make sync-api-version-checkmake go-check-drift,make kt-check-drift,make py-check-drift,make auth-routable-checkmake ts-check,make rb-check,make py-check,make kt-check,make swift-check,make go-checkmake conformance-{go,kotlin,typescript,ruby,python}make check-bucket-flat-parity(1 bucket-scoped list op, allowlisted)make validate-api-gaps(11 entries + allowlist, clean)make sync-statusreports both.bc3(primary:master) and.compatibility.bc3-four(compat:four) blocks, clearly labeledbash scripts/validate-api-gaps.shpasses from a clean shell (env -i ...,ruby --disable-gems ...)make lint-actionsWhat this PR is NOT
This PR is the foundation. It does not include:
check-bc5-compatorchestrator (PR 4)