Support multiple IDs in assign and unassign

[jeremy]

Summary

• basecamp assign and basecamp unassign now accept variadic IDs for batch operations, following the done/reopen pattern
• Items are validated before the person picker appears (preserving PR Validate item before person picker in assign/unassign #279 ordering); assignee is resolved lazily on the first valid item
• Partial success reports "Assigned N, failed M"; all-fail preserves the typed error (not-found, API) from the first failure

Test plan

• bin/ci green (lint, unit, e2e, surface, skill drift, naming, bare groups)
• Smoke: basecamp assign <id1> <id2> --to me -p <project> assigns both
• Smoke: basecamp unassign <id1> <id2> --from me -p <project> unassigns both
• Single-ID still works identically to before (breadcrumbs, summary)
• Invalid ID in batch is skipped, valid IDs still succeed
• All-invalid batch returns typed error with all IDs listed

[github-actions]

⚠️ Potential breaking changes detected:

• The assign and unassign commands changed from accepting exactly one argument (id) to accepting multiple arguments (id|url ...). This breaks compatibility for scripts or invocations relying on the prior ExactArgs(1) behavior.
• Modification in the Long documentation for assign and unassign commands to describe multiple IDs may cause incompatibilities if scripts referenced this for validation.
• Tests that assert on specific error messages for assign and unassign commands (e.g., accepts 1 arg) will now behave differently due to updated argument handling logic.

Review carefully before merging. Consider a major version bump.

[cubic-dev-ai]

3 issues found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/commands/assign_test.go">

<violation number="1" location="internal/commands/assign_test.go:578">
P2: Zero-initialized indices can cause this ordering assertion to give false positives. If the GET for `/todos/222` never appears in the request log, `todoGetIdx` stays 0, and any positive `profileIdx` makes the assertion pass incorrectly. Initialize both to `-1` and assert they were actually found.</violation>
</file>

<file name="internal/commands/assign.go">

<violation number="1" location="internal/commands/assign.go:198">
P2: Batch breadcrumb uses `extractedIDs[0]` which may be a failed item ID. When the first arg fails validation and later ones succeed, the breadcrumb suggests un/assigning from the wrong item. Use the ID from the first successful result instead.</violation>
</file>

<file name=".surface-breaking">

<violation number="1" location=".surface-breaking:2">
P2: The current-canonical `<id>` entries don't belong in `.surface-breaking`. This file records *removals* from the previous surface; the legacy `<todo_id>` lines already cover that. The new `<id>` arg names should only appear in `.surface`.

(Based on your team's feedback about keeping legacy names in .surface-breaking and current names in .surface only.) [FEEDBACK_USED]</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0da7be9a24

ℹ️ 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".

[Copilot]

Pull request overview

Adds batch support to the Basecamp CLI’s assign/unassign commands so users can apply assignment changes to multiple items in one invocation (IDs and Basecamp URLs), and updates docs/tests accordingly.

Changes:

• Update assign/unassign to accept variadic <id|url>... and process items in a batch loop with partial-success handling.
• Extend unit + smoke coverage for multi-ID behavior and batch failure reporting.
• Refresh SKILL quick reference and CLI arg contract expectations for the new positional arg shape.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
skills/basecamp/SKILL.md	Updates quick reference examples to show multi-ID assign/unassign usage.
internal/commands/doc_contract_test.go	Updates doc/arg contract expectations for assign/unassign positional args.
internal/commands/assign.go	Implements variadic ID/URL support, batch processing, and new result/error plumbing.
internal/commands/assign_test.go	Adds coverage for multi-ID acceptance and batch behavior (partial success, all fail, lazy resolution).
e2e/smoke/smoke_assign.bats	Adds smoke coverage for multi-todo assign/unassign.
.surface-breaking	Records acknowledged CLI surface arg changes for assign/unassign.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".surface-breaking">

<violation number="1" location=".surface-breaking:2">
P2: The `<id>` entries are *additions* to the current surface, not removals from the previous baseline. They belong in `.surface`, not `.surface-breaking`. Only legacy names (`<todo_id>`) should appear here.

(Based on your team's feedback about keeping legacy argument names in .surface-breaking and current names in .surface.) [FEEDBACK_USED]</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

This PR extends the Basecamp CLI’s assign/unassign commands to support operating on multiple items in a single invocation, and updates documentation and tests accordingly.

Changes:

• Update assign/unassign to accept multiple <id|url> arguments and process them in a batch loop with partial-success handling.
• Refresh docs/contract tests to reflect the new argument shape and add unit + smoke coverage for batch behavior.
• Add CLI surface breaking-change allowlist entries related to the updated positional arg contract.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
skills/basecamp/SKILL.md	Updates quick reference and todos section examples to show multi-ID assign/unassign usage.
internal/commands/doc_contract_test.go	Updates doc-contract expectations for assign/unassign positional arg naming.
internal/commands/assign.go	Implements batch assign/unassign flow, result aggregation, and batch error handling.
internal/commands/assign_test.go	Adds unit tests for multi-ID acceptance, batch failure formatting, and lazy assignee resolution ordering.
e2e/smoke/smoke_assign.bats	Adds smoke coverage for multi-todo assign/unassign in one command.
.surface-breaking	Updates allowlist entries for intentional CLI surface changes related to assign/unassign args.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.