Skip to content

Fix unsafe partial-update serialization across all Go SDK Update methods#201

Merged
jeremy merged 8 commits intomainfrom
go-todos-update-sa
Mar 18, 2026
Merged

Fix unsafe partial-update serialization across all Go SDK Update methods#201
jeremy merged 8 commits intomainfrom
go-todos-update-sa

Conversation

@jeremy
Copy link
Copy Markdown
Member

@jeremy jeremy commented Mar 18, 2026

Summary

  • Fix 17 Update methods that unconditionally copied all request fields into generated request bodies, causing zero-valued fields to leak onto the wire and potentially clear existing data server-side
  • For value-type generated fields (types.Date, time.Time, nested structs) whose Go zero values serialize as non-empty JSON, bypass generated body types entirely via map[string]any + *WithBody variant (6 methods)
  • For string/slice/pointer fields where json:"omitempty" works correctly, add conditional guards before assignment (11 methods)

Breaking changes

  • AllDay changed from bool to *bool in CreateScheduleEntryRequest and UpdateScheduleEntryRequest to distinguish "not provided" from "set to false"
  • Hour and Minute changed from int to *int in QuestionSchedule to distinguish "not provided" from "set to 0" (midnight / top of hour)

Known limitation

With conditional guards, the SDK cannot express "clear to empty/zero" partial updates for fields modeled as plain strings, slices, or zero-valued scalars (e.g., sending "assignee_ids": [] to unassign everyone). This is now a documented product limitation rather than an accidental serialization leak. A future PR can address this with explicit ClearX fields or pointer-to-slice patterns.

Test plan

  • go test ./pkg/basecamp/... — all existing tests pass
  • go vet ./... — clean
  • New wire-level tests capture actual HTTP request bodies and assert unset fields are absent:
    • TestTodosService_UpdatePartial — due_on, starts_on, description, assignee_ids, completion_subscriber_ids, notify
    • TestSchedulesService_UpdateEntryPartial — starts_at, ends_at, all_day, notify, participant_ids
    • TestSchedulesService_UpdateEntryAllDay — *bool false IS present when explicitly set
    • TestSchedulesService_CreateEntryPartial — notify absent when not requested
    • TestCardsService_UpdatePartial — due_on, content, assignee_ids
    • TestCardStepsService_UpdatePartial — due_on, assignees
    • TestProjectsService_UpdatePartial — schedule_attributes, description, admissions
    • TestProjectsService_UpdateEmptyScheduleAttributes — empty nested struct does not leak as {}
    • TestCheckinsService_UpdateQuestionPartial — schedule, paused
    • TestCheckinsService_UpdateQuestionPartialSchedule — partial schedule sends only provided fields
    • TestCheckinsService_UpdateQuestionEmptySchedule — empty nested struct does not leak as {}

Copilot AI review requested due to automatic review settings March 18, 2026 20:25
@github-actions github-actions bot added go bug Something isn't working labels Mar 18, 2026
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue 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/checkins.go">

<violation number="1" location="go/pkg/basecamp/checkins.go:849">
P2: `Hour` and `Minute` zero-value guards silently drop valid schedule times. `Hour=0` (midnight) and `Minute=0` (top of hour) are meaningful values, but the `!= 0` checks treat them as "not provided." These fields should use `*int` in `QuestionSchedule` (like `WeekInstance`/`WeekInterval`/`MonthInterval` already do) so the map builder can distinguish "not set" from "set to zero."</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes unsafe partial-update behavior in the Go SDK where zero-valued fields could be serialized into Update request bodies and inadvertently clear server-side data. It introduces guarded field assignment (or map-based bodies where needed) and adds service-level tests that assert omitted fields stay off the wire; it also makes AllDay a *bool for schedule entry create/update requests (breaking change).

Changes:

  • Prevent zero-value leakage in multiple Update* methods by conditionally setting fields and, for problematic value-types, switching to map[string]any + *WithBodyWithResponse.
  • Add marshalBody helper to consistently JSON-encode map bodies for generated client *WithBodyWithResponse methods.
  • Add wire-level httptest-based tests verifying partial update bodies omit unset fields; update schedule entry requests to use *bool for AllDay.

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 21 out of 21 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
go/pkg/basecamp/helpers.go Adds marshalBody helper for map-based JSON request bodies.
go/pkg/basecamp/todos.go Updates Todo partial update serialization to avoid leaking date/boolean zero values.
go/pkg/basecamp/todos_test.go Adds wire-level tests asserting omitted fields stay absent in Update bodies.
go/pkg/basecamp/schedules.go Switches schedule entry updates to map-based body; changes AllDay to *bool; guards notify.
go/pkg/basecamp/schedules_test.go Adds service-level tests for partial update bodies and explicit all_day=false.
go/pkg/basecamp/projects.go Uses map body to avoid leaking empty nested schedule_attributes structs.
go/pkg/basecamp/projects_test.go Adds tests ensuring partial updates omit schedule_attributes and other absent fields.
go/pkg/basecamp/checkins.go Uses map body for UpdateQuestion; adds schedule-to-map conversion to avoid nested zero-value leaks.
go/pkg/basecamp/checkins_test.go Adds wire-level tests for partial question updates and schedule omission behavior.
go/pkg/basecamp/cards.go Uses map body for card/step updates to avoid date zero-value leaks; adds guarded assignments elsewhere.
go/pkg/basecamp/cards_test.go Adds wire-level tests asserting partial card/step updates omit absent fields.
go/pkg/basecamp/webhooks.go Adds guards to omit absent webhook update fields.
go/pkg/basecamp/vaults.go Adds guards to omit absent vault/document/upload update fields.
go/pkg/basecamp/todolists.go Adds guards to omit absent todolist update fields.
go/pkg/basecamp/todolist_groups.go Adds guards to omit absent todolist-group update fields.
go/pkg/basecamp/timesheet.go Adds guards to omit absent timesheet entry update fields.
go/pkg/basecamp/templates.go Guards template description update while keeping required name behavior.
go/pkg/basecamp/subscriptions.go Adds guards to omit empty subscription/unsubscription arrays.
go/pkg/basecamp/messages.go Adds guards to omit absent message update fields.
go/pkg/basecamp/message_types.go Adds guards to omit absent message type update fields.
go/pkg/basecamp/lineup.go Adds guards to omit absent lineup marker update fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6824c24692

ℹ️ 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 jeremy requested a review from Copilot March 18, 2026 20:47
@github-actions github-actions bot added the breaking Breaking change to public API label Mar 18, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 18, 2026

⚠️ Potential breaking changes detected:

  • QuestionSchedule.Hour and QuestionSchedule.Minute fields changed from int to *int, introducing potential nil values where none existed before.
  • CreateScheduleEntryRequest.AllDay and UpdateScheduleEntryRequest.AllDay fields changed from bool to *bool, introducing potential nil values where none existed before.

Review carefully before merging. Consider a major version bump.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes unsafe partial-update serialization in the Go SDK by ensuring Update methods only send fields explicitly provided by callers, preventing zero values from unintentionally clearing server-side data. It also introduces map-based request bodies for endpoints where generated value-type fields (dates/times/nested structs) would otherwise serialize non-empty zero values.

Changes:

  • Update 17 Go SDK Update methods to omit unset fields (guards for simple fields; map[string]any + *WithBodyWithResponse for value-type/nested fields).
  • Add marshalBody helper to safely JSON-encode map-based bodies for generated *WithBodyWithResponse methods.
  • Add wire-level httptest-based service tests asserting request bodies omit unset fields (and that empty nested structs don’t leak as {}).

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 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
go/pkg/basecamp/webhooks.go Guard UpdateWebhook body assignments to omit unset fields.
go/pkg/basecamp/vaults.go Guard update bodies for vaults/documents/uploads to avoid zero-value leaks.
go/pkg/basecamp/todos.go Switch UpdateTodo to map-based body + explicit key insertion and marshalBody.
go/pkg/basecamp/todos_test.go Add service-level wire tests for Todos.Update partial serialization.
go/pkg/basecamp/todolists.go Guard UpdateTodolistOrGroup body assignments for partial updates.
go/pkg/basecamp/todolist_groups.go Guard group update body (polymorphic endpoint) to omit unset fields.
go/pkg/basecamp/timesheet.go Guard timesheet update fields so unset values are omitted.
go/pkg/basecamp/test_helpers_test.go Add shared decodeRequestBody helper (and intPtr) for wire tests.
go/pkg/basecamp/templates.go Adjust template update serialization (always includes required name; guards description).
go/pkg/basecamp/subscriptions.go Guard subscription update slices to omit unset fields.
go/pkg/basecamp/schedules.go Make AllDay pointer-based; use map-based body for UpdateEntry to avoid time zero-value leaks; guard CreateEntry notify.
go/pkg/basecamp/schedules_test.go Add wire tests for schedules partial updates and explicit all_day=false.
go/pkg/basecamp/projects.go Use map-based body to avoid leaking empty nested schedule_attributes.
go/pkg/basecamp/projects_test.go Add wire tests for Projects.Update partial updates and empty nested struct omission.
go/pkg/basecamp/messages.go Guard message update fields to omit unset values.
go/pkg/basecamp/message_types.go Guard message type update fields to omit unset values.
go/pkg/basecamp/lineup.go Guard lineup marker update fields to omit unset values.
go/pkg/basecamp/helpers.go Add marshalBody helper and document rationale/usage sites.
go/pkg/basecamp/checkins.go Use map-based body for UpdateQuestion; convert schedule to map to avoid nested zero-value leaks; make Hour/Minute pointers.
go/pkg/basecamp/checkins_test.go Update schedule tests for pointer hour/minute; add wire tests for UpdateQuestion partial schedule behavior.
go/pkg/basecamp/cards.go Use map-based bodies for Updates that include date fields; guard other update structs.
go/pkg/basecamp/cards_test.go Add wire tests for Cards.Update and CardSteps.Update partial serialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jeremy jeremy requested a review from Copilot March 18, 2026 20:54
jeremy added 4 commits March 18, 2026 13:59
Update methods were copying all request fields unconditionally into
generated request bodies, causing zero-valued fields to be sent on the
wire. A title-only todo update would send "notify": false,
"assignee_ids": null, "due_on": null, etc., potentially clearing
existing data server-side.

Two distinct fix strategies based on the generated field type:

For string, slice, and pointer fields (where json omitempty works):
guard each field with a zero-value check before assigning to the
generated body struct. Applied to 11 Update methods.

For value-type fields (types.Date, time.Time, nested structs) whose
Go zero values serialize as non-empty JSON ("due_on":null,
"starts_at":"0001-01-01T00:00:00Z", "schedule_attributes":{}):
build map[string]any and use the generated client's *WithBody variant
to bypass broken zero-value serialization. Applied to 6 methods:
Todos.Update, Schedules.UpdateEntry, Cards.Update, CardSteps.Update,
Projects.Update, Checkins.UpdateQuestion.

BREAKING: AllDay changed from bool to *bool in CreateScheduleEntryRequest
and UpdateScheduleEntryRequest to distinguish "not provided" from
"set to false."
HTTP-level tests that capture the actual request body sent to the server
and assert that unset fields are absent from the JSON. Each test sends a
minimal update and verifies zero-valued fields do not leak.

Coverage:
- TodosService.Update: due_on, starts_on, description, assignee_ids,
  completion_subscriber_ids, notify all absent on title-only update
- SchedulesService.UpdateEntry: starts_at, ends_at, all_day, notify,
  participant_ids all absent on summary-only update
- SchedulesService.UpdateEntry AllDay: *bool false IS sent when explicit
- SchedulesService.CreateEntry: notify absent when not requested
- CardsService.Update: due_on, content, assignee_ids absent
- CardStepsService.Update: due_on, assignees absent
- ProjectsService.Update: schedule_attributes, description, admissions
  absent; empty ScheduleAttributes{} does not leak as {}
- CheckinsService.UpdateQuestion: schedule, paused absent on title-only;
  partial schedule (end_date only) does not leak frequency/days/hour/minute;
  empty Schedule{} does not leak as {}
- Change QuestionSchedule.Hour and .Minute from int to *int so that
  zero values (midnight, top of hour) are distinguishable from "not
  provided" (breaking change, same pattern as AllDay *bool)
- Extract decodeRequestBody test helper to centralize request body
  parsing with proper error handling via t.Fatal
- Replace all manual io.ReadAll + json.Decode with decodeRequestBody
  across 5 test files
The version number is determined at release time, not at PR time.
@jeremy jeremy force-pushed the go-todos-update-sa branch from a38f3ae to c36603d Compare March 18, 2026 20:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes unsafe partial-update request serialization in the Go SDK by ensuring Update methods only send fields the caller explicitly provided, avoiding accidental “zero-value” overwrites on the server (notably for value-type fields like types.Date, time.Time, and nested structs).

Changes:

  • Updated multiple Go service Update* methods to conditionally include fields (or build JSON bodies via map[string]any + *WithBodyWithResponse) to prevent zero values leaking onto the wire.
  • Introduced marshalBody(map[string]any) helper to support safe partial-update JSON encoding for problematic value-type fields.
  • Added new wire-level tests using httptest to assert omitted fields are not present in partial update request bodies, and updated request types for breaking-change pointer fields (AllDay, Hour, Minute).

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 22 out of 22 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
go/pkg/basecamp/webhooks.go Guarded field assignments in webhook update body to omit unset fields.
go/pkg/basecamp/vaults.go Guarded field assignments in vault/document/upload update bodies to omit unset fields.
go/pkg/basecamp/todos.go Switched todo update to map-based JSON body + marshalBody to avoid types.Date{} leaks.
go/pkg/basecamp/todos_test.go Added wire-level tests asserting partial update request bodies omit unset fields.
go/pkg/basecamp/todolists.go Guarded todolist/group update body fields to omit unset values.
go/pkg/basecamp/todolist_groups.go Guarded todolist group update body fields to omit unset values.
go/pkg/basecamp/timesheet.go Guarded timesheet update body fields to omit unset values.
go/pkg/basecamp/test_helpers_test.go Added shared decodeRequestBody helper (and intPtr) for wire-level tests.
go/pkg/basecamp/templates.go Updated template update serialization to omit empty description.
go/pkg/basecamp/subscriptions.go Guarded subscription update fields to omit empty slices.
go/pkg/basecamp/schedules.go Breaking change: AllDay is now *bool; update entry uses map-based body to avoid time.Time{} leaks.
go/pkg/basecamp/schedules_test.go Updated schedule tests and added wire-level tests for partial update and explicit all_day=false.
go/pkg/basecamp/projects.go Switched project update to map-based JSON body to prevent empty nested schedule_attributes: {} leaks.
go/pkg/basecamp/projects_test.go Added wire-level tests for partial update and empty nested schedule attributes omission.
go/pkg/basecamp/messages.go Guarded message update body fields to omit empty strings.
go/pkg/basecamp/message_types.go Guarded message type update body fields to omit empty strings.
go/pkg/basecamp/lineup.go Guarded lineup marker update body fields to omit empty strings.
go/pkg/basecamp/helpers.go Added marshalBody helper and documented why some updates must bypass typed generated bodies.
go/pkg/basecamp/checkins.go Breaking change: Hour/Minute now *int; update question uses map-based body; added schedule-to-map helper.
go/pkg/basecamp/checkins_test.go Updated tests for pointer schedule fields and added wire-level tests for partial update behavior.
go/pkg/basecamp/cards.go Switched card and card-step updates to map-based JSON bodies to avoid types.Date{} leaks; guarded column updates.
go/pkg/basecamp/cards_test.go Added wire-level tests asserting partial update request bodies omit unset fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jeremy jeremy requested a review from Copilot March 18, 2026 21:08
CreateQuestion used the generated QuestionSchedule struct where
Hour/Minute are int32 with omitempty, dropping zero values (midnight,
top of hour). Switch to map + WithBody like UpdateQuestion.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Go SDK’s partial-update behavior by preventing zero-value fields (especially value-type fields like types.Date, time.Time, and nested structs) from being serialized into Update request bodies and unintentionally clearing server-side data. It does this by conditionally populating generated request-body structs where omitempty is sufficient, and by bypassing generated body types entirely (via map[string]any + *WithBodyWithResponse) where omitempty cannot safely omit zero values.

Changes:

  • Updated multiple Go SDK Update methods to avoid sending unset/zero-value fields (guards or map-based bodies, depending on field types).
  • Added marshalBody helper to safely encode map-based JSON payloads for generated client *WithBodyWithResponse methods.
  • Added wire-level tests (httptest servers) asserting that partial-update request bodies omit unset fields; updated request types where needed to preserve “unset vs explicit zero/false” semantics.

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 22 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
go/pkg/basecamp/helpers.go Adds marshalBody helper to support map-based JSON bodies for safe partial updates.
go/pkg/basecamp/todos.go Switches Update to map-based body to avoid types.Date{} leaking as JSON.
go/pkg/basecamp/todos_test.go Adds wire-level tests asserting omitted fields in partial updates.
go/pkg/basecamp/schedules.go Updates schedule entry create/update serialization; UpdateEntry uses map-based body; AllDay becomes *bool.
go/pkg/basecamp/schedules_test.go Adds wire-level tests for partial update/create semantics (including explicit all_day=false).
go/pkg/basecamp/projects.go Uses map-based Update body to avoid leaking empty nested schedule_attributes.
go/pkg/basecamp/projects_test.go Adds wire-level tests for project Update partial/empty nested struct behavior.
go/pkg/basecamp/cards.go Uses map-based Update bodies for card and step updates to avoid types.Date{} leaks.
go/pkg/basecamp/cards_test.go Adds wire-level tests verifying partial Update request bodies for cards/steps.
go/pkg/basecamp/checkins.go Updates question schedule types (Hour/Minute*int) and uses map-based UpdateQuestion body to prevent empty schedule leaks.
go/pkg/basecamp/checkins_test.go Updates schedule assertions for pointer hour/minute and adds wire-level UpdateQuestion tests.
go/pkg/basecamp/test_helpers_test.go Adds decodeRequestBody (fatal-on-error) and intPtr helpers to support new tests.
go/pkg/basecamp/webhooks.go Adds conditional guards for Update body fields (avoid sending empty values).
go/pkg/basecamp/vaults.go Adds conditional guards for Update bodies (vaults/documents/uploads).
go/pkg/basecamp/todolists.go Adds conditional guards for Update body fields.
go/pkg/basecamp/todolist_groups.go Adds conditional guards for Update body fields.
go/pkg/basecamp/timesheet.go Adds conditional guards for UpdateTimesheetEntry body fields.
go/pkg/basecamp/templates.go Adjusts UpdateTemplate body assignment to omit empty description while keeping required name.
go/pkg/basecamp/subscriptions.go Adds conditional guards for subscription/unsubscription arrays in Update body.
go/pkg/basecamp/messages.go Adds conditional guards for UpdateMessage body fields.
go/pkg/basecamp/message_types.go Adds conditional guards for UpdateMessageType body fields.
go/pkg/basecamp/lineup.go Adds conditional guards for UpdateLineupMarker body fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 330537644d

ℹ️ 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".

No callers remain after switching both CreateQuestion and
UpdateQuestion to the map-based body path.
@jeremy jeremy requested a review from Copilot March 18, 2026 21:16
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59e674426d

ℹ️ 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".

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Go SDK’s partial-update behavior by preventing zero-valued fields from being serialized into Update* request bodies (which could unintentionally clear server-side state). It does so by selectively switching some updates to map-based JSON bodies and, elsewhere, adding conditional guards so only explicitly provided fields are sent.

Changes:

  • Fix partial-update serialization across multiple services by guarding optional assignments and, where needed, using map[string]any + *WithBodyWithResponse to avoid value-type zero-value leaks.
  • Introduce breaking API tweaks for “optional-but-false/zero” fields (AllDay to *bool; Hour/Minute to *int) so callers can explicitly send false/0.
  • Add wire-level tests that assert omitted fields are truly absent from outbound HTTP JSON bodies.

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 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
go/pkg/basecamp/helpers.go Adds marshalBody helper for map-based JSON encoding used to avoid zero-value leaks.
go/pkg/basecamp/todos.go Switches TodosService.Update to map-based body + WithBodyWithResponse to omit unset fields safely.
go/pkg/basecamp/todos_test.go Adds service-level wire tests verifying partial-update omission behavior for todos.
go/pkg/basecamp/schedules.go Changes AllDay to *bool; updates schedule entry create/update serialization; uses map-based body for update.
go/pkg/basecamp/schedules_test.go Adds wire tests to assert partial update omission and explicit all_day=false behavior.
go/pkg/basecamp/projects.go Uses map-based body for ProjectsService.Update to avoid leaking empty nested schedule attributes.
go/pkg/basecamp/projects_test.go Adds wire tests covering partial updates and empty schedule_attributes omission.
go/pkg/basecamp/cards.go Uses map-based body for card and card-step updates where value-type fields could leak.
go/pkg/basecamp/cards_test.go Adds wire tests validating partial-update omission for cards and card steps.
go/pkg/basecamp/checkins.go Changes schedule hour/minute to pointers; updates question create/update to map-based bodies to avoid schedule leaks.
go/pkg/basecamp/checkins_test.go Adds wire tests for partial question updates and schedule field omission behavior.
go/pkg/basecamp/test_helpers_test.go Adds shared test helpers (decodeRequestBody, intPtr) for new wire-level tests.
go/pkg/basecamp/webhooks.go Adds guards in webhook update to avoid sending empty/zero values by default.
go/pkg/basecamp/vaults.go Adds guards in vault/document/upload update methods to omit unset optional fields.
go/pkg/basecamp/todolists.go Adds guards in todolist update to only send provided fields.
go/pkg/basecamp/todolist_groups.go Adds guards in todolist-group update to only send provided fields.
go/pkg/basecamp/timesheet.go Adds guards in timesheet entry update to omit unset optional fields.
go/pkg/basecamp/templates.go Adjusts template update serialization to avoid sending empty description unintentionally.
go/pkg/basecamp/subscriptions.go Adds guards in subscription update to omit empty subscription arrays by default.
go/pkg/basecamp/messages.go Adds guards in message update to omit unset optional fields.
go/pkg/basecamp/message_types.go Adds guards in message type update to omit unset optional fields.
go/pkg/basecamp/lineup.go Adds guards in lineup marker update to omit unset optional fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jeremy jeremy requested a review from Copilot March 18, 2026 22:13
@jeremy jeremy merged commit 4947598 into main Mar 18, 2026
45 of 46 checks passed
@jeremy jeremy deleted the go-todos-update-sa branch March 18, 2026 22:19
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Go SDK’s partial-update behavior by ensuring Update (and a couple of closely related mutation) methods only serialize fields that callers explicitly set, preventing unintended “zero value” overwrites on the server.

Changes:

  • Switch selected Update payloads to map[string]any + marshalBody + *WithBodyWithResponse to avoid leaking non-empty Go zero values for value-type fields (e.g., types.Date, time.Time, nested structs).
  • Add conditional guards (!= "", len(...) > 0, != nil) for string/slice/pointer fields where omitempty semantics are sufficient.
  • Add wire-level tests that assert omitted fields are truly absent from outgoing JSON bodies; introduce shared test helpers for decoding request bodies.

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 22 out of 22 changed files in this pull request and generated no comments.

Show a summary per file
File Description
go/pkg/basecamp/helpers.go Adds marshalBody helper to safely build JSON bodies for partial updates without zero-value leakage.
go/pkg/basecamp/webhooks.go Builds update body conditionally to avoid sending empty fields.
go/pkg/basecamp/vaults.go Guards update fields (vaults/documents/uploads) so unset fields are omitted.
go/pkg/basecamp/todos.go Uses map-based body + JSON marshal to prevent types.Date zero-value serialization in partial updates.
go/pkg/basecamp/todos_test.go Adds service-level tests asserting partial update bodies omit unset fields.
go/pkg/basecamp/todolists.go Guards update fields so only provided values are sent.
go/pkg/basecamp/todolist_groups.go Guards group update fields for partial-update semantics.
go/pkg/basecamp/timesheet.go Guards update fields so empty values don’t get serialized unintentionally.
go/pkg/basecamp/templates.go Avoids serializing empty description on update while keeping name required.
go/pkg/basecamp/subscriptions.go Avoids serializing empty subscription arrays on update unless provided.
go/pkg/basecamp/schedules.go Introduces *bool for AllDay and uses map-based update body to prevent time.Time zero-value leaks; conditionally sets notify.
go/pkg/basecamp/schedules_test.go Adds wire-level schedule tests for partial updates and explicit all_day=false.
go/pkg/basecamp/projects.go Uses map-based update body to avoid leaking empty nested schedule_attributes.
go/pkg/basecamp/projects_test.go Adds wire-level tests ensuring nested empty schedule attributes aren’t sent.
go/pkg/basecamp/messages.go Guards update fields (subject/content/status) to omit unset values.
go/pkg/basecamp/message_types.go Guards update fields (name/icon) to omit unset values.
go/pkg/basecamp/lineup.go Guards marker update fields (name/date) to omit unset values.
go/pkg/basecamp/checkins.go Makes Hour/Minute pointer-backed; uses map-based bodies for question create/update to avoid nested zero-value leaks.
go/pkg/basecamp/checkins_test.go Updates schedule assertions for pointer hour/minute; adds wire-level tests for partial question updates/schedule omission.
go/pkg/basecamp/cards.go Uses map-based update bodies for card/card-step updates to avoid types.Date zero-value leaks; guards other update fields.
go/pkg/basecamp/cards_test.go Adds wire-level tests asserting partial card/card-step updates omit unset fields.
go/pkg/basecamp/test_helpers_test.go Adds shared decodeRequestBody and intPtr helpers for the new wire-level tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change to public API bug Something isn't working go

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants