Go: five wrapper timestamps could not represent absence (#620)

go/pkg/basecamp/checkins.go

@@ -144,10 +144,13 @@ type QuestionAnswer struct {
 
 // QuestionReminder represents a pending check-in reminder for the current user.
 type QuestionReminder struct {
-	GroupOn    string    `json:"group_on,omitempty"`
-	Question   Question  `json:"question"`
-	RemindAt   time.Time `json:"remind_at"`
-	ReminderID *int64    `json:"reminder_id,omitempty"`
+	GroupOn  string   `json:"group_on,omitempty"`
+	Question Question `json:"question"`
+	// RemindAt is optional — nil means the API did not send it. A value type
+	// here would fabricate 0001-01-01T00:00:00Z for an absent timestamp, which
+	// no consumer can tell from a real one.
+	RemindAt   *time.Time `json:"remind_at,omitempty"`
+	ReminderID *int64     `json:"reminder_id,omitempty"`
 }
 
 // QuestionNotificationSettings represents the current user's notification
@@ -1239,7 +1242,7 @@ func questionAnswerFromGenerated(ga generated.QuestionAnswer) QuestionAnswer {
 // questionReminderFromGenerated converts a generated QuestionReminder to our clean type.
 func questionReminderFromGenerated(gr generated.QuestionReminder) QuestionReminder {
 	r := QuestionReminder{
-		RemindAt:   deref(gr.RemindAt),
+		RemindAt:   gr.RemindAt,
 		ReminderID: gr.ReminderId,
 	}
 

go/pkg/basecamp/checkins_test.go

@@ -1361,7 +1361,9 @@ func TestCheckinsService_ListQuestionReminders(t *testing.T) {
 	if r1.GroupOn != "2022-10-28" {
 		t.Errorf("expected GroupOn '2022-10-28', got %q", r1.GroupOn)
 	}
-	if r1.RemindAt.IsZero() {
+	if r1.RemindAt == nil {
+		t.Error("expected RemindAt to be non-nil")
+	} else if r1.RemindAt.IsZero() {
 		t.Error("expected RemindAt to be non-zero")
 	}
 	if r1.ReminderID == nil || *r1.ReminderID != 123 {

go/pkg/basecamp/client_approvals.go

@@ -62,21 +62,24 @@ type ClientApproval struct {
 
 // ClientApprovalResponse represents a response to a client approval.
 type ClientApprovalResponse struct {
-	ID               int64     `json:"id"`
-	Status           string    `json:"status"`
-	VisibleToClients bool      `json:"visible_to_clients"`
-	CreatedAt        time.Time `json:"created_at"`
-	UpdatedAt        time.Time `json:"updated_at"`
-	Title            string    `json:"title"`
-	InheritsStatus   bool      `json:"inherits_status"`
-	Type             string    `json:"type"`
-	AppURL           string    `json:"app_url"`
-	BookmarkURL      string    `json:"bookmark_url"`
-	Parent           *Parent   `json:"parent,omitempty"`
-	Bucket           *Bucket   `json:"bucket,omitempty"`
-	Creator          *Person   `json:"creator,omitempty"`
-	Content          string    `json:"content"`
-	Approved         bool      `json:"approved"`
+	ID               int64  `json:"id"`
+	Status           string `json:"status"`
+	VisibleToClients bool   `json:"visible_to_clients"`
+	// CreatedAt and UpdatedAt are optional — nil means the API did not send
+	// them. Value types here would fabricate 0001-01-01T00:00:00Z for an absent
+	// timestamp, which no consumer can tell from a real one.
+	CreatedAt      *time.Time `json:"created_at,omitempty"`
+	UpdatedAt      *time.Time `json:"updated_at,omitempty"`
+	Title          string     `json:"title"`
+	InheritsStatus bool       `json:"inherits_status"`
+	Type           string     `json:"type"`
+	AppURL         string     `json:"app_url"`
+	BookmarkURL    string     `json:"bookmark_url"`
+	Parent         *Parent    `json:"parent,omitempty"`
+	Bucket         *Bucket    `json:"bucket,omitempty"`
+	Creator        *Person    `json:"creator,omitempty"`
+	Content        string     `json:"content"`
+	Approved       bool       `json:"approved"`
 }
 
 // ClientApprovalListResult contains the results from listing client approvals.
@@ -290,8 +293,8 @@ func clientApprovalFromGenerated(ga generated.ClientApproval) ClientApproval {
 			resp := ClientApprovalResponse{
 				Status:           deref(gr.Status),
 				VisibleToClients: deref(gr.VisibleToClients),
-				CreatedAt:        deref(gr.CreatedAt),
-				UpdatedAt:        deref(gr.UpdatedAt),
+				CreatedAt:        gr.CreatedAt,
+				UpdatedAt:        gr.UpdatedAt,
 				Title:            deref(gr.Title),
 				InheritsStatus:   deref(gr.InheritsStatus),
 				Type:             deref(gr.Type),

go/pkg/basecamp/optional_timestamps_test.go

@@ -78,6 +78,34 @@ func timestampCarriers() []timestampCarrier {
 			present: []byte(`{"id":1,"created_at":"2026-07-31T12:34:56Z","updated_at":"2026-07-30T01:02:03Z",` +
 				`"content":null,"description":null}`),
 		},
+		// #620: the five holdouts of the same class. Each was typed value
+		// time.Time with a bare `json:"..."` tag — no omitempty to even hint the
+		// field was optional — while its generated counterpart was *time.Time.
+		{
+			name:    "TimelineEvent",
+			decode:  unmarshalInto[TimelineEvent],
+			keys:    []string{"created_at"},
+			present: []byte(`{"id":1,"created_at":"2026-07-31T12:34:56Z","kind":"todo_created"}`),
+		},
+		{
+			name:    "WebhookDelivery",
+			decode:  unmarshalInto[WebhookDelivery],
+			keys:    []string{"created_at"},
+			present: []byte(`{"id":1,"created_at":"2026-07-31T12:34:56Z"}`),
+		},
+		{
+			name:    "QuestionReminder",
+			decode:  unmarshalInto[QuestionReminder],
+			keys:    []string{"remind_at"},
+			present: []byte(`{"remind_at":"2026-07-31T12:34:56Z"}`),
+		},
+		{
+			name:   "ClientApprovalResponse",
+			decode: unmarshalInto[ClientApprovalResponse],
+			keys:   []string{"created_at", "updated_at"},
+			present: []byte(`{"id":1,"created_at":"2026-07-31T12:34:56Z",` +
+				`"updated_at":"2026-07-30T01:02:03Z","approved":true}`),
+		},
 	}
 }
 
@@ -209,9 +237,12 @@ func TestNilOptionalTimestampPanicsOnValueReceiverCall(t *testing.T) {
 // time.Time fields carrying `,omitempty`. A value-typed time.Time with NO
 // omitempty whose generated counterpart is a pointer — the shape
 // SearchResult.CreatedAt had — is NOT detectable from this package's source
-// alone; catching that class needs the (wrapper, generated) pairing that
-// scripts/check-wrapper-drift already computes for tag names but not for
-// types. That gap is reported on the PR, not closed here.
+// alone; catching that class needs a (wrapper, generated) pairing.
+//
+// That gap is now closed by TestNoWrapperTimestampNarrowerThanGenerated below,
+// which reads both sides and pairs them by struct name + json key (#620). This
+// guard stays: it catches an inert `,omitempty` on a wrapper struct that has no
+// generated counterpart at all, which the cross-reference cannot see.
 func TestNoValueTypedOptionalTimestamps(t *testing.T) {
 	entries, err := os.ReadDir(".")
 	if err != nil {
@@ -282,6 +313,160 @@ func TestNoValueTypedOptionalTimestamps(t *testing.T) {
 	}
 }
 
+// timestampField is one (struct, json key) timestamp declaration, recorded from
+// either the wrapper surface or the generated client.
+type timestampField struct {
+	pointer bool
+	field   string
+	file    string
+}
+
+// collectTimestampFields keys every time.Time / *time.Time struct field by
+// (struct name, json key). The json key is the pairing axis rather than the Go
+// field name: the wrapper renames fields (Id → ID) but the wire key is the
+// thing both sides must agree about.
+func collectTimestampFields(t *testing.T, files map[string]*ast.File) map[[2]string]timestampField {
+	t.Helper()
+	out := map[[2]string]timestampField{}
+	for path, file := range files {
+		ast.Inspect(file, func(n ast.Node) bool {
+			ts, ok := n.(*ast.TypeSpec)
+			if !ok {
+				return true
+			}
+			st, ok := ts.Type.(*ast.StructType)
+			if !ok {
+				return true
+			}
+			for _, f := range st.Fields.List {
+				if f.Tag == nil || len(f.Names) == 0 {
+					continue
+				}
+				value := isValueTime(f.Type)
+				star, isStar := f.Type.(*ast.StarExpr)
+				pointer := isStar && isValueTime(star.X)
+				if !value && !pointer {
+					continue
+				}
+				raw, err := strconv.Unquote(f.Tag.Value)
+				if err != nil {
+					continue
+				}
+				jsonTag, ok := reflect.StructTag(raw).Lookup("json")
+				if !ok {
+					continue
+				}
+				key := strings.Split(jsonTag, ",")[0]
+				if key == "" || key == "-" {
+					continue
+				}
+				out[[2]string{ts.Name.Name, key}] = timestampField{
+					pointer: pointer,
+					field:   f.Names[0].Name,
+					file:    filepath.Base(path),
+				}
+			}
+			return true
+		})
+	}
+	return out
+}
+
+func parseGoFiles(t *testing.T, paths []string) map[string]*ast.File {
+	t.Helper()
+	fset := token.NewFileSet()
+	out := map[string]*ast.File{}
+	for _, p := range paths {
+		f, err := parser.ParseFile(fset, p, nil, 0)
+		if err != nil {
+			t.Fatalf("parse %s: %v", p, err)
+		}
+		out[p] = f
+	}
+	return out
+}
+
+// TestNoWrapperTimestampNarrowerThanGenerated closes the class #620 named.
+//
+// A wrapper field typed value time.Time whose generated counterpart is
+// *time.Time cannot represent absence, and — this is what makes it invisible —
+// NOTHING IN THE WRAPPER SOURCE SAYS SO. The evidence lives in the generated
+// client, so the guard has to read both sides.
+//
+// Why the two neighbouring gates cannot do this:
+//
+//   - TestNoValueTypedOptionalTimestamps (above) keys on `,omitempty` and
+//     `continue`s without it. All five #620 fields were tagged bare
+//     `json:"created_at"`, so they were invisible to it. Its own doc comment
+//     named this gap and deferred it; this is the close.
+//   - scripts/check-wrapper-drift has the (wrapper, generated) pairing but
+//     compares tag NAMES only. Teaching it types would still miss
+//     WebhookDelivery, which its header excludes BY DESIGN as a parallel
+//     webhook-flavored shape — and WebhookDelivery.CreatedAt is one of the five.
+//
+// So the pairing here is by struct name + json key, which is blind to the
+// converter tiers and therefore covers all of them.
+//
+// Scoped to timestamps deliberately. The wrapper flattens optional
+// *string/*bool/*int to value types on purpose; a nil-capability rule applied
+// broadly reports ~345 intentional fields. Timestamps are the principled
+// carve-out: an absent string marshals away harmlessly, an absent time.Time
+// marshals as a wrong value.
+func TestNoWrapperTimestampNarrowerThanGenerated(t *testing.T) {
+	entries, err := os.ReadDir(".")
+	if err != nil {
+		t.Fatalf("read package dir: %v", err)
+	}
+	var wrapperPaths []string
+	for _, e := range entries {
+		name := e.Name()
+		if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+			continue
+		}
+		wrapperPaths = append(wrapperPaths, name)
+	}
+
+	wrapper := collectTimestampFields(t, parseGoFiles(t, wrapperPaths))
+	gen := collectTimestampFields(t, parseGoFiles(t, []string{
+		filepath.Join("..", "generated", "client.gen.go"),
+	}))
+
+	if len(wrapper) == 0 || len(gen) == 0 {
+		t.Fatalf("collected %d wrapper and %d generated timestamp fields — the walk is broken, not the surface",
+			len(wrapper), len(gen))
+	}
+
+	var (
+		violations []string
+		compared   int
+	)
+	for key, w := range wrapper {
+		g, ok := gen[key]
+		if !ok {
+			continue
+		}
+		compared++
+		if w.pointer || !g.pointer {
+			continue
+		}
+		violations = append(violations, fmt.Sprintf(
+			"%s.%s (json:%q, %s) is time.Time but generated.%s.%s is *time.Time",
+			key[0], w.field, key[1], w.file, key[0], g.field))
+	}
+
+	// A name-keyed join that matches nothing passes vacuously.
+	if compared == 0 {
+		t.Fatal("zero (wrapper, generated) timestamp fields paired — the join key is wrong, not the surface")
+	}
+
+	sort.Strings(violations)
+	for _, v := range violations {
+		t.Errorf("wrapper timestamp is narrower than the schema it decodes: %s. "+
+			"An omitted value decodes to the zero time and re-marshals as a fabricated "+
+			"0001-01-01T00:00:00Z, indistinguishable from a real timestamp", v)
+	}
+}
+
 // isValueTime reports whether expr is the type `time.Time` exactly — not
 // *time.Time, not []time.Time, not a named alias.
 func isValueTime(expr ast.Expr) bool {

go/pkg/basecamp/timeline.go

@@ -16,8 +16,11 @@ const DefaultTimelineLimit = 100
 
 // TimelineEvent represents an activity event in the timeline.
 type TimelineEvent struct {
-	ID        int64     `json:"id"`
-	CreatedAt time.Time `json:"created_at"`
+	ID int64 `json:"id"`
+	// CreatedAt is optional — nil means the API did not send it. A value type
+	// here would fabricate 0001-01-01T00:00:00Z for an absent timestamp, which
+	// no consumer can tell from a real one.
+	CreatedAt *time.Time `json:"created_at,omitempty"`
 	// Kind is an open, non-exhaustive vocabulary (BC3 adds new kinds over time);
 	// treat unrecognized values as valid. Common values include message_created,
 	// todo_created, todo_completed, upload_created, schedule_entry_created,
@@ -497,7 +500,7 @@ func timelineEventFromGenerated(ge generated.TimelineEvent) TimelineEvent {
 		e.ParentRecordingID = *ge.ParentRecordingId
 	}
 
-	e.CreatedAt = deref(ge.CreatedAt)
+	e.CreatedAt = ge.CreatedAt
 
 	if ge.Creator != nil {
 		creator := personFromGenerated(*ge.Creator)

go/pkg/basecamp/timeline_test.go

@@ -78,8 +78,10 @@ func TestTimelineEvent_Unmarshal(t *testing.T) {
 
 	// Check timestamp
 	expectedTime := time.Date(2024, 3, 15, 10, 30, 0, 0, time.UTC)
-	if !event.CreatedAt.Equal(expectedTime) {
-		t.Errorf("expected CreatedAt %v, got %v", expectedTime, event.CreatedAt)
+	if event.CreatedAt == nil {
+		t.Error("expected CreatedAt to be non-nil")
+	} else if !event.CreatedAt.Equal(expectedTime) {
+		t.Errorf("expected CreatedAt %v, got %v", expectedTime, *event.CreatedAt)
 	}
 }
 

go/pkg/basecamp/webhooks.go

@@ -36,8 +36,11 @@ type Webhook struct {
 
 // WebhookDelivery represents a recent delivery attempt for a webhook.
 type WebhookDelivery struct {
-	ID        int64                   `json:"id"`
-	CreatedAt time.Time               `json:"created_at"`
+	ID int64 `json:"id"`
+	// CreatedAt is optional — nil means the API did not send it. A value type
+	// here would fabricate 0001-01-01T00:00:00Z for an absent timestamp, which
+	// no consumer can tell from a real one.
+	CreatedAt *time.Time              `json:"created_at,omitempty"`
 	Request   WebhookDeliveryRequest  `json:"request"`
 	Response  WebhookDeliveryResponse `json:"response"`
 }
@@ -362,7 +365,7 @@ func webhookFromGenerated(gw generated.Webhook) Webhook {
 		w.RecentDeliveries = make([]WebhookDelivery, len(gw.RecentDeliveries))
 		for i, gd := range gw.RecentDeliveries {
 			d := WebhookDelivery{
-				CreatedAt: deref(gd.CreatedAt),
+				CreatedAt: gd.CreatedAt,
 			}
 			if gd.Request != nil {
 				d.Request = WebhookDeliveryRequest{

go/pkg/basecamp/webhooks_test.go

@@ -174,7 +174,9 @@ func TestWebhook_UnmarshalGetWithRecentDeliveries(t *testing.T) {
 	if delivery.ID != 1230 {
 		t.Errorf("expected delivery ID 1230, got %d", delivery.ID)
 	}
-	if delivery.CreatedAt.IsZero() {
+	if delivery.CreatedAt == nil {
+		t.Error("expected non-nil delivery CreatedAt")
+	} else if delivery.CreatedAt.IsZero() {
 		t.Error("expected non-zero delivery CreatedAt")
 	}