Add todolists position to reorder to-do lists (#484) - #562
Merged
Conversation
github-actions
Bot
added
commands
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Adds a new basecamp todolists position <id|url>... command to reorder to-do lists within a todoset, aligning the todolist surface area with existing todos position / todolistgroups position capabilities and the SDK’s new TodolistsService.Reposition.
Changes:
- Add
todolists position(withmove/reorderaliases) including bulk reordering with same-todoset + incomplete-only preflight checks. - Bump Basecamp SDK dependency/provenance to include
TodolistsService.Reposition, and update API coverage + CLI surface metadata accordingly. - Add unit + e2e coverage for the new command behavior and aliases.
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.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
internal/commands/todolists.go |
Implements todolists position including single + bulk ordering logic, preflight validation, and output/breadcrumbs. |
internal/commands/todolists_test.go |
Adds unit coverage for guards, preflight validation, reverse-apply semantics, and output shape. |
e2e/errors.bats |
Adds CLI usage-error coverage for missing args / missing --to / invalid bulk --to. |
e2e/smoke/smoke_todos_write.bats |
Adds a smoke test that exercises todolists position against a live-ish environment. |
e2e/smoke/smoke_aliases.bats |
Marks todolists move/reorder aliases out-of-scope (covered via canonical command). |
internal/commands/commands.go |
Exposes position in the todolists command catalog entry. |
go.mod |
Updates SDK dependency to the pseudo-version containing TodolistsService.Reposition. |
go.sum |
Records checksums for the updated SDK pseudo-version. |
internal/version/sdk-provenance.json |
Updates recorded SDK module version/revision/timestamp. |
API-COVERAGE.md |
Updates in-scope endpoint counts and notes the new todolist reposition endpoint coverage. |
skills/basecamp/SKILL.md |
Documents the new command usage patterns for the skill surface. |
.surface |
Updates the command/flag surface snapshot for todolists position and aliases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://cdn.github.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 12 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Adds `basecamp todolists position <id|url>...` to reorder a to-do list
within its todoset — the one repositioning gap in the to-do family
(`todos` and `todolistgroups` already had it).
Single-list mode sets one explicit position (`--to N`, 1-based). Bulk
mode accepts several incomplete lists from the same todoset and sets
their visible order top-to-bottom; it is restricted to position 1
because reverse-iterating onto any other slot cannot reconstruct the
typed order once loose to-dos or hidden completed lists shift the base.
Bulk preflight enforces a shared todoset/bucket and rejects completed
lists before issuing any PUT, and application stops at the first failure
(a half-applied reorder is a wrong order), reporting applied-vs-attempted.
Uses the dedicated `PUT /todosets/todolists/{id}/position.json` route,
not the generic `/recordings/{id}/position.json` the issue's workaround
used — the generic one skips the to-do-list position math and mis-places
lists when loose to-dos or hidden completed lists exist.
Naming follows the to-do family (`position`, aliases `move`/`reorder`,
`--to` leading with `--position` accepted); a to-do list has no
destination container, so `--to N` is unambiguously an index.
SDK bumped to 93ace8d5 (basecamp-sdk#398), which also absorbs #396
(Templates.CreateProject envelope fix, signature unchanged) and #392
(SDK tests only).
Closes #484
The bulk-apply failure path reclassified every non-*output.Error (raw transport/runtime failures, context cancellation) as code=usage via ErrUsageHint, overwrote the SDK's structured hint with the rerun hint, and claimed the todoset was left in an intermediate order even when the failing PUT was the first/only one and nothing had been mutated. Now: when zero PUTs have landed (always true in single-list mode, and when a bulk run's first PUT fails), surface the underlying error unchanged — its code, HTTP status, and hint intact, no false intermediate-order claim. Only once a partial reorder has actually landed does it add the applied-count accounting and rerun hint, and even then it preserves the underlying error's classification (appending the rerun text to the original hint rather than replacing it) and wraps raw errors with %w instead of reclassifying them as usage. Adds regression coverage for Code/HTTPStatus preservation, original-hint preservation, and the zero-applied single-list and bulk-first-PUT paths.
A pasted Basecamp URL for a different recording type (a todo, a card, or a todolists *collection* URL) still yields a trailing numeric ID via ExtractIDs, which — because this command mutates — would silently reposition an unrelated todolist that happens to share that ID. Validate any URL argument up front: it must be a non-collection `todolists` URL, mirroring the guard todos position already applies to its --list destination URL. Bare numeric IDs are unaffected.
The raw-error branch of the partial-failure path wrapped the cause with %w and the applied-count prefix, but dropped the recovery hint entirely. After one successful PUT followed by a transport failure (a dropped connection, a canceled context), the operator saw the count but was never told the todoset was left partially reordered and needs a rerun. Return a non-usage structured error that carries the applied-count message, the rerun/intermediate-order hint, and the raw cause (via Cause, so errors.Is/As still unwrap). Code is api_error, never usage. Adds a regression test that drops the connection on the second applied PUT and asserts the non-usage code, the accounting message, the rerun hint, the preserved cause, and that the third list is never touched.
Rebasing onto main (#554) advanced the SDK pin to e2c1abea, five commits past the 93ace8d5 this branch first pinned. Refresh the API-COVERAGE SDK line to the merged pin (which additionally carries create-time visible_to_clients support) and regenerate the CLI surface so it reflects both todolists position and main's todolists create --visible-to-clients.
jeremy
force-pushed
the
todolists-reposition
branch
from
July 24, 2026 21:56
55156d9 to
fed0899
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #484.
Adds
basecamp todolists position <id|url>...to reorder a to-do list within its todoset.todosandtodolistgroupsalready hadposition; to-do lists could only be created/updated/archived/restored/trashed, so a multi-phase plan displayed newest-first with no CLI fix short of the rawapipassthrough looped in reverse.Behavior
basecamp todolists position 789 --to 1— one explicit 1-based position (1 = top).--torequired.basecamp todolists position 701 702 703 704 705— sets the visible order top-to-bottom. Accepts several incomplete lists from the same todoset; always places them at the top (position 1).ID / URL args only, matching
todolists show/update(no name resolution, no--todoset).Why the restrictions
1; for a baseN > 1, whether a list lands above or below the block depends on where it started, on the server'sposition_offsetfor loose to-dos, and on hidden completed lists.2+IDs with any--toother than 1 is a usage error.Gets every list and rejects anyCompleted == true(naming it) before issuing any PUT.Parent.ID/Bucket.ID(and non-nil parent/bucket) before mutating, so lists from different todosets aren't independently reordered while the output claims one order.Reordered 1 of 3 todolists; failed at #702), hinting to rerun the whole command.Dedicated route (please don't "simplify" this)
Uses
PUT /todosets/todolists/{id}/position.json, not the generic/recordings/{id}/position.jsonthe issue's workaround used. The generic route skips the to-do-list position math and mis-places lists when loose to-dos or hidden completed lists exist.Naming
Follows the to-do family: canonical
position, aliasesmove/reorder,--toleading in help with--positionaccepted for parity. A to-do list has no destination container, so--to Nis unambiguously an index (no clash withcards move --to <column>).SDK
Bumped to
93ace8d5(basecamp-sdk#398,TodolistsService.Reposition) as a pseudo-version — no wait for av0.8.1tag. The same span also absorbs, both benign:Templates.CreateProjectrequest-envelope fix; internal, signature unchanged, no CLI edit.Only one new SDK service method arrived, so the completeness bar is satisfied by this one command.
Tests
internal/commands/todolists_test.go): guard tests + request-level tests asserting reverse apply order and{"position":1}bodies, single-list body, same-parent/nil/completed rejection with zero PUTs, preflight-failure = zero mutations, partial-failure accounting (exactly two PUTs,Code/HTTPStatuspreserved), output shape, and every breadcrumb variant.errors.bats), a smoke test (smoke_todos_write.bats, run before archive so the target is active), and alias OOS markers (smoke_aliases.bats).API-COVERAGE.md, catalog,SKILL.md, and.surfaceupdated. Fullbin/cigreen.Summary by cubic
Adds
basecamp todolists positionto reorder to‑do lists within a todoset, covering the last missing reposition action in the to‑do family. Supports single moves and a safe bulk top‑of‑list reorder, with improved error handling that preserves underlying codes and warns on partial reorders. Closes #484.New Features
basecamp todolists position <id|url>...(aliases:move,reorder).--to Nrequired (1-based).todolistsURLs; rejectstodo/card/collection URLs.PUT /todosets/todolists/{id}/position.json; adds breadcrumbs when project context is known.Dependencies
github.com/basecamp/basecamp-sdk/gotov0.8.1-0.20260724184307-e2c1abea4aeaaddingTodolists.Reposition. Also includes create-timevisible_to_clientssupport fortodolists create, and a templates request-envelope fix (no CLI behavior change here).Written for commit fed0899. Summary will update on new commits.