Todolist and Card Table Templates - #705
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The pinned SDK leaves the CLI uncompilable, and coverage, test, completion-breadcrumb, and documentation fixes remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR groups template commands by kind and adds card-table templates plus templatification workflows while preserving legacy aliases.
Changes:
- Adds grouped project, to-do list, and card-table template commands.
- Adds duplication and templatification operations, including card triage support.
- Updates registrations, documentation, generated surfaces, tests, and smoke coverage.
File summaries
| File | Reviewed changes and findings |
|---|---|
skills/basecamp/SKILL.md |
Documents grouped commands and aliases. Nit (1 vote, line 1022): document card-table compatibility aliases. |
internal/commands/todolists.go |
Registers to-do-list templatification commands. |
internal/commands/templates.go |
Implements template operations. Critical (3 votes, line 148): bump the SDK and refresh provenance/checksums; the current pin leaves the CLI uncompilable. Moderate (2 votes, line 1128): add card-table templatification tests. Moderate (1 vote, line 267): add card-table creation tests. Nit (1 vote, line 148): update API coverage. Moderate (1 vote, line 330): preserve the destination project in completion breadcrumbs. |
internal/commands/templates_test.go |
Adds request and output tests. |
internal/commands/commands.go |
Updates the command catalog. Nit (1 vote, line 46): update the stale API coverage matrix and summary. |
internal/commands/commands_test.go |
Updates command registration fixtures. |
internal/commands/cardtables.go |
Adds the card-tables command group. |
internal/cli/root.go |
Registers card-tables at the root. |
e2e/templates.bats |
Adds CLI surface and validation coverage. Nit (1 vote, line 359): rename the duplicated test case. |
e2e/smoke/smoke_lifecycle.bats |
Excludes new mutations from lifecycle smoke coverage. |
e2e/smoke/smoke_account.bats |
Adds account-level template smoke coverage. |
.surface |
Updates generated command and flag metadata. |
Review details
Suppressed comments (6)
e2e/templates.bats:359
- This test name is duplicated at lines 302 and 359, so the Bats report cannot distinguish the template-group help check from the root card-tables help check. Rename this newly added case to describe the root command explicitly.
@test "card-tables without subcommand shows help" {
internal/commands/commands.go:46
- The new SDK-backed methods also leave the coverage matrix stale:
API-COVERAGE.mdstill reports 10 template operations and 192/192 tracked endpoints. The repository's SDK completeness bar inAGENTS.md:160-164requires a coverage row for every new SDK service method; update the matrix and summary alongside this catalog entry.
{Name: "card-tables", Category: "core", Description: "Manage card tables", Actions: []string{"templatify", "templatification"}},
internal/commands/templates.go:269
CreateLibraryCardTableis a new SDK operation, but the tests cover only card-table listing and duplication; unlike the to-do-list create path, there is no recording test for this endpoint's request body and rendered result. Add one alongsideTestTemplatesTodolistsCreateSendsNameAndDescriptionso a wrong route or payload cannot pass.
cardTable, err := app.Account().Templates().CreateLibraryCardTable(cmd.Context(), name)
if err != nil {
return convertSDKError(err)
internal/commands/templates.go:149
- The new template operations are not reflected in
API-COVERAGE.md: its Templates row still lists 10 operations and omits card-table template reads/creation, to-do-list template creation, and templatification. The repository's SDK sync rule inAGENTS.md:146-164requires an API-COVERAGE entry for every new SDK method, so update the matrix and totals with this change.
library, err := app.Account().Templates().GetLibraryCardTables(cmd.Context())
if err != nil {
internal/commands/templates.go:330
- The completion breadcrumb drops the destination project.
cards listalways resolves a project before using--card-table(seeinternal/commands/cards.go:511-525), so without a configured project this follow-up prompts or can target the wrong project and fail to find the duplicated board. Include the destination bucket in the breadcrumb so the generated command is self-contained.
Cmd: fmt.Sprintf("basecamp cards list --card-table %d%s", table.ID, contextArgs),
skills/basecamp/SKILL.md:1023
- The implementation exposes
copyandcopy-statusas aliases under both grouped kinds, but this documentation only mentionstemplates todolists. Please mentiontemplates card-tableshere too so users can discover the compatibility spelling for card-table templates.
`copy` and `copy-status` also work inside `templates todolists`. Prefer the grouped,
canonical spellings above when writing new commands.
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| library, err := app.Account().Templates().GetLibraryCardTables(cmd.Context()) | ||
| if err != nil { | ||
| return convertSDKError(err) |
There was a problem hiding this comment.
🤖 Known and intended. This is stated in the PR description under "Depends on": three commits use operations added in basecamp-sdk#877 and do not compile against the current release, and go.mod is bumped once that merges and the SDK ships. The PR is open ahead of the bump so the surface can be reviewed while the dependency lands, not to be merged at this pin.
API coverage is a fair catch and will be updated in the same commit as the bump, since the endpoint list is only final once the SDK is.
| group: "card-tables", | ||
| offersTriage: true, | ||
| describeCompletedResult: func(save *basecamp.Templatification, contextArgs string) (string, []output.Breadcrumb, bool) { | ||
| table := save.DestinationCardTable | ||
| if table == nil { |
There was a problem hiding this comment.
🤖 Fair catch, and this one is not covered by a prior decision. You are right that nothing asserts move_cards_to_triage reaches the wire, and nothing asserts the card-table completion renders a real title and id rather than falling through to the generic line. TestTemplatifyOffersTriageOnlyForCardTables only checks flag registration, which proves the flag exists and not that it does anything.
Both behaviours were verified end to end against a live server, including running the same board twice with and without the flag and diffing the resulting template structure. But that is not in CI, so it does not protect against a regression, which is exactly your point about a silent fallback.
Adding a recording test for both.
The templates command flattened two kinds of template into one list of verbs, so a command's name never said whether it acted on a project template or a to-do list template. The only way to find out was to run one and read what came back. Basecamp's own template library keeps the kinds apart, and the CLI now does too. Duplicating follows the product, which calls the operation duplicating wherever it appears. The API resource stays a copy. Every spelling that shipped before still works. Keeping them visible rather than hidden is deliberate: the surface snapshot skips hidden commands, so hiding them would record them as removed while they still work.
The address this listing read predates the library holding more than one kind of template. It survives only as a redirect, and the SDK now marks it deprecated, so a newly grouped command would have been built on a call already scheduled to disappear.
The template library has held card table templates since the API gained them, and the CLI could not see them. Someone who had built a board worth reusing could reach it from Basecamp but not from a script. A card table duplicate lands on the destination project rather than in one of its tools, so this command takes no container flag. A project has exactly one dock. This does not build against the released SDK. The operations it needs are not published yet, so the dependency is deliberately left where it was. Bump it before merging.
… board The card table half of the template library could be seeded from the CLI and the to-do list half could not, so someone scripting the library had to open Basecamp for one kind and not the other. The asymmetry was never intended. It existed because the SDK wrapped one endpoint before the other.
Building a template meant starting from an empty one and retyping work that already existed somewhere. Basecamp itself offers this from the list's own menu, and the route is nested under the recording, so the verb belongs on the to-do list rather than in the template library. The save runs asynchronously and hands back an ID to poll, matching how constructing a project and duplicating a template already behave. Polling inside the command would leave someone who interrupted it holding a finished template with no way to ask about it. Templatify and templatification are made-up words, and the only honest ones available. Construct pairs with construction and duplicate with duplication, each naming the async record it polls, and saving work as a template has no plain English verb and noun to take those slots. The help text carries the definition.
Saving existing work as a template reached to-do lists but not card tables, even though the same endpoint serves both. Card tables had no home in the CLI for actions on the table itself: the cards group manages what is inside a board, and its card-table flag only picks which board to look in. Gathering the cards into Triage is offered only here. bc3 accepts that attribute on any recording but never reads it for a to-do list, so a per-group command is what keeps the invalid combination unreachable.
7d91270 to
ec83f76
Compare
The template library holds three kinds of template. Basecamp's own navigation separates them, and the CLI did not:
templates listmeant project templates,templates librarymeant to-do list templates, and nothing in either name said so. You found out by running one and reading what came back.This groups the commands by the kind they act on, adds card table templates, and adds a way to turn work you already have into a template.
The command tree
Nothing that shipped before stops working. Every flat spelling (
templates list,templates show,templates create,templates update,templates delete,templates construct,templates construction,templates library,templates copy,templates copy-status) still resolves, andcopyandcopy-statusalso work as aliases inside the grouped paths.templates librarymaps totemplates todolists list.Naming
Three asynchronous operations now read the same way, each status command named after the record it polls:
duplicatefollows the product, which calls the operation Duplicate everywhere it appears. The API resource stayscopiesand the SDK operation staysCreateLibraryCopy; that separation is deliberate and matches what the front-end already does, where the UI says Duplicate and the route says copies.templatifyandtemplatificationare coined words, and they are the only honest ones available. The product has no name for this operation at all, so rather than borrowing an unrelated English word that would read ambiguously next totodolists' other verbs, the CLI takes the resource noun. The help text carries the definition at the point you meet it.A card table group
There was no home for actions on a card table itself.
cardsmanages what is inside a board, and its--card-tableflag only picks which board to look in.basecamp card-tablesis that home, and it starts with one verb because a command group is a resource identity rather than a subcommand quota.messageboardshas exactly one action for the same reason.Duplicating names the project, not the container
duplicatesends the destination project and Basecamp resolves the container from the template's kind: a to-do list into the project's To-dos tool, a card table onto its dock.--todosetstill pins the container when a project has more than one to-do set, and still checks that it belongs to the project and is enabled, because a clear message beats a 404.card-tables duplicatehas no container flag, because a project has exactly one dock.Verification
Unit tests cover request shape and output rendering. e2e covers error paths, flag surface and help.
Run against a live server with a seeded database:
templatify <id>with no flags sends{}. The template came back carrying the source's title, "Strategy ideas".card-tables templatifyon the same board twice, with and without--move-cards-to-triage. With it, all 7 cards including one under an on-hold container are inKanban::Triage. Without it,Triage 1, Column 4, NotNowColumn 1, OnHold 1. The two results differ, so the flag reaches the wire rather than being dropped.templatificationon both kinds reports a real template name and id, and the matchingdestination_todolist/destination_card_tableis populated, so neither falls through to the generic completion message.templatificationwith its two ids transposed exits 2 withnot_found, rather than reading a different record.templatifyon a recording that cannot be templatified exits 4 withforbidden, which is a different code and shape from the transposed case, so the two stay distinguishable.templates todolists create,templates todolists listandtemplates card-tables listall succeed, and everything created during the run appears in the listings.Depends on
basecamp/basecamp-sdk#877
Three commits use SDK operations added in basecamp-sdk#877 and do not compile against the current release. Bump
go.modbefore merging.