Skip to content

Migrate to basecamp-sdk TodoListOptions.Completed#456

Merged
jeremy merged 1 commit intomainfrom
todos-completed
Apr 28, 2026
Merged

Migrate to basecamp-sdk TodoListOptions.Completed#456
jeremy merged 1 commit intomainfrom
todos-completed

Conversation

@jeremy
Copy link
Copy Markdown
Member

@jeremy jeremy commented Apr 28, 2026

Summary

basecamp-sdk PR #279 (merged 2026-04-23, unreleased — no tag past v0.7.3) restructures TodoListOptions:

  • Status becomes lifecycle-only ("archived", "trashed", or empty).
  • A new Completed bool field handles the completion filter.
  • Unsupported Status values are rejected at the wrapper boundary.
  • Status: "" no longer returns all todos — it now returns incomplete only.

The CLI today encodes the completion filter via Status: "completed" / Status: "pending" and relies on Status: "" returning all todos so listAllTodos can filter client-side. Both assumptions break under the new SDK.

This PR ships the SDK bump and the CLI migration together.

What changed

  • SDK bumpgo.mod / go.sum / internal/version/sdk-provenance.json to v0.7.4-0.20260423230153-f54589f0924a (merge-commit pseudo-version).
  • internal/commands/todos.go — new resolveStatusFilter helper maps --status {completed,incomplete,archived,trashed} to (sdkStatus, sdkCompleted) and rejects everything else (including legacy pending) with ErrUsage. Threaded the pair through runTodosListlistTodosInList / listAllTodosfetchTodosIncludingGroups. Deleted the incomplete→pending coercion and the cross-list client-side status filter — the server is now the single source of truth on both paths.
  • internal/tui/workspace/data/hub.goCompletedTodos switches to {Completed: true}. Behavior fix in Hub.Todos: the empty {} now correctly returns only incomplete todos, so the active pane no longer leaks completed ones.
  • internal/tui/empty/messages.go — drop the dead "pending" alias from the empty-state context switch.
  • skills/basecamp/SKILL.md--status flag values updated to (completed/incomplete/archived/trashed).
  • Teststodos_test.go extended statusCapturingTransport to capture every /todos.json request's query params and to simulate server-side filtering. Rewrote 3 existing tests, added 8 new ones covering the matrix (single-list × cross-list × completed/archived/trashed, plus bogus and pending rejection asserting output.CodeUsage and totalCount == 0).
  • WithMaxRetries(0) → (1) sweep — the new SDK enforces a minimum of 1 attempt; (1) still means "no retries."

Breaking change

--status pending is no longer accepted and now returns ErrUsage. This is option A from the migration handoff (drop pending as a synonym for incomplete). Option B (keep pending as an alias mapped to the API default) is a one-line addition to resolveStatusFilter. Flag the choice if you'd prefer B before merge.

Test plan

  • bin/ci green (formatting, vet, lint, unit + e2e BATS, surface, skill drift, provenance, tidy)
  • --status completed, --status archived, --status trashed all send the expected query params on both single-list and cross-list paths (covered by new tests)
  • --status pending and bogus values return ErrUsage before any HTTP fires
  • No replace directive in go.mod; no legacy Status: "completed" / Status: "pending" strings remain in internal/
  • TUI manual check: ./bin/basecamp tui → project → completed-todos pane populates; active pane no longer mixes in completed todos

Summary by cubic

Migrates to github.com/basecamp/basecamp-sdk/go’s new TodoListOptions (lifecycle-only Status + Completed bool) and updates CLI/TUI filtering to use the server as the source of truth. Fixes the TUI active todos pane showing completed items and rejects --status pending.

  • Refactors

    • Map --status {completed,incomplete,archived,trashed} to (Status, Completed) via a new resolver; invalid values return ErrUsage.
    • Remove client-side status filtering for cross-list and single-list paths; API applies filters directly.
    • TUI: use {Completed: true} for completed panes; empty options now return only incomplete todos.
    • Tests: add coverage for query params and server-side filtering across single-list/cross-list.
    • Enforce WithMaxRetries(1) per new SDK requirement; bump SDK to v0.7.4-0.20260423230153-f54589f0924a.
  • Migration

    • Replace --status pending with --status incomplete.
    • Supported values are now: completed, incomplete, archived, trashed.

Written for commit 6f24cb9. Summary will update on new commits. Review in cubic

Bump basecamp-sdk to v0.7.4-0.20260423230153-f54589f0924a (merge-commit
pseudo-version; no tag past v0.7.3 yet) and migrate the CLI to its new
TodoListOptions shape. The SDK now uses Status for lifecycle only
("archived"/"trashed") and a separate Completed bool for the completion
filter; unsupported Status values ErrUsage at the wrapper boundary, and
omitting Status no longer returns all todos — it returns incomplete only.

Add resolveStatusFilter in internal/commands/todos.go to map --status
{completed,incomplete,archived,trashed} to (sdkStatus, sdkCompleted).
Thread the pair through runTodosList → listTodosInList / listAllTodos →
fetchTodosIncludingGroups; drop the old "incomplete→pending" coercion
and the cross-list client-side status filter (the server is now the
single source of truth on both paths).

In the TUI data layer, switch Hub.CompletedTodos to {Completed: true}.
Hub.Todos's empty {} now correctly returns only incomplete todos —
behavior fix: the active pool no longer leaks completed todos into the
active todos pane.

User-facing breaking change: --status pending is no longer accepted and
returns ErrUsage. Drop the dead "pending" alias from the empty-state
context switch.

Sweep WithMaxRetries(0) → (1) across test setups; the new SDK enforces
a minimum of 1 attempt (1 still means "no retries").
Copilot AI review requested due to automatic review settings April 28, 2026 19:34
@github-actions github-actions Bot added commands CLI command implementations tui Terminal UI sdk SDK wrapper and provenance tests Tests (unit and e2e) skills Agent skills deps breaking Breaking change bug Something isn't working labels Apr 28, 2026
@github-actions
Copy link
Copy Markdown

⚠️ Potential breaking changes detected:

  • The '--status' flag for the 'todos list' command has new valid values ('archived' and 'trashed') and rejects the previously valid value 'pending' with an error. This is a breaking change for users who previously used 'pending' as an argument for the '--status' flag.
  • The '--status' flag in the 'todos list' command now has validation that rejects invalid values (e.g., 'bogus') with an error instead of ignoring them. Scripts passing invalid values that relied on the previous behavior may now break.

Review carefully before merging. Consider a major version bump.

@jeremy jeremy added this to the v0.8.0 milestone Apr 28, 2026
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

Migrates the CLI (and a small TUI path) to the basecamp-sdk TodoListOptions restructure by replacing legacy Status completion filtering with the new Completed flag, and updating validation + tests accordingly.

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:

  • Bumps github.com/basecamp/basecamp-sdk/go to v0.7.4-0... and updates provenance/go.sum accordingly.
  • Updates todos list to translate --status into the SDK’s (Status, Completed) pair (rejecting unsupported values like pending) and threads those options through single-list and cross-list fetch paths.
  • Adjusts TUI completed-todos fetch to use Completed: true, updates empty-state messaging, and expands/rewrites todo status tests to assert query params + server-side filtering behavior.

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated no comments.

Show a summary per file
File Description
skills/basecamp/SKILL.md Updates documented --status values to include archived/trashed and drop legacy assumptions.
internal/version/sdk-provenance.json Records the new SDK pseudo-version, revision, and update timestamp.
internal/tui/workspace/data/hub_test.go Updates tests to comply with SDK retry minimum (WithMaxRetries(1)).
internal/tui/workspace/data/hub.go Switches completed-todos listing to TodoListOptions{Completed: true}.
internal/tui/resolve/account_test.go Updates tests to use WithMaxRetries(1).
internal/tui/empty/messages.go Removes the dead "pending" alias from todo empty-state messaging.
internal/names/resolver_test.go Updates tests to use WithMaxRetries(1).
internal/completion/refresh_test.go Updates tests to use WithMaxRetries(1).
internal/commands/todos_test.go Expands status filter test harness to capture query params and validates new (status, completed) behavior + usage errors.
internal/commands/todos.go Adds resolveStatusFilter, threads (sdkStatus, sdkCompleted) through listing paths, and removes client-side completion filtering.
internal/commands/search_test.go Updates tests to use WithMaxRetries(1).
internal/commands/recordings_test.go Updates tests to use WithMaxRetries(1).
internal/commands/quickstart_test.go Updates tests to use WithMaxRetries(1).
internal/commands/profile_test.go Updates tests to use WithMaxRetries(1).
internal/commands/people_test.go Updates tests to use WithMaxRetries(1).
internal/commands/messages_test.go Updates tests to use WithMaxRetries(1).
internal/commands/doctor_test.go Updates tests to use WithMaxRetries(1).
internal/commands/cards_test.go Updates tests to use WithMaxRetries(1).
go.sum Updates dependency checksums for the SDK bump and related transitive upgrades.
go.mod Bumps SDK and a few indirect deps pulled along by the upgrade.

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

Copy link
Copy Markdown
Contributor

@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.

No issues found across 20 files

@jeremy jeremy merged commit aa7055e into main Apr 28, 2026
30 checks passed
@jeremy jeremy deleted the todos-completed branch April 28, 2026 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change bug Something isn't working commands CLI command implementations deps sdk SDK wrapper and provenance skills Agent skills tests Tests (unit and e2e) tui Terminal UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants