diff --git a/.surface b/.surface index a02c2c4c2..14737e010 100644 --- a/.surface +++ b/.surface @@ -308,6 +308,9 @@ ARG basecamp tlgroups update 00 ARG basecamp tlgroups update 01 ARG basecamp todolist archive 00 ARG basecamp todolist create 00 +ARG basecamp todolist move 00 ... +ARG basecamp todolist position 00 ... +ARG basecamp todolist reorder 00 ... ARG basecamp todolist restore 00 ARG basecamp todolist show 00 ARG basecamp todolist trash 00 @@ -330,6 +333,9 @@ ARG basecamp todolistgroups update 00 ARG basecamp todolistgroups update 01 ARG basecamp todolists archive 00 ARG basecamp todolists create 00 +ARG basecamp todolists move 00 ... +ARG basecamp todolists position 00 ... +ARG basecamp todolists reorder 00 ... ARG basecamp todolists restore 00 ARG basecamp todolists show 00 ARG basecamp todolists trash 00 @@ -896,6 +902,9 @@ CMD basecamp todolist CMD basecamp todolist archive CMD basecamp todolist create CMD basecamp todolist list +CMD basecamp todolist move +CMD basecamp todolist position +CMD basecamp todolist reorder CMD basecamp todolist restore CMD basecamp todolist show CMD basecamp todolist trash @@ -920,6 +929,9 @@ CMD basecamp todolists CMD basecamp todolists archive CMD basecamp todolists create CMD basecamp todolists list +CMD basecamp todolists move +CMD basecamp todolists position +CMD basecamp todolists reorder CMD basecamp todolists restore CMD basecamp todolists show CMD basecamp todolists trash @@ -12482,6 +12494,75 @@ FLAG basecamp todolist list --styled type=bool FLAG basecamp todolist list --todolist type=string FLAG basecamp todolist list --todoset type=string FLAG basecamp todolist list --verbose type=count +FLAG basecamp todolist move --account type=string +FLAG basecamp todolist move --agent type=bool +FLAG basecamp todolist move --cache-dir type=string +FLAG basecamp todolist move --count type=bool +FLAG basecamp todolist move --help type=bool +FLAG basecamp todolist move --hints type=bool +FLAG basecamp todolist move --ids-only type=bool +FLAG basecamp todolist move --in type=string +FLAG basecamp todolist move --jq type=string +FLAG basecamp todolist move --json type=bool +FLAG basecamp todolist move --markdown type=bool +FLAG basecamp todolist move --md type=bool +FLAG basecamp todolist move --no-hints type=bool +FLAG basecamp todolist move --no-stats type=bool +FLAG basecamp todolist move --position type=int +FLAG basecamp todolist move --profile type=string +FLAG basecamp todolist move --project type=string +FLAG basecamp todolist move --quiet type=bool +FLAG basecamp todolist move --stats type=bool +FLAG basecamp todolist move --styled type=bool +FLAG basecamp todolist move --to type=int +FLAG basecamp todolist move --todolist type=string +FLAG basecamp todolist move --verbose type=count +FLAG basecamp todolist position --account type=string +FLAG basecamp todolist position --agent type=bool +FLAG basecamp todolist position --cache-dir type=string +FLAG basecamp todolist position --count type=bool +FLAG basecamp todolist position --help type=bool +FLAG basecamp todolist position --hints type=bool +FLAG basecamp todolist position --ids-only type=bool +FLAG basecamp todolist position --in type=string +FLAG basecamp todolist position --jq type=string +FLAG basecamp todolist position --json type=bool +FLAG basecamp todolist position --markdown type=bool +FLAG basecamp todolist position --md type=bool +FLAG basecamp todolist position --no-hints type=bool +FLAG basecamp todolist position --no-stats type=bool +FLAG basecamp todolist position --position type=int +FLAG basecamp todolist position --profile type=string +FLAG basecamp todolist position --project type=string +FLAG basecamp todolist position --quiet type=bool +FLAG basecamp todolist position --stats type=bool +FLAG basecamp todolist position --styled type=bool +FLAG basecamp todolist position --to type=int +FLAG basecamp todolist position --todolist type=string +FLAG basecamp todolist position --verbose type=count +FLAG basecamp todolist reorder --account type=string +FLAG basecamp todolist reorder --agent type=bool +FLAG basecamp todolist reorder --cache-dir type=string +FLAG basecamp todolist reorder --count type=bool +FLAG basecamp todolist reorder --help type=bool +FLAG basecamp todolist reorder --hints type=bool +FLAG basecamp todolist reorder --ids-only type=bool +FLAG basecamp todolist reorder --in type=string +FLAG basecamp todolist reorder --jq type=string +FLAG basecamp todolist reorder --json type=bool +FLAG basecamp todolist reorder --markdown type=bool +FLAG basecamp todolist reorder --md type=bool +FLAG basecamp todolist reorder --no-hints type=bool +FLAG basecamp todolist reorder --no-stats type=bool +FLAG basecamp todolist reorder --position type=int +FLAG basecamp todolist reorder --profile type=string +FLAG basecamp todolist reorder --project type=string +FLAG basecamp todolist reorder --quiet type=bool +FLAG basecamp todolist reorder --stats type=bool +FLAG basecamp todolist reorder --styled type=bool +FLAG basecamp todolist reorder --to type=int +FLAG basecamp todolist reorder --todolist type=string +FLAG basecamp todolist reorder --verbose type=count FLAG basecamp todolist restore --account type=string FLAG basecamp todolist restore --agent type=bool FLAG basecamp todolist restore --cache-dir type=string @@ -13025,6 +13106,75 @@ FLAG basecamp todolists list --styled type=bool FLAG basecamp todolists list --todolist type=string FLAG basecamp todolists list --todoset type=string FLAG basecamp todolists list --verbose type=count +FLAG basecamp todolists move --account type=string +FLAG basecamp todolists move --agent type=bool +FLAG basecamp todolists move --cache-dir type=string +FLAG basecamp todolists move --count type=bool +FLAG basecamp todolists move --help type=bool +FLAG basecamp todolists move --hints type=bool +FLAG basecamp todolists move --ids-only type=bool +FLAG basecamp todolists move --in type=string +FLAG basecamp todolists move --jq type=string +FLAG basecamp todolists move --json type=bool +FLAG basecamp todolists move --markdown type=bool +FLAG basecamp todolists move --md type=bool +FLAG basecamp todolists move --no-hints type=bool +FLAG basecamp todolists move --no-stats type=bool +FLAG basecamp todolists move --position type=int +FLAG basecamp todolists move --profile type=string +FLAG basecamp todolists move --project type=string +FLAG basecamp todolists move --quiet type=bool +FLAG basecamp todolists move --stats type=bool +FLAG basecamp todolists move --styled type=bool +FLAG basecamp todolists move --to type=int +FLAG basecamp todolists move --todolist type=string +FLAG basecamp todolists move --verbose type=count +FLAG basecamp todolists position --account type=string +FLAG basecamp todolists position --agent type=bool +FLAG basecamp todolists position --cache-dir type=string +FLAG basecamp todolists position --count type=bool +FLAG basecamp todolists position --help type=bool +FLAG basecamp todolists position --hints type=bool +FLAG basecamp todolists position --ids-only type=bool +FLAG basecamp todolists position --in type=string +FLAG basecamp todolists position --jq type=string +FLAG basecamp todolists position --json type=bool +FLAG basecamp todolists position --markdown type=bool +FLAG basecamp todolists position --md type=bool +FLAG basecamp todolists position --no-hints type=bool +FLAG basecamp todolists position --no-stats type=bool +FLAG basecamp todolists position --position type=int +FLAG basecamp todolists position --profile type=string +FLAG basecamp todolists position --project type=string +FLAG basecamp todolists position --quiet type=bool +FLAG basecamp todolists position --stats type=bool +FLAG basecamp todolists position --styled type=bool +FLAG basecamp todolists position --to type=int +FLAG basecamp todolists position --todolist type=string +FLAG basecamp todolists position --verbose type=count +FLAG basecamp todolists reorder --account type=string +FLAG basecamp todolists reorder --agent type=bool +FLAG basecamp todolists reorder --cache-dir type=string +FLAG basecamp todolists reorder --count type=bool +FLAG basecamp todolists reorder --help type=bool +FLAG basecamp todolists reorder --hints type=bool +FLAG basecamp todolists reorder --ids-only type=bool +FLAG basecamp todolists reorder --in type=string +FLAG basecamp todolists reorder --jq type=string +FLAG basecamp todolists reorder --json type=bool +FLAG basecamp todolists reorder --markdown type=bool +FLAG basecamp todolists reorder --md type=bool +FLAG basecamp todolists reorder --no-hints type=bool +FLAG basecamp todolists reorder --no-stats type=bool +FLAG basecamp todolists reorder --position type=int +FLAG basecamp todolists reorder --profile type=string +FLAG basecamp todolists reorder --project type=string +FLAG basecamp todolists reorder --quiet type=bool +FLAG basecamp todolists reorder --stats type=bool +FLAG basecamp todolists reorder --styled type=bool +FLAG basecamp todolists reorder --to type=int +FLAG basecamp todolists reorder --todolist type=string +FLAG basecamp todolists reorder --verbose type=count FLAG basecamp todolists restore --account type=string FLAG basecamp todolists restore --agent type=bool FLAG basecamp todolists restore --cache-dir type=string @@ -16545,6 +16695,9 @@ SUB basecamp todolist SUB basecamp todolist archive SUB basecamp todolist create SUB basecamp todolist list +SUB basecamp todolist move +SUB basecamp todolist position +SUB basecamp todolist reorder SUB basecamp todolist restore SUB basecamp todolist show SUB basecamp todolist trash @@ -16569,6 +16722,9 @@ SUB basecamp todolists SUB basecamp todolists archive SUB basecamp todolists create SUB basecamp todolists list +SUB basecamp todolists move +SUB basecamp todolists position +SUB basecamp todolists reorder SUB basecamp todolists restore SUB basecamp todolists show SUB basecamp todolists trash diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 70acb4411..42589e3b9 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -6,17 +6,17 @@ Coverage of Basecamp 3 API endpoints. Source: [bc3-api/sections](https://github. | Status | Sections | Endpoints | |--------|----------|-----------| -| ✅ Implemented | 43 | 162 | +| ✅ Implemented | 43 | 163 | | ⏭️ Out of scope | 4 | 12 | -| **Total (docs)** | **47** | **174** | +| **Total (docs)** | **47** | **175** | -**100% coverage of in-scope API** (162/162 endpoints) +**100% coverage of in-scope API** (163/163 endpoints) Out-of-scope sections are excluded from parity totals and scripts: chatbots (different auth), legacy Clientside (deprecated) > Note: the per-row `Endpoints` column in the Coverage by Section table sums higher than the Summary totals above. The discrepancy predates the BC5 baseline; the row count (47 sections) is authoritative for the `Since` column. Reconciling endpoint counts is pre-existing maintenance, tracked separately. -**SDK version:** v0.8.0 — adds `Campfires.UpdateLine` (`PUT /chats/{c}/lines/{l}`, basecamp/basecamp-sdk#295), consumed by `chat update`; also carries the BC5 `Tools.Create` create-by-type reshape. API date advanced to 2026-07-22. +**SDK version:** v0.8.1-0.20260724184307-e2c1abea4aea — adds `Todolists.Reposition` (`PUT /todosets/todolists/{id}/position.json`, basecamp/basecamp-sdk#398), consumed by `todolists position`; also carries the create-time `visible_to_clients` support behind `todolists create --visible-to-clients`, `Campfires.UpdateLine` (`PUT /chats/{c}/lines/{l}`, basecamp/basecamp-sdk#295), consumed by `chat update`, and the BC5 `Tools.Create` create-by-type reshape. API date 2026-07-24. ## Coverage by Section @@ -27,7 +27,7 @@ The **Since** column tags each row with the Basecamp version that introduced its | **Core** | | projects | 9 | `projects` | ✅ | BC4 | - | list, show, create, update, delete | | todos | 11 | `todos`, `todo`, `done`, `reopen` | ✅ | BC4 | - | list, show, create, update, complete, uncomplete, position (BC5: `steps` shown on `todos show`; edit via `cards step`) | -| todolists | 8 | `todolists` | ✅ | BC4 | - | list, show, create, update | +| todolists | 9 | `todolists` | ✅ | BC4 | - | list, show, create, update, position | | todosets | 3 | `todosets` | ✅ | BC4 | - | Container for todolists, accessed via project dock (BC5: `todos_count`, `completed_loose_todos_count`, `todos_url`, `app_todos_url`) | | todolist_groups | 8 | `todolistgroups` | ✅ | BC4 | - | list, show, create, update, position | | dock_tools | 7 | `tools` | ✅ | BC4 | - | Dock tool management: show, update, trash, enable, disable, reposition. `create` is BC5-only (create-by-type: `POST /buckets/{id}/dock/tools.json`), replacing the removed clone call | diff --git a/e2e/errors.bats b/e2e/errors.bats index 5eebb7e1b..56b768aea 100644 --- a/e2e/errors.bats +++ b/e2e/errors.bats @@ -348,6 +348,44 @@ load test_helper assert_output_contains "required" } +@test "todolists position without id shows error" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run basecamp todolists position --to 1 + assert_failure + assert_json_value '.error' '... required' + assert_json_value '.code' 'usage' +} + +@test "todolists position without --to shows error" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run basecamp todolists position 123 + assert_failure + assert_output_contains "required" +} + +@test "todolists position multiple ids with --to 2 shows error" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run basecamp todolists position 123 456 --to 2 + assert_failure + assert_output_contains "position 1" +} + +@test "todolists position with only a comma shows error" { + create_credentials + create_global_config '{"account_id": 99999, "project_id": 123}' + + run basecamp todolists position "," + assert_failure + assert_json_value '.error' '... required' + assert_json_value '.code' 'usage' +} + @test "comments without subcommand shows help" { create_credentials create_global_config '{"account_id": 99999, "project_id": 123}' diff --git a/e2e/smoke/smoke_aliases.bats b/e2e/smoke/smoke_aliases.bats index 301b4f248..2b3ab278a 100644 --- a/e2e/smoke/smoke_aliases.bats +++ b/e2e/smoke/smoke_aliases.bats @@ -223,6 +223,16 @@ load smoke_helper mark_out_of_scope "Alias for todos position — tested via canonical form" } +# --- todolists --- + +@test "todolists move is out of scope" { + mark_out_of_scope "Alias for todolists position — tested via canonical form" +} + +@test "todolists reorder is out of scope" { + mark_out_of_scope "Alias for todolists position — tested via canonical form" +} + # --- todolistgroups --- @test "todolistgroups move is out of scope" { diff --git a/e2e/smoke/smoke_todos_write.bats b/e2e/smoke/smoke_todos_write.bats index 56ad9db1a..750980630 100644 --- a/e2e/smoke/smoke_todos_write.bats +++ b/e2e/smoke/smoke_todos_write.bats @@ -237,6 +237,18 @@ setup_file() { echo "$tl_id" > "$BATS_FILE_TMPDIR/todolist_target_id" } +@test "todolists position repositions a todolist" { + local id_file="$BATS_FILE_TMPDIR/todolist_target_id" + [[ -f "$id_file" ]] || mark_unverifiable "No todolist created in prior test" + local tl_id + tl_id=$(<"$id_file") + + # Runs before archive/trash so the target list is still active. + run_smoke basecamp todolists position "$tl_id" --to 1 -p "$QA_PROJECT" --json + assert_success + assert_json_value '.ok' 'true' +} + @test "todolists archive archives a todolist" { local id_file="$BATS_FILE_TMPDIR/todolist_target_id" [[ -f "$id_file" ]] || mark_unverifiable "No todolist created in prior test" diff --git a/internal/commands/commands.go b/internal/commands/commands.go index e8c660dce..f63f2b4ad 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -36,7 +36,7 @@ func CommandCategories() []CommandCategory { Commands: []CommandInfo{ {Name: "projects", Category: "core", Description: "Manage projects", Actions: []string{"list", "show", "create", "update", "delete"}}, {Name: "todos", Category: "core", Description: "Manage to-dos", Actions: []string{"list", "show", "create", "update", "complete", "uncomplete", "position", "trash", "archive", "restore"}}, - {Name: "todolists", Category: "core", Description: "Manage to-do lists", Actions: []string{"list", "show", "create", "update", "trash", "archive", "restore"}}, + {Name: "todolists", Category: "core", Description: "Manage to-do lists", Actions: []string{"list", "show", "create", "update", "position", "trash", "archive", "restore"}}, {Name: "todosets", Category: "core", Description: "Manage to-do set containers", Actions: []string{"list", "show"}}, {Name: "hillcharts", Category: "core", Description: "Manage hill charts", Actions: []string{"show", "track", "untrack"}}, {Name: "gauges", Category: "core", Description: "Manage gauges", Actions: []string{"list", "needles", "needle", "create", "update", "delete", "enable", "disable"}}, diff --git a/internal/commands/todolists.go b/internal/commands/todolists.go index acab87915..9405e69a7 100644 --- a/internal/commands/todolists.go +++ b/internal/commands/todolists.go @@ -1,8 +1,10 @@ package commands import ( + "errors" "fmt" "strconv" + "strings" "github.com/spf13/cobra" @@ -10,6 +12,7 @@ import ( "github.com/basecamp/basecamp-cli/internal/appctx" "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/urlarg" ) // NewTodolistsCmd creates the todolists command group. @@ -36,6 +39,7 @@ to disambiguate when needed.`, newTodolistsShowCmd(&project), newTodolistsCreateCmd(&project, &todosetID), newTodolistsUpdateCmd(&project), + newTodolistsPositionCmd(&project), newRecordableTrashCmd("todolist"), newRecordableArchiveCmd("todolist"), newRecordableRestoreCmd("todolist"), @@ -457,3 +461,237 @@ You can pass either a todolist ID or a Basecamp URL: return cmd } + +func newTodolistsPositionCmd(project *string) *cobra.Command { + var position int + + cmd := &cobra.Command{ + Use: "position ...", + Aliases: []string{"move", "reorder"}, + Short: "Change todolist position", + Long: `Reorder a todolist within its todoset. Position is 1-based (1 = top). + + basecamp todolists position 789 --to 1 + basecamp todolists position https://3.basecamp.com/1/buckets/2/todolists/789 --to 1 + +Pass several todolists to set their order, top to bottom. Bulk reordering always +places them at the top of the todoset; every list must live in the same todoset +and be incomplete (completed lists are positioned separately by Basecamp): + + basecamp todolists position 701 702 703 704 705 + +Positioning is relative and cascades: sibling lists shift to make room, and the +server translates the position for loose to-dos and hidden completed lists. +Confirm with ` + "`basecamp todolists list`" + `.`, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return missingArg(cmd, "...") + } + + app := appctx.FromContext(cmd.Context()) + if app == nil { + return fmt.Errorf("app not initialized") + } + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + // A pasted URL for a different recording type (a todo, card, or a + // collection URL) still extracts a trailing numeric ID, which would + // silently reposition an unrelated todolist. Require any URL arg to + // be a non-collection todolists URL before trusting its ID. + for _, arg := range args { + for _, token := range strings.Split(arg, ",") { + token = strings.TrimSpace(token) + if token == "" { + continue + } + if parsed := urlarg.Parse(token); parsed != nil { + if parsed.RecordingID == "" || parsed.Type != "todolists" || parsed.IsCollection { + return output.ErrUsage("Expected a todolist URL (.../todolists/), or pass a todolist ID.") + } + } + } + } + + // ExtractIDs drops empty segments, so a bare "," yields zero IDs. + ids := extractIDs(args) + if len(ids) == 0 { + return missingArg(cmd, "...") + } + + // Distinguish an omitted flag from an explicit --to 0. + supplied := cmd.Flags().Changed("to") || cmd.Flags().Changed("position") + if supplied && position < 1 { + return output.ErrUsage("--to must be at least 1 (1 = top)") + } + switch { + case len(ids) == 1 && !supplied: + return output.ErrUsage("--to is required (1 = top)") + case len(ids) > 1 && supplied && position != 1: + return output.ErrUsage("Reordering multiple todolists is only supported at position 1; " + + "drop --to or pass --to 1") + case !supplied: + position = 1 // bulk default + } + + // Validate every ID and reject duplicates before mutating anything. + parsed := make([]int64, 0, len(ids)) + seen := make(map[int64]bool, len(ids)) + for _, idStr := range ids { + id, err := strconv.ParseInt(idStr, 10, 64) + if err != nil { + return output.ErrUsage("Invalid todolist ID") + } + if seen[id] { + return output.ErrUsage(fmt.Sprintf("Duplicate todolist ID %d", id)) + } + seen[id] = true + parsed = append(parsed, id) + } + + // Sibling preflight (bulk only): every list must belong to the same + // todoset and bucket, and be incomplete, before any PUT is issued. + var bucketID, parentID int64 + if len(parsed) > 1 { + for _, id := range parsed { + tl, err := app.Account().Todolists().Get(cmd.Context(), id) + if err != nil { + return convertSDKError(err) + } + if tl.Parent == nil || tl.Bucket == nil { + return output.ErrUsageHint( + fmt.Sprintf("Todolist #%d is missing its todoset or project context.", id), + "Reorder one todoset at a time.", + ) + } + if tl.Completed { + return output.ErrUsageHint( + fmt.Sprintf("Todolist #%d (%s) is completed; bulk reordering accepts incomplete lists only.", id, tl.Name), + "Basecamp positions completed lists separately — reorder the incomplete lists on their own.", + ) + } + if parentID == 0 { + parentID = tl.Parent.ID + bucketID = tl.Bucket.ID + } else if tl.Parent.ID != parentID || tl.Bucket.ID != bucketID { + return output.ErrUsageHint( + fmt.Sprintf("Todolist #%d belongs to a different todoset (#%d) than the others.", id, tl.Parent.ID), + "Reorder one todoset at a time.", + ) + } + } + } + + // Apply in reverse so the typed order lands top→bottom onto position 1. + // Stop at the first failure: a half-applied reorder is a wrong order. + applied := 0 + for i := len(parsed) - 1; i >= 0; i-- { + if err := app.Account().Todolists().Reposition(cmd.Context(), parsed[i], position); err != nil { + converted := convertSDKError(err) + // No PUT has landed yet (single-list mode always takes this + // path): the todoset is untouched. Surface the underlying + // error unchanged — keep its code/category/hint and don't + // claim an intermediate order that doesn't exist. + if applied == 0 { + return converted + } + // A partial reorder has landed: the todoset is now in an + // intermediate order. Add applied-count accounting and a + // rerun hint while preserving the underlying error's + // classification (never reclassify a transport/runtime + // failure as a usage error). + msg := fmt.Sprintf("Reordered %d of %d todolists; failed at #%d", applied, len(parsed), parsed[i]) + const rerun = "Rerun the whole command once the cause is fixed; the todoset is now in an intermediate order." + var outErr *output.Error + if errors.As(converted, &outErr) { + hint := rerun + if outErr.Hint != "" { + hint = outErr.Hint + " " + rerun + } + return &output.Error{ + Code: outErr.Code, + Message: fmt.Sprintf("%s: %s", msg, outErr.Message), + Hint: hint, + HTTPStatus: outErr.HTTPStatus, + Retryable: outErr.Retryable, + Cause: outErr, + } + } + // Raw transport/runtime failure (e.g. a dropped connection or + // a canceled context): no *output.Error to preserve, but the + // partial reorder still demands the rerun warning. Wrap it in + // a non-usage structured error that carries the hint and the + // raw cause (Unwrap keeps errors.Is/As working). + return &output.Error{ + Code: output.CodeAPI, + Message: fmt.Sprintf("%s: %s", msg, converted.Error()), + Hint: rerun, + Cause: converted, + } + } + applied++ + } + + var summary string + var breadcrumbs []output.Breadcrumb + if len(parsed) == 1 { + summary = fmt.Sprintf("Moved todolist #%d to position %d", parsed[0], position) + // No preflight Get in single mode: resolve project from ambient + // context (URL > group flag > flags > config) for the breadcrumbs. + _, urlProjectID := extractWithProject(args[0]) + proj := urlProjectID + if proj == "" { + proj = *project + } + if proj == "" { + proj = app.Flags.Project + } + if proj == "" { + proj = app.Config.ProjectID + } + if proj != "" { + breadcrumbs = []output.Breadcrumb{ + { + Action: "show", + Cmd: fmt.Sprintf("basecamp todolists show %d --in %s", parsed[0], proj), + Description: "View todolist", + }, + { + Action: "list", + Cmd: fmt.Sprintf("basecamp todolists list --in %s", proj), + Description: "List todolists", + }, + } + } + } else { + summary = fmt.Sprintf("Reordered %d todolists to the top of the todoset", len(parsed)) + // Build from authoritative preflight data, not ambient project. + breadcrumbs = []output.Breadcrumb{ + { + Action: "list", + Cmd: fmt.Sprintf("basecamp todolists list --in %d --todoset %d", bucketID, parentID), + Description: "List todolists", + }, + } + } + + opts := []output.ResponseOption{output.WithSummary(summary)} + if len(breadcrumbs) > 0 { + opts = append(opts, output.WithBreadcrumbs(breadcrumbs...)) + } + + return app.OK(map[string]any{ + "repositioned": true, + "position": position, + "todolist_ids": parsed, + }, opts...) + }, + } + + cmd.Flags().IntVar(&position, "to", 0, "Target position, 1-based (1 = top)") + cmd.Flags().IntVar(&position, "position", 0, "Target position (alias for --to)") + + return cmd +} diff --git a/internal/commands/todolists_test.go b/internal/commands/todolists_test.go index 95b9e6db1..32441096d 100644 --- a/internal/commands/todolists_test.go +++ b/internal/commands/todolists_test.go @@ -1,17 +1,747 @@ package commands import ( + "bytes" "encoding/json" "errors" + "fmt" "io" "net/http" + "net/http/httptest" "strings" + "sync" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/auth" + "github.com/basecamp/basecamp-cli/internal/config" + "github.com/basecamp/basecamp-cli/internal/names" + "github.com/basecamp/basecamp-cli/internal/output" ) +// --- A. Guard tests (no-network transport, guards fire before any request) --- + +func TestTodolistsPositionSingleRequiresTo(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + app, _ := setupTestApp(t) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "789") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Equal(t, "--to is required (1 = top)", e.Message) +} + +func TestTodolistsPositionRejectsZeroAndNegative(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + + cases := []struct { + name string + args []string + }{ + {"single --to 0", []string{"789", "--to", "0"}}, + {"single --to -1", []string{"789", "--to", "-1"}}, + {"bulk --to 0", []string{"701", "702", "--to", "0"}}, + {"bulk --to -1", []string{"701", "702", "--to", "-1"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + app, _ := setupTestApp(t) + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, tc.args...) + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Equal(t, "--to must be at least 1 (1 = top)", e.Message) + }) + } +} + +func TestTodolistsPositionAcceptsPositionAlias(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + app, _ := setupTestApp(t) + + project := "" + cmd := newTodolistsPositionCmd(&project) + // --position 2 satisfies the requirement → guard does not fire; the command + // reaches the network layer and fails there, not with the usage message. + err := executeCommand(cmd, app, "789", "--position", "2") + + require.NotNil(t, err) + var e *output.Error + if errors.As(err, &e) { + assert.NotEqual(t, "--to is required (1 = top)", e.Message) + assert.NotEqual(t, "--to must be at least 1 (1 = top)", e.Message) + } +} + +func TestTodolistsPositionBulkRejectsNonOnePosition(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + app, _ := setupTestApp(t) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702", "--to", "2") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "only supported at position 1") +} + +func TestTodolistsPositionBulkAllowsOmittedAndExplicitOne(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + + for _, args := range [][]string{ + {"701", "702"}, + {"701", "702", "--to", "1"}, + } { + app, _ := setupTestApp(t) + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, args...) + + // Guard must not fire: the error comes from the preflight network call. + require.NotNil(t, err) + var e *output.Error + if errors.As(err, &e) { + assert.NotContains(t, e.Message, "only supported at position 1") + assert.NotEqual(t, "--to is required (1 = top)", e.Message) + } + } +} + +func TestTodolistsPositionEmptyAfterExtraction(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + app, _ := setupTestApp(t) + + project := "" + cmd := newTodolistsPositionCmd(&project) + // A bare comma extracts to zero IDs — must report the missing-arg usage error. + err := executeCommand(cmd, app, ",", "--to", "1") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Equal(t, "... required", e.Message) +} + +func TestTodolistsPositionInvalidID(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + app, _ := setupTestApp(t) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "abc", "--to", "1") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Equal(t, "Invalid todolist ID", e.Message) +} + +func TestTodolistsPositionDuplicateID(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + app, _ := setupTestApp(t) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "701") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "Duplicate todolist ID 701") +} + +func TestTodolistsPositionURLArgPassesValidation(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + app, _ := setupTestApp(t) + + project := "" + cmd := newTodolistsPositionCmd(&project) + // A todolist URL should extract its ID and get past validation, reaching the + // network layer (which fails) — not a "Invalid todolist ID" usage error. + err := executeCommand(cmd, app, + "https://3.basecamp.com/99999/buckets/2/todolists/789", "--to", "1") + + require.NotNil(t, err) + var e *output.Error + if errors.As(err, &e) { + assert.NotEqual(t, "Invalid todolist ID", e.Message) + } +} + +func TestTodolistsPositionRejectsWrongURLType(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + + cases := []struct { + name string + url string + }{ + {"todo URL", "https://3.basecamp.com/99999/buckets/2/todos/789"}, + {"card URL", "https://3.basecamp.com/99999/buckets/2/card_tables/cards/789"}, + {"todolists collection URL", "https://3.basecamp.com/99999/buckets/2/todolists"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + app, _ := setupTestApp(t) + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, tc.url, "--to", "1") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "Expected a todolist URL") + }) + } +} + +// --- B. Request-level tests (httptest server records methods/paths/bodies) --- + +type recordedRequest struct { + method string + path string + body map[string]any +} + +type requestRecorder struct { + mu sync.Mutex + requests []recordedRequest +} + +func (r *requestRecorder) record(req *http.Request) { + r.mu.Lock() + defer r.mu.Unlock() + var body map[string]any + if req.Body != nil { + data, _ := io.ReadAll(req.Body) + if len(data) > 0 { + _ = json.Unmarshal(data, &body) + } + } + r.requests = append(r.requests, recordedRequest{ + method: req.Method, + path: req.URL.Path, + body: body, + }) +} + +func (r *requestRecorder) puts() []recordedRequest { + r.mu.Lock() + defer r.mu.Unlock() + var out []recordedRequest + for _, req := range r.requests { + if req.method == http.MethodPut { + out = append(out, req) + } + } + return out +} + +// todolistGetResponse describes a canned Get response for a todolist ID. +type todolistGetResponse struct { + status int + parentID int64 + bucketID int64 + completed bool + nilParent bool + nilBucket bool +} + +// putHijack is a sentinel putStatus value that makes the server drop the +// connection mid-request, forcing a raw transport error. +const putHijack = -1 + +// newTodolistsPositionServer builds an httptest server. getResponses maps a +// todolist ID to its Get response; putStatus maps a todolist ID to the status +// its reposition PUT returns (defaults to 204 when absent). A putStatus value of +// putHijack simulates a transport failure by dropping the connection before any +// response, so the SDK surfaces a raw (non-*basecamp.Error) error. +func newTodolistsPositionServer(t *testing.T, rec *requestRecorder, getResponses map[int64]todolistGetResponse, putStatus map[int64]int) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rec.record(r) + w.Header().Set("Content-Type", "application/json") + + if r.Method == http.MethodPut && strings.Contains(r.URL.Path, "/position.json") { + id := idFromPositionPath(r.URL.Path) + status := http.StatusNoContent + if s, ok := putStatus[id]; ok { + status = s + } + if status == putHijack { + // Drop the connection mid-request → client sees a transport + // error, not an HTTP status. + hj, ok := w.(http.Hijacker) + require.True(t, ok, "test server must support hijacking") + conn, _, err := hj.Hijack() + require.NoError(t, err) + _ = conn.Close() + return + } + w.WriteHeader(status) + if status >= 400 { + // error_description becomes the SDK error Hint — lets tests + // assert the underlying hint is preserved, not overwritten. + fmt.Fprint(w, `{"error":"denied","error_description":"check your role"}`) + } + return + } + + if r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/todolists/") { + id := idFromGetPath(r.URL.Path) + resp, ok := getResponses[id] + if !ok { + w.WriteHeader(http.StatusNotFound) + fmt.Fprintf(w, `{"error":"not found"}`) + return + } + if resp.status != 0 && resp.status != http.StatusOK { + w.WriteHeader(resp.status) + fmt.Fprintf(w, `{"error":"not found"}`) + return + } + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, todolistJSON(id, resp)) + return + } + + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{}`) + })) + t.Cleanup(server.Close) + return server +} + +func idFromPositionPath(path string) int64 { + // .../todosets/todolists/{id}/position.json + parts := strings.Split(path, "/") + for i, p := range parts { + if p == "todolists" && i+1 < len(parts) { + var id int64 + fmt.Sscanf(parts[i+1], "%d", &id) + return id + } + } + return 0 +} + +func idFromGetPath(path string) int64 { + // .../todolists/{id} + parts := strings.Split(path, "/") + last := parts[len(parts)-1] + last = strings.TrimSuffix(last, ".json") + var id int64 + fmt.Sscanf(last, "%d", &id) + return id +} + +func todolistJSON(id int64, resp todolistGetResponse) string { + parent := fmt.Sprintf(`,"parent":{"id":%d,"title":"Set","type":"Todoset"}`, resp.parentID) + if resp.nilParent { + parent = "" + } + bucket := fmt.Sprintf(`,"bucket":{"id":%d,"name":"Proj","type":"Project"}`, resp.bucketID) + if resp.nilBucket { + bucket = "" + } + return fmt.Sprintf(`{"id":%d,"name":"List %d","completed":%t%s%s}`, + id, id, resp.completed, parent, bucket) +} + +// newRequestLevelApp wires an app to a live test server URL. +func newRequestLevelApp(t *testing.T, serverURL string) (*appctx.App, *bytes.Buffer) { + t.Helper() + t.Setenv("BASECAMP_NO_KEYRING", "1") + + buf := &bytes.Buffer{} + cfg := &config.Config{AccountID: "99999"} + authMgr := auth.NewManager(cfg, nil) + sdkClient := basecamp.NewClient( + &basecamp.Config{BaseURL: serverURL}, + &testTokenProvider{}, + basecamp.WithMaxRetries(1), + // Keep any retry backoff near-instant so transport-failure tests + // don't stall on exponential waits. + basecamp.WithBaseDelay(time.Millisecond), + basecamp.WithMaxJitter(0), + ) + nameResolver := names.NewResolver(sdkClient, authMgr, cfg.AccountID) + + app := &appctx.App{ + Config: cfg, + Auth: authMgr, + SDK: sdkClient, + Names: nameResolver, + Output: output.New(output.Options{Format: output.FormatJSON, Writer: buf}), + } + app.Flags.Hints = true // emit breadcrumbs (stripped otherwise) + return app, buf +} + +func TestTodolistsPositionBulkReverseOrderAndBodies(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 10, bucketID: 20}, + 702: {parentID: 10, bucketID: 20}, + 703: {parentID: 10, bucketID: 20}, + } + server := newTodolistsPositionServer(t, rec, gets, nil) + app, buf := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702", "703") + require.NoError(t, err) + + puts := rec.puts() + require.Len(t, puts, 3) + // Reverse apply order: 703, 702, 701; each body {"position":1}. + assert.Equal(t, int64(703), idFromPositionPath(puts[0].path)) + assert.Equal(t, int64(702), idFromPositionPath(puts[1].path)) + assert.Equal(t, int64(701), idFromPositionPath(puts[2].path)) + for _, p := range puts { + assert.Contains(t, p.path, "/todosets/todolists/") + assert.Equal(t, float64(1), p.body["position"]) + } + + // Output shape: typed top→bottom order, not reversed. + env := decodeEnvelope(t, buf) + assert.True(t, env.OK) + assert.Equal(t, true, env.Data["repositioned"]) + assert.Equal(t, float64(1), env.Data["position"]) + assert.Equal(t, []any{float64(701), float64(702), float64(703)}, env.Data["todolist_ids"]) +} + +func TestTodolistsPositionSingleBody(t *testing.T) { + rec := &requestRecorder{} + server := newTodolistsPositionServer(t, rec, nil, nil) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "789", "--to", "3") + require.NoError(t, err) + + puts := rec.puts() + require.Len(t, puts, 1) + assert.Equal(t, int64(789), idFromPositionPath(puts[0].path)) + assert.Equal(t, float64(3), puts[0].body["position"]) +} + +func TestTodolistsPositionRejectsDifferentParents(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 10, bucketID: 20}, + 702: {parentID: 11, bucketID: 20}, + } + server := newTodolistsPositionServer(t, rec, gets, nil) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "different todoset") + assert.Empty(t, rec.puts(), "no PUT should be issued when preflight fails") +} + +func TestTodolistsPositionRejectsNilParentOrBucket(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 10, bucketID: 20}, + 702: {nilParent: true, bucketID: 20}, + } + server := newTodolistsPositionServer(t, rec, gets, nil) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "missing its todoset or project context") + assert.Empty(t, rec.puts()) +} + +func TestTodolistsPositionRejectsCompletedList(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 10, bucketID: 20}, + 702: {parentID: 10, bucketID: 20, completed: true}, + } + server := newTodolistsPositionServer(t, rec, gets, nil) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "#702") + assert.Contains(t, e.Message, "completed") + assert.Empty(t, rec.puts()) +} + +func TestTodolistsPositionPreflightFailureMeansZeroMutations(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 10, bucketID: 20}, + 702: {status: http.StatusNotFound}, + } + server := newTodolistsPositionServer(t, rec, gets, nil) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702") + + require.NotNil(t, err) + assert.Empty(t, rec.puts(), "a preflight Get failure must issue no PUTs") +} + +func TestTodolistsPositionPartialFailureAccounting(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 10, bucketID: 20}, + 702: {parentID: 10, bucketID: 20}, + 703: {parentID: 10, bucketID: 20}, + } + // Apply order is 703, 702, 701. Fail on 702 (the second PUT). + server := newTodolistsPositionServer(t, rec, gets, map[int64]int{702: http.StatusForbidden}) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702", "703") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + // One applied (703) before failing at 702. + assert.Contains(t, e.Message, "Reordered 1 of 3 todolists; failed at #702") + // Underlying error classification is preserved, not reclassified as usage. + assert.Equal(t, "forbidden", e.Code) + assert.Equal(t, http.StatusForbidden, e.HTTPStatus) + // Both the original SDK hint and the rerun hint survive. + assert.Contains(t, e.Hint, "check your role") + assert.Contains(t, e.Hint, "Rerun the whole command") + + // Exactly two PUTs recorded: it stopped instead of continuing to 701. + assert.Len(t, rec.puts(), 2) +} + +// A single-list PUT failure means zero mutations: the error is surfaced with +// its own classification, with no accounting prefix and no false claim that the +// todoset is in an intermediate order. +func TestTodolistsPositionSingleFailureSurfacesUnderlyingError(t *testing.T) { + rec := &requestRecorder{} + server := newTodolistsPositionServer(t, rec, nil, map[int64]int{789: http.StatusNotFound}) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "789", "--to", "3") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Equal(t, "not_found", e.Code, "must not be reclassified as usage") + assert.Equal(t, http.StatusNotFound, e.HTTPStatus) + assert.NotContains(t, e.Message, "Reordered") + assert.NotContains(t, e.Hint, "intermediate order") + assert.Len(t, rec.puts(), 1) +} + +// When the very first applied PUT in a bulk run fails, nothing has been +// mutated yet — the error is surfaced directly, with no accounting prefix and +// no intermediate-order claim, and exactly one PUT is recorded. +func TestTodolistsPositionBulkFirstPutFailureMeansZeroApplied(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 10, bucketID: 20}, + 702: {parentID: 10, bucketID: 20}, + } + // Apply order is 702 then 701; fail on the first PUT (702). + server := newTodolistsPositionServer(t, rec, gets, map[int64]int{702: http.StatusNotFound}) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Equal(t, "not_found", e.Code) + assert.NotContains(t, e.Message, "Reordered") + assert.NotContains(t, e.Hint, "intermediate order") + assert.Len(t, rec.puts(), 1, "stopped after the first failing PUT") +} + +// A raw transport failure after a successful PUT must still warn that the +// todoset is partially reordered: non-usage code, applied-count accounting, the +// rerun hint, and the raw cause preserved. +func TestTodolistsPositionPartialTransportFailureWarnsIntermediate(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 10, bucketID: 20}, + 702: {parentID: 10, bucketID: 20}, + 703: {parentID: 10, bucketID: 20}, + } + // Apply order is 703, 702, 701. 703 succeeds; 702's connection is dropped. + server := newTodolistsPositionServer(t, rec, gets, map[int64]int{702: putHijack}) + app, _ := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702", "703") + + require.NotNil(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.NotEqual(t, "usage", e.Code, "transport failure must not be classified as usage") + assert.Contains(t, e.Message, "Reordered 1 of 3 todolists; failed at #702") + assert.Contains(t, e.Hint, "intermediate order", "operator must be warned of the partial reorder") + assert.NotNil(t, e.Cause, "raw transport cause preserved") + // The transport failure may be retried, so assert on which lists were + // touched rather than a raw request count: 703 applied, 702 attempted, + // and 701 never reached (the loop stopped). + touched := map[int64]bool{} + for _, p := range rec.puts() { + touched[idFromPositionPath(p.path)] = true + } + assert.True(t, touched[703], "703 should have been repositioned") + assert.True(t, touched[702], "702 should have been attempted") + assert.False(t, touched[701], "701 must not be attempted after the failure") +} + +func TestTodolistsPositionBulkBreadcrumbUsesPreflightData(t *testing.T) { + rec := &requestRecorder{} + gets := map[int64]todolistGetResponse{ + 701: {parentID: 55, bucketID: 66}, + 702: {parentID: 55, bucketID: 66}, + } + server := newTodolistsPositionServer(t, rec, gets, nil) + app, buf := newRequestLevelApp(t, server.URL) + // Configure an unrelated project — the breadcrumb must ignore it. + app.Config.ProjectID = "999" + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "701", "702") + require.NoError(t, err) + + env := decodeEnvelope(t, buf) + require.NotEmpty(t, env.Breadcrumbs) + joined := breadcrumbCmds(env) + assert.Contains(t, joined, "--in 66") + assert.Contains(t, joined, "--todoset 55") + assert.NotContains(t, joined, "--in 999") +} + +func TestTodolistsPositionSingleBreadcrumbContextKnown(t *testing.T) { + rec := &requestRecorder{} + server := newTodolistsPositionServer(t, rec, nil, nil) + app, buf := newRequestLevelApp(t, server.URL) + + project := "myproj" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "789", "--to", "1") + require.NoError(t, err) + + env := decodeEnvelope(t, buf) + require.NotEmpty(t, env.Breadcrumbs) + joined := breadcrumbCmds(env) + assert.Contains(t, joined, "basecamp todolists show 789 --in myproj") + assert.Contains(t, joined, "basecamp todolists list --in myproj") +} + +func TestTodolistsPositionSingleBreadcrumbProjectFromURL(t *testing.T) { + rec := &requestRecorder{} + server := newTodolistsPositionServer(t, rec, nil, nil) + app, buf := newRequestLevelApp(t, server.URL) + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, + "https://3.basecamp.com/99999/buckets/42/todolists/789", "--to", "1") + require.NoError(t, err) + + env := decodeEnvelope(t, buf) + require.NotEmpty(t, env.Breadcrumbs) + joined := breadcrumbCmds(env) + assert.Contains(t, joined, "--in 42") +} + +func TestTodolistsPositionSingleBreadcrumbContextAbsent(t *testing.T) { + rec := &requestRecorder{} + server := newTodolistsPositionServer(t, rec, nil, nil) + app, buf := newRequestLevelApp(t, server.URL) + // No project anywhere. + + project := "" + cmd := newTodolistsPositionCmd(&project) + err := executeCommand(cmd, app, "789", "--to", "1") + require.NoError(t, err) + + // Exactly one request: the PUT. No project-resolution GETs. + require.Len(t, rec.requests, 1) + assert.Equal(t, http.MethodPut, rec.requests[0].method) + + env := decodeEnvelope(t, buf) + assert.Empty(t, env.Breadcrumbs, "no breadcrumbs when no project context is known") +} + +// --- helpers --- + +type testEnvelope struct { + OK bool `json:"ok"` + Data map[string]any `json:"data"` + Summary string `json:"summary"` + Breadcrumbs []map[string]any `json:"breadcrumbs"` +} + +func decodeEnvelope(t *testing.T, buf *bytes.Buffer) testEnvelope { + t.Helper() + var env testEnvelope + require.NoError(t, json.Unmarshal(buf.Bytes(), &env)) + return env +} + +func breadcrumbCmds(env testEnvelope) string { + var b strings.Builder + for _, bc := range env.Breadcrumbs { + if cmd, ok := bc["cmd"].(string); ok { + b.WriteString(cmd) + b.WriteString("\n") + } + } + return b.String() +} + +// --- Client-visibility create tests (from #554) --- + // mockTodolistCreateTransport resolves the todoset via the project dock and // captures the POST body sent to create a todolist. type mockTodolistCreateTransport struct { diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 913284ae1..1d7c421e3 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -559,8 +559,13 @@ basecamp todolists create "Name" --in --json # Create basecamp todolists create "Name" --description "Desc" --in basecamp todolists create "Name" --visible-to-clients --in # Visible to clients basecamp todolists update --name "New" --in # Update +basecamp todolists position --to 1 # Reorder one list (1 = top) +basecamp todolists position # Order incomplete lists, top→bottom ``` +Bulk `position` sets the visible order in one command: pass incomplete lists from +the same todoset, top to bottom. It always places them at the top. + ### Cards (Kanban) **Note:** Cards do NOT support `--assignee` filtering like todos. Fetch all cards and filter client-side if needed. If a project has multiple card tables, you must specify `--card-table `. When you get an "Ambiguous card table" error, the hint shows available table IDs and names.