BC5 readiness: Go wrapper forward-compat audit + field-level drift check

Makefile

@@ -204,7 +204,7 @@ sync-api-version-check:
 # Go SDK targets (delegates to go/Makefile)
 #------------------------------------------------------------------------------
 
-.PHONY: go-test go-lint go-check go-clean go-check-drift
+.PHONY: go-test go-lint go-check go-clean go-check-drift go-check-wrapper-drift
 
 go-test:
 	@$(MAKE) -C go test
@@ -223,6 +223,15 @@ go-check-drift:
 	@echo "==> Checking service layer drift..."
 	@./scripts/check-service-drift.sh
 
+# Check for field-level drift between generated structs and hand-written
+# wrappers in go/pkg/basecamp/. Sibling of go-check-drift; that check is
+# operation-level, this one is field-level.
+go-check-wrapper-drift:
+	@echo "==> Running wrapper-drift checker tests..."
+	@go test ./scripts/check-wrapper-drift/
+	@echo "==> Checking wrapper field-level drift..."
+	@go run ./scripts/check-wrapper-drift/
+
 .PHONY: auth-routable-check
 
 # Check that hop-2-only primitives are not called outside the authenticated
@@ -623,7 +632,7 @@ generate:
 	@echo "==> Generation complete"
 
 # Run all checks (Smithy + Go + TypeScript + Ruby + Kotlin + Swift + Python + Behavior Model + Conformance + Provenance + Actions lint)
-check: lint-actions sync-spec-version-check smithy-check behavior-model-check provenance-check sync-api-version-check go-check-drift auth-routable-check kt-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps
+check: lint-actions sync-spec-version-check smithy-check behavior-model-check provenance-check sync-api-version-check go-check-drift go-check-wrapper-drift auth-routable-check kt-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps
 	@echo "==> All checks passed"
 
 # Clean all build artifacts
@@ -654,7 +663,8 @@ help:
 	@echo "  go-test          Run Go tests"
 	@echo "  go-lint          Run Go linter"
 	@echo "  go-check         Run all Go checks"
-	@echo "  go-check-drift   Check service layer drift vs generated client"
+	@echo "  go-check-drift           Check service layer drift vs generated client (operation-level)"
+	@echo "  go-check-wrapper-drift   Check wrapper struct drift vs generated structs (field-level)"
 	@echo "  go-clean         Remove Go build artifacts"
 	@echo ""
 	@echo "TypeScript SDK:"

go.work

@@ -3,4 +3,5 @@ go 1.26
 use (
 	./conformance/runner/go
 	./go
+	./scripts/check-wrapper-drift
 )

go/BRIEF-bc5-forward-compat-wrappers.md

@@ -0,0 +1,207 @@
+# Brief: Surface BC5 forward-compat fields through Go hand-written wrappers
+
+**Status**: blocker for `basecamp-cli` Phase 1 absorption.
+**Audience**: SDK dev agent working from `basecamp-5-is-releasing-eager-spring.md`.
+**Source**: filed by `basecamp-cli` `five` branch after attempting Phase 1 against SDK commit `7e9c4345` (Go regen of BC5 spec additions).
+
+## Summary
+
+SDK PR 1 added BC5 forward-compat fields to the **generated** Go client
+(`go/pkg/generated/client.gen.go`), but did not propagate them through the
+**hand-written wrappers** in `go/pkg/basecamp/`. The wrappers are the
+public Go API; CLI / external Go consumers never see the generated types
+directly. As a result, the new fields are dropped during the
+`*FromGenerated` conversion step and Phase 1 of the CLI absorption plan
+cannot deliver any user-visible value.
+
+The CLI's andon cord — *"if the SDK lacks a Go service wrapper for a
+generated endpoint, never call the raw generated client from CLI code"* —
+applies here in spirit: the wrapper exists, but it discards fields the
+spec now declares. The fix lives in the SDK, not the CLI.
+
+## Evidence
+
+`generated.Todo` now has `Steps []CardStep` (per `7e9c4345`'s diff to
+`client.gen.go:~1990`). The wrapper:
+
+```go
+// go/pkg/basecamp/todos.go:543
+func todoFromGenerated(gt generated.Todo) Todo {
+    t := Todo{
+        Status:      gt.Status,
+        Title:       gt.Title,
+        // … no Steps assignment …
+    }
+    // …
+    return t
+}
+```
+
+`Todo` (`go/pkg/basecamp/todos.go:17`) has no `Steps` field. CLI builds
+fail when referencing `todo.Steps` after `app.Account().Todos().Get(...)`.
+
+The same gap exists for every Phase-1 surface the CLI plan calls out:
+
+| Generated type (after PR 1) | New fields in `generated/client.gen.go` | Hand-written wrapper file | Wrapper exposes them? |
+|---|---|---|---|
+| `generated.Todo` | `Steps []CardStep` | `pkg/basecamp/todos.go` `Todo` (line 17) | **no** |
+| `generated.Todoset` | `TodosCount`, `CompletedLooseTodosCount`, `TodosUrl`, `AppTodosUrl` | `pkg/basecamp/todosets.go` `Todoset` | **no** |
+| `generated.Person` | `Tagline` (alongside existing `Bio`) | `pkg/basecamp/todos.go` `Person` (line 50) | **no** |
+| `generated.Notification` | `BubbleUpUrl`, `BubbleUpAt` | `pkg/basecamp/my_notifications.go` `Notification` (line 13) | **no** |
+| `generated.GetMyNotificationsResponseContent` | `BubbleUps []Notification`, `ScheduledBubbleUps []Notification` | `pkg/basecamp/my_notifications.go` `NotificationsResult` (line 38) | **no** |
+
+(There are likely more — these are just the ones the CLI Phase 1 hits
+first. A wider audit comparing every `*FromGenerated` function against
+its generated source after `7e9c4345` would be welcome.)
+
+## Why this happened
+
+The SDK plan §1 ("Smithy spec — forward-compat additions") prescribes
+the spec edits and PR 1 ships their regenerated client code. There is no
+explicit step in the plan that says *"after regen, propagate new fields
+through the hand-written `*FromGenerated` wrappers."* Implicit because
+the Go SDK is the only one with this layer, easy to miss when the rest
+of the SDK family is purely generated.
+
+`SPEC.md` documents the architecture ("Go demonstrates the hand-written
+service wrapper pattern"), and `CONTRIBUTING.md` mentions the
+`go-check-drift` target — that target verifies *all generated operations
+are covered by hand-written services*, but does **not** verify that
+every generated *field* on covered structures is propagated. The current
+drift check is operation-level, not field-level.
+
+## Contract / acceptance criteria
+
+For each generated field added in PR 1, do exactly one of:
+
+1. **Add a corresponding field on the wrapper struct** with the same
+   semantic meaning, JSON tag matching the wire format, and propagate
+   it inside the relevant `*FromGenerated` conversion. Default to this
+   for fields the CLI / external consumers will want to read.
+2. **Document the omission inline** with an `// intentionally-omitted:
+   <tag> - <reason>` comment anywhere inside the wrapper struct's
+   declaration block, if a field is genuinely not appropriate for the
+   public Go surface (e.g. an internal echo). `<tag>` is the JSON tag of
+   the omitted field on the GENERATED struct (e.g. `hidden`, not
+   `Hidden`); `<reason>` is required and free-form. The drift check
+   (`scripts/check-wrapper-drift`) recognises only this exact marker
+   form — see its `markerRe` regexp for the canonical syntax. Phase 1
+   fields should not need this — they're all user-visible.
+
+Per-type concrete proposals (signatures, no behavior change for
+existing fields):
+
+```go
+// pkg/basecamp/todos.go — Todo
+type Todo struct {
+    // … existing fields …
+    Steps []CardStep `json:"steps,omitempty"`
+}
+
+func todoFromGenerated(gt generated.Todo) Todo {
+    t := Todo{ /* existing assignments */ }
+    if len(gt.Steps) > 0 {
+        t.Steps = make([]CardStep, 0, len(gt.Steps))
+        for _, gs := range gt.Steps {
+            t.Steps = append(t.Steps, cardStepFromGenerated(gs))
+        }
+    }
+    return t
+}
+```
+
+```go
+// pkg/basecamp/todosets.go — Todoset
+type Todoset struct {
+    // … existing fields …
+    TodosCount               int    `json:"todos_count"`                 // BC5
+    CompletedLooseTodosCount int    `json:"completed_loose_todos_count"` // BC5
+    TodosURL                 string `json:"todos_url,omitempty"`         // BC5
+    AppTodosURL              string `json:"app_todos_url,omitempty"`     // BC5
+}
+```
+
+```go
+// pkg/basecamp/todos.go — Person
+type Person struct {
+    // … existing fields, including Bio …
+    Tagline string `json:"tagline,omitempty"` // BC5; alias of Bio per spec note
+}
+```
+
+```go
+// pkg/basecamp/my_notifications.go
+type Notification struct {
+    // … existing fields …
+    BubbleUpURL string     `json:"bubble_up_url,omitempty"` // BC5
+    BubbleUpAt  *time.Time `json:"bubble_up_at,omitempty"`  // BC5
+}
+
+type NotificationsResult struct {
+    // … existing fields, including Memories …
+    BubbleUps          []Notification `json:"bubble_ups,omitempty"`           // BC5
+    ScheduledBubbleUps []Notification `json:"scheduled_bubble_ups,omitempty"` // BC5
+}
+```
+
+For `Notification.BubbleUpAt`, use `*time.Time` rather than the bare
+`time.Time` pattern the wrapper uses for `ReadAt` / `UnreadAt`. The
+pointer is what shipped (see `my_notifications.go`): a bare `time.Time`
+with `omitempty` still marshals the zero time as
+`"0001-01-01T00:00:00Z"` (Go's `omitempty` does not suppress structs
+that are merely zero-valued), so the absence of a scheduled bubble-up
+would surface as a wrong wire value rather than an omitted key. The
+`*time.Time` convention mirrors `Card.CompletedAt` and `Todo.CompletedAt`,
+where the same omit-cleanly semantics matter. Consumers should
+`nil`-check or use the `time.Time` returned by dereferencing rather than
+calling `IsZero()`.
+
+## Verification
+
+A field-level drift check would prevent recurrence. Sketch:
+
+- Walk every wrapper struct in `pkg/basecamp/*.go`.
+- Locate the corresponding generated struct (by either explicit
+  conversion func or by name match).
+- For each generated field, fail if the wrapper has no field with a
+  matching JSON tag *and* no `// intentionally-omitted: ...` marker on
+  the wrapper struct.
+
+This is a separate hardening PR; not blocking the immediate wrapper
+update.
+
+For the immediate PR, manual verification is enough:
+
+```
+go build ./...                         # cli & sdk both clean
+go test ./pkg/basecamp/...             # existing wrapper tests pass
+make go-check-drift                    # current drift check passes
+```
+
+Then the CLI side runs:
+
+```
+make bump-sdk REF=<wrapper-PR-merge-commit>
+go build ./...                         # cli compiles with new fields
+go test ./...                          # cli tests still pass
+```
+
+## Out of scope for this brief
+
+- Other-language SDK behavior. TS / Ruby / Python / Swift / Kotlin
+  consume generated types directly (per `AGENTS.md`: "No hand-written
+  API methods exist in any SDK runtime"); those SDKs picked up the new
+  fields automatically when their generators ran.
+- Spec changes. Spec is correct; this is purely a Go-layer omission.
+- Recording.Bubbleupable. Per the CLI plan, that field tracks Phase 3e
+  (`recording-bubbleupable-field` brief, currently `no-json-contract`)
+  rather than Phase 1.
+
+## Why now
+
+`basecamp-cli` Phase 1 is the first downstream consumer to attempt
+absorbing the BC5 forward-compat fields. The Phase 1 work is presenter-only
+on paper and could ship in a single commit per surface — but every
+surface is gated on the wrapper exposing the field. CLI work will resume
+the moment a wrapper-update PR lands; the bump-sdk → presenter changes
+chain is well understood.

go/pkg/basecamp/boosts.go

@@ -364,14 +364,8 @@ func boostFromGenerated(gb generated.Boost) Boost {
 	b.ID = gb.Id
 
 	if gb.Booster.Id != 0 || gb.Booster.Name != "" {
-		b.Booster = &Person{
-			ID:           int64(gb.Booster.Id),
-			Name:         gb.Booster.Name,
-			EmailAddress: gb.Booster.EmailAddress,
-			AvatarURL:    gb.Booster.AvatarUrl,
-			Admin:        gb.Booster.Admin,
-			Owner:        gb.Booster.Owner,
-		}
+		booster := personFromGenerated(gb.Booster)
+		b.Booster = &booster
 	}
 
 	if gb.Recording.Id != 0 || gb.Recording.Title != "" {

go/pkg/basecamp/campfires.go

@@ -67,10 +67,14 @@ type Campfire struct {
 	CreatedAt        time.Time `json:"created_at"`
 	UpdatedAt        time.Time `json:"updated_at"`
 	Title            string    `json:"title"`
+	Topic            string    `json:"topic,omitempty"`
+	Position         int       `json:"position,omitempty"`
 	InheritsStatus   bool      `json:"inherits_status"`
 	Type             string    `json:"type"`
 	URL              string    `json:"url"`
 	AppURL           string    `json:"app_url"`
+	BookmarkURL      string    `json:"bookmark_url,omitempty"`
+	SubscriptionURL  string    `json:"subscription_url,omitempty"`
 	LinesURL         string    `json:"lines_url"`
 	FilesURL         string    `json:"files_url,omitempty"`
 	Bucket           *Bucket   `json:"bucket,omitempty"`
@@ -90,12 +94,14 @@ type CampfireLine struct {
 	Type             string                   `json:"type"`
 	URL              string                   `json:"url"`
 	AppURL           string                   `json:"app_url"`
+	BookmarkURL      string                   `json:"bookmark_url,omitempty"`
+	BoostsCount      int                      `json:"boosts_count,omitempty"`
+	BoostsURL        string                   `json:"boosts_url,omitempty"`
 	Content          string                   `json:"content,omitempty"`
 	Attachments      []CampfireLineAttachment `json:"attachments,omitempty"`
 	Parent           *Parent                  `json:"parent,omitempty"`
 	Bucket           *Bucket                  `json:"bucket,omitempty"`
 	Creator          *Person                  `json:"creator,omitempty"`
-	BoostsCount      int                      `json:"boosts_count,omitempty"`
 }
 
 // CampfireLineAttachment represents a file attached to an upload line.
@@ -905,10 +911,14 @@ func campfireFromGenerated(gc generated.Campfire) Campfire {
 		Status:           gc.Status,
 		VisibleToClients: gc.VisibleToClients,
 		Title:            gc.Title,
+		Topic:            gc.Topic,
+		Position:         int(gc.Position),
 		InheritsStatus:   gc.InheritsStatus,
 		Type:             gc.Type,
 		URL:              gc.Url,
 		AppURL:           gc.AppUrl,
+		BookmarkURL:      gc.BookmarkUrl,
+		SubscriptionURL:  gc.SubscriptionUrl,
 		LinesURL:         gc.LinesUrl,
 		FilesURL:         gc.FilesUrl,
 		CreatedAt:        gc.CreatedAt,
@@ -926,14 +936,8 @@ func campfireFromGenerated(gc generated.Campfire) Campfire {
 	}
 
 	if gc.Creator.Id != 0 || gc.Creator.Name != "" {
-		c.Creator = &Person{
-			ID:           int64(gc.Creator.Id),
-			Name:         gc.Creator.Name,
-			EmailAddress: gc.Creator.EmailAddress,
-			AvatarURL:    gc.Creator.AvatarUrl,
-			Admin:        gc.Creator.Admin,
-			Owner:        gc.Creator.Owner,
-		}
+		creator := personFromGenerated(gc.Creator)
+		c.Creator = &creator
 	}
 
 	return c
@@ -949,10 +953,12 @@ func campfireLineFromGenerated(gl generated.CampfireLine) CampfireLine {
 		Type:             gl.Type,
 		URL:              gl.Url,
 		AppURL:           gl.AppUrl,
+		BookmarkURL:      gl.BookmarkUrl,
+		BoostsCount:      int(gl.BoostsCount),
+		BoostsURL:        gl.BoostsUrl,
 		Content:          gl.Content,
 		CreatedAt:        gl.CreatedAt,
 		UpdatedAt:        gl.UpdatedAt,
-		BoostsCount:      int(gl.BoostsCount),
 	}
 
 	l.ID = gl.Id
@@ -990,14 +996,8 @@ func campfireLineFromGenerated(gl generated.CampfireLine) CampfireLine {
 	}
 
 	if gl.Creator.Id != 0 || gl.Creator.Name != "" {
-		l.Creator = &Person{
-			ID:           int64(gl.Creator.Id),
-			Name:         gl.Creator.Name,
-			EmailAddress: gl.Creator.EmailAddress,
-			AvatarURL:    gl.Creator.AvatarUrl,
-			Admin:        gl.Creator.Admin,
-			Owner:        gl.Creator.Owner,
-		}
+		creator := personFromGenerated(gl.Creator)
+		l.Creator = &creator
 	}
 
 	return l