Conversation
github-actions
bot
added
commands
There was a problem hiding this comment.
Pull request overview
This PR extends multi-todoset project support across name resolution, TUI/CLI resolution, and the CLI surface so users can reliably scope todolist selection/creation when projects contain multiple todosets.
Changes:
- Update names resolver todolist fetching to merge results across all todoset dock entries and fail on partial fetch errors.
- Thread an explicit todoset ID through todolist resolution (TUI resolver + CLI helpers) and add
--todosettotodo/todos create. - Add
todosets listsubcommand and makebasecamp todosetsdefault to listing todosets; update surface snapshot + e2e coverage.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tui/resolve/todolist.go | Adds explicit todoset scoping to TUI todolist resolution/fetch/create-default flows. |
| internal/tui/resolve/dock_test.go | Updates resolver tests for new Todolist signature. |
| internal/names/resolver.go | Merges todolists across multiple todosets and hard-fails on partial fetch errors. |
| internal/names/resolver_test.go | Adds tests for multi-todoset merge behavior and partial fetch failure. |
| internal/commands/helpers.go | Adds todoset-scoped todolist name resolution helper; threads todoset into ensureTodolist. |
| internal/commands/todos.go | Adds --todoset to todo and todos create and uses todoset-scoped list resolution. |
| internal/commands/todos_test.go | Adds tests ensuring --todoset scopes todolist resolution (including pagination case). |
| internal/commands/todosets.go | Adds todosets list, defaults todosets to list, and scopes --todoset to show. |
| internal/commands/commands.go | Updates command catalog metadata for todosets actions. |
| e2e/todosets.bats | Updates e2e expectations: todosets defaults to list and missing-project behavior. |
| .surface | Updates CLI compatibility snapshot for new subcommand/flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bb93c3b2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Projects can have multiple todoset dock entries but getTodolists() was breaking on the first match, silently discarding the rest. Now collects all todoset IDs and merges their todolists. Partial fetch errors fail hard instead of being swallowed.
Add explicitTodosetID parameter to Resolver.Todolist(), TodolistWithPersist(), ensureTodolist(), and their internal helpers so that --todoset can scope which todolists are visible. Add resolveTodolistInTodoset() for todoset-scoped name resolution using paginated fetch via GetAll.
When a project has multiple todosets, --todoset <id> constrains todolist resolution to that specific todoset. Covers both the shortcut (todo) and full (todos create) paths, including config todolist_id fallback. Tests cover scoped resolution, wrong-list rejection, duplicate name disambiguation, and paginated fetch.
Bare `basecamp todosets` now lists all todoset dock entries for a project (enabled and disabled). Move --todoset from persistent to local on `show` so it doesn't leak onto `list`. Update catalog, surface snapshot, and e2e tests.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
names/resolver.gonow collects all todoset dock entries instead of breaking on the first match, and fails hard on partial fetch errors--todosetthrough resolution:Resolver.Todolist(),ensureTodolist(), and a newresolveTodolistInTodoset()helper accept an explicit todoset ID to scope which todolists are visible--todosettotodoandtodos create: constrains todolist name resolution to a single todoset when projects have multipletodosets listsubcommand: lists all todoset dock entries (enabled and disabled);--todosetflag scoped toshowonlyCLI surface changes
basecamp todoandbasecamp todos creategain--todoset/-tflagbasecamp todosets listis a new subcommand--todosetmoved from persistent (all todosets subcommands) to local onshowonlyPrecedence semantics
Resolution order for todolist identity:
--listflag >todolist_idconfig > interactive picker. When--todosetis provided, all three paths are scoped to that todoset — name resolution only sees todolists belonging to it, and the interactive picker fetches from it exclusively.Numeric
--listvalues pass through without validation (trusted as IDs regardless of--todoset). Name values go through exact → case-insensitive → partial matching, same as the project-wide resolver.Why fail hard on partial todoset fetches?
The previous code silently continued when one todoset's fetch failed, returning todolists from only the successful todosets. This is dangerous: a transient 500 on one todoset could make its todolists invisible to name resolution, causing "not found" errors or — worse — resolving an ambiguous name to the wrong list in a different todoset. Failing the entire resolution on any todoset error is the safer default.
Review guidance
Concentrate on:
--todosetvs--listvstodolist_idconfig interactionsresolveTodolistInTodoset()in helpers.go duplicates matching logic fromnames/resolver.go— aligned now but a future drift point. Extracting a shared matcher is deferred to keep this PR focused.Prefer rebase or merge, not squash — the 4 commits are structured to be individually reviewable.
Test plan
bin/cipasses (unit, e2e, surface snapshot, skill drift, lint)basecamp todosets list --in 6792661shows all 4 todosetsbasecamp todo "test" --in 6792661 --todoset 9147931763 --list "Critical bits"resolves within scoped todoset (BC5 to-dos)basecamp todo "test" --in 6792661 --list "Bananarama"resolves across all todosets without --todoset (no regression)