Skip to content

Add the basecamp subtasks umbrella on the SDK's Subtasks service - #791

Open
zachasme wants to merge 3 commits into
mainfrom
feat/subtasks
Open

zachasme wants to merge 3 commits into
mainfrom
feat/subtasks

Conversation

@zachasme

Copy link
Copy Markdown
Contributor

What

basecamp/basecamp-sdk#883 adds a Subtasks service on bc3's flat canonical subtask routes (bc3#12659). This surfaces all eight of its operations as a new basecamp subtasks group:

basecamp subtasks list       <todo-or-card-id|url> [--limit N | --all | --page N]
basecamp subtasks show       <id|url>
basecamp subtasks create     <todo-or-card-id|url> <title> [--due DATE] [--assignees LIST]
basecamp subtasks update     <id|url> [title] [--title T] [--due DATE | --no-due] [--assignees LIST | --no-assignees]
basecamp subtasks complete   <id|url>
basecamp subtasks uncomplete <id|url>
basecamp subtasks move       <id|url> --position N      # 1-based
basecamp subtasks delete     <id|url>
  • The routes are scoped to the account, so no verb takes --in. A parent is a to-do or a card, given by id or pasted URL. A subtask URL is its parent's URL ending in #__recording_<id>, and the fragment names the subtask.
  • update is partial, the way the SDK and bc3 are. --no-due and --no-assignees send the explicit clears ("due_on": "", "assignee_ids": []).
  • complete, uncomplete, move and delete get a bare 204 back, so their payload reports the state that was requested.

Registration, catalog entry, .surface, API-COVERAGE row, MCP model sync, skill docs, unit tests, e2e error-handling tests and a level-1 smoke suite all follow the shape of #677 (bubble-up), with #682 and #689 for the SDK-bump-plus-new-group pieces. The skill's old section on to-do subtasks, which used raw basecamp api calls, is replaced with the new commands.

⚠️ SDK pinned to main: swap for a release before merge

#883 has not been released yet, so go.mod pins basecamp-sdk main at the immutable commit 8216dd62c329 (v0.20.1-0.20260928111832-8216dd62c329), using make bump-sdk REF=<sha>. sdk-provenance.json, the vendored MCP model (PROVENANCE.json) and the Nix vendorHash all agree with that pin. Before merge, once basecamp-sdk cuts the release that ships Subtasks, re-pin to it (make bump-sdk REF=go/vX.Y.Z, scripts/sync-mcp-model.sh, make update-nix-hash).

What the pin brings along

Moving from v0.19.0 to main also brings in SDK changes the CLI had to absorb:

  • #930 split the Automation tag into Checkins, Templates, Webhooks, Lineup, Dock, Recordings and Search. The MCP automation domain now claims all seven, so the tools it serves are unchanged (the catalog snapshot diff is only the subtask actions). Subtasks joins the todos domain.
  • #924 checks event-feed refusal bodies when they are decoded. A body that isn't the documented shape now comes back as a statusless malformed response, which the SDK used to half-type. Four internal/mcpserver/feed_test.go cases assumed the old behavior. They now assert request_failed with no http_status, and the filter-mismatch fixture uses digests in the documented 16-hex shape. feed.go itself is unchanged.
  • #883 makes CardSteps.Reposition refuse 0. bc3 has always counted positions from 1, even though its docs said "zero indexed", so cards step move --position is now documented and validated as 1-based.

Test plan

  • bin/ci green locally
  • New unit tests (internal/commands/subtasks_test.go) cover each verb's route, method and body against the recording transport
  • e2e/subtasks.bats covers argument and flag errors
  • e2e/smoke/smoke_subtasks.bats (level 1) runs the full lifecycle against a real to-do on the next smoke run

Card: https://app.basecamp.com/2914079/buckets/48521764/card_tables/cards/10346213842

🤖 Generated with Claude Code

basecamp/basecamp-sdk#883 models subtasks as a first-class resource on
bc3's flat canonical routes. This surfaces all eight operations as
`basecamp subtasks list|show|create|update|complete|uncomplete|move|delete`,
account-scoped (no --in), taking a to-do or card as the parent.

The SDK is pinned to basecamp-sdk main at 8216dd62c329 because #883 is not
released yet; the pin must be swapped for the release that ships it before
merge.

The pin carries three SDK changes past v0.19.0 that the CLI has to absorb:
- #930 split the Automation tag into Checkins, Templates, Webhooks, Lineup,
  Dock, Recordings and Search. The MCP automation domain claims them all,
  so its served surface is unchanged; Subtasks joins the todos domain.
- #924 refuses malformed event-feed refusal bodies at the decode as a
  statusless malformed response. The MCP feed tests now assert that, and
  the filter-mismatch fixture uses digests of the documented shape.
- #883 itself makes CardStepsService.Reposition refuse 0, since bc3 always
  counted from 1, so `cards step move --position` is now 1-based.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 28, 2026 11:46
@github-actions github-actions Bot added commands CLI command implementations sdk SDK wrapper and provenance tests Tests (unit and e2e) skills Agent skills docs deps labels Sep 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The unreleased SDK pin, unsafe permanent deletion, and input-validation issues must be resolved before approval.

Review effort: Balanced
Findings: 2 High severity · 2 Medium severity

Open (4)
What changed in this PR

Adds account-scoped subtask management across the CLI, SDK-backed MCP catalog, documentation, and test suites.

Changes:

  • Adds eight basecamp subtasks operations with URL support and pagination.
  • Refreshes SDK/MCP provenance and updates card-step positioning to 1-based.
  • Adds unit, e2e, and smoke coverage.

[!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.

File Description
.surface Records the new CLI surface.
API-COVERAGE.md Documents subtask API coverage.
e2e/​smoke/​run_smoke.sh Registers subtask smoke tests.
e2e/​smoke/​smoke_subtasks.bats Tests the live subtask lifecycle.
e2e/​subtasks.bats Tests CLI validation errors.
go.mod Pins the unreleased SDK revision.
go.sum Updates SDK checksums.
internal/​cli/​root.go Registers the command group.
internal/​commands/​cards.go Enforces 1-based step positions.
internal/​commands/​cards_test.go Updates position validation expectations.
internal/​commands/​commands.go Adds subtasks to the catalog.
internal/​commands/​commands_test.go Registers subtasks in catalog tests.
internal/​commands/​subtasks.go Implements all subtask commands.
internal/​commands/​subtasks_test.go Tests routes, bodies, and validation.
internal/​mcpserver/​catalog_test.go Updates MCP operation counts.
internal/​mcpserver/​domains.go Maps subtasks and split SDK tags.
internal/​mcpserver/​feed_test.go Adapts malformed-response assertions.
internal/​mcpserver/​model/​PROVENANCE.json Records MCP model provenance.
internal/​mcpserver/​model/​behavior-model.json Adds subtask operation behavior.
internal/​mcpserver/​model/​openapi.json Syncs the updated OpenAPI model.
internal/​mcpserver/​testdata/​catalog_snapshot.txt Updates the MCP catalog snapshot.
internal/​version/​sdk-provenance.json Records SDK/API revisions.
nix/​package.nix Refreshes the vendor hash.
skills/​basecamp/​SKILL.md Documents native subtask commands.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/commands/subtasks.go
Comment thread internal/commands/subtasks.go
Comment thread go.mod
Comment thread internal/commands/subtasks.go
…gative --limit

Review findings on #791:
- `subtasks delete` is a permanent, non-trashable delete, so it now goes
  through ensureDeleteConfirmable and tui.ConfirmDangerous with --force,
  the same gate `chat delete` uses.
- A to-do or card URL without a #__recording_ fragment names the parent;
  the per-subtask verbs no longer read its id as a subtask id. Subtask and
  card-step URLs, which name the subtask in the path, are accepted.
- `subtasks list --limit -1` is a usage error, as on the other lists.
- The skill's endpoint count follows API-COVERAGE to 203.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 28, 2026 12:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Collection URLs can be misinterpreted as individual subtask IDs, including by permanent deletion.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (4)
Previously missed (1)

In code that hasn't changed since last review

Low severity Use singular noun in summaries with one result

internal/​commands/​subtasks.go:126

The summary renders grammatically incorrect output for a single result (1 subtasks on #123). Select the noun based on the count.

Comment thread internal/commands/subtasks.go Outdated
Codex and Copilot found two URL shapes subtaskIDArg still misread. One is a
subtasks or card-steps collection URL, which carries the parent's id. The
other is a numeric fragment on a URL that is not a to-do or card, which
includes a bare #<id> and a fragment on a subtask URL. A fragment now counts
only as #__recording_<id> on a to-do or card URL. A path id counts only on a
single subtask or step URL that has no fragment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 28, 2026 12:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The SDK release re-pin remains a stated pre-merge requirement, and the new summary has a minor grammatical defect.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Use singular subtask label when count is one

internal/​commands/​subtasks.go:126

This summary renders 1 subtasks on #… for a single result, and the new unit test currently codifies that grammatical error. Select subtask/subtasks from the count and update the expected summary accordingly.

This branch has not been deployed

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

Labels

commands CLI command implementations deps docs sdk SDK wrapper and provenance skills Agent skills tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants