test(todos): guard todos show --json surfaces SDK fields (Refs #449)

[jeremy]

What

Adds TestTodosShowJSONSurfacesSDKFields, a regression guard for #449: the fields that issue reported as missing from todos show --json — due_on, starts_on, comments_count, boosts_count, completion_subscribers, description_attachments — must all surface in the JSON output.

Why this is Refs, not Fixes

The implementation already landed on main (unreleased — latest published CLI is v0.7.2, which predates it). Provenance chain:

• SDK Fix update detection for pseudo-version builds #375 added completion_subscribers and comments_count.
• SDK Send combined CLI and SDK versions in API user agent #400 added description_attachments.
• CLI Bump SDK to 81660dc1 and surface BC5 forward-compat fields #552 absorbed the earlier field set; CLI Expose client visibility at create time (Fixes #457) #554's SDK bump absorbed Send combined CLI and SDK versions in API user agent #400 (main now rides a98f52fd3ddd).
• due_on, starts_on, boosts_count already existed with omitempty.

todos show emits the SDK Todo object directly (todos.go data := any(todo)), so all six fields surface today. The remaining work is coverage, not a fix.

The test

Serves a fully-populated todo from a focused GET transport (nonzero due_on/starts_on/comments_count/boosts_count so the omitempty fields are actually exercised, plus a non-empty completion_subscribers and description_attachments), runs todos show 999 with JSON output, and asserts both the presence and the values of all six fields. This keeps a future SDK bump from silently dropping any of them.

Deliberately does not add a CLI presenter/model wrapper to force stable zero-valued keys — that would fork the SDK contract; a stable-zero-key policy, if ever wanted, is a separate upstream SDK-model decision.

bin/ci green.

Refs #449

---

Summary by cubic

Add a regression test (TestTodosShowJSONSurfacesSDKFields) to ensure todos show --json includes due_on, starts_on, comments_count, boosts_count, completion_subscribers, and description_attachments (Refs #449). The test serves a fully populated todo and asserts presence and values; this is coverage only since the behavior already exists on main via recent SDK bumps.

^{Written for commit 98fe6e0. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 1 file

_{Re-trigger cubic}

[Copilot]

Pull request overview

Adds a regression test to ensure basecamp todos show (JSON output mode) continues to surface specific SDK-backed fields that were reported missing in #449, guarding against future SDK bumps or serialization changes that silently drop those keys/values.

Changes:

• Add a focused test (TestTodosShowJSONSurfacesSDKFields) asserting presence (and some values) for due_on, starts_on, comments_count, boosts_count, completion_subscribers, and description_attachments.
• Introduce a dedicated http.RoundTripper that serves a fully-populated todo payload for the show path.

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.

Comments suppressed due to low confidence (1)

internal/commands/todos_test.go:3025

• The description_attachments assertion only checks the filename. Since this test’s goal is to guard that the SDK field is surfaced correctly, it would be stronger to also assert a couple of other expected attachment fields (e.g., id and download_url) so shape/value regressions are caught.

	var attachments []struct {
		Filename string `json:"filename"`
	}
	require.NoError(t, json.Unmarshal(resp.Data["description_attachments"], &attachments))
	require.Len(t, attachments, 1)

---

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

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/commands/todos_test.go:2936

• populatedTodoTransport.RoundTrip returns a successful 200 response for any request, which makes this regression test less precise and could allow future changes (additional SDK calls or different paths/methods) to slip through without being detected. Guard the transport to only accept the expected GET for this todo ID and fail fast on anything else.

func (populatedTodoTransport) RoundTrip(req *http.Request) (*http.Response, error) {
	// Fail closed: this transport only knows how to answer the todo GET. Any
	// other route or method means `todos show` took an unexpected path, which
	// should fail the test rather than silently pass on the canned body.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/commands/todos_test.go:2939

• The RoundTrip route guard is still too permissive: strings.Contains(req.URL.Path, "/todos/999") will also match unexpected paths like "/todos/9999.json" or "/todos/999/comments.json". That can let the test pass even if the command hits the wrong endpoint/ID. Tighten the match to the expected todo GET path suffix so unexpected requests fail closed.

	if req.Method != http.MethodGet || !strings.Contains(req.URL.Path, "/todos/999") {
		return nil, fmt.Errorf("unexpected request: %s %s", req.Method, req.URL.Path)
	}