Conversation
github-actions
bot
added
commands
There was a problem hiding this comment.
Pull request overview
Adds assignee support when creating cards via both basecamp cards create and the basecamp card shortcut, including flag aliases, completion, and tests.
Changes:
- Introduces
--assigneeand--to(alias) flags for card creation commands, with pre-resolution of assignee input. - Performs a follow-up card update to set
assignee_idsafter card creation (and improves failure messaging on partial success). - Adds unit/integration-style tests and updates the CLI surface snapshot for the new flags.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/commands/cards.go | Adds assignee flag handling, ID/name resolution helper, and post-create assignment update logic. |
| internal/commands/cards_test.go | Adds tests for new flags, resolution validation, and that assignment triggers an update request. |
| .surface | Updates CLI surface snapshot to include the new flags on relevant commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 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/cards.go">
<violation number="1" location="internal/commands/cards.go:424">
P1: Nil pointer dereference: `card` is reassigned by `Update()`, so if it returns `(nil, err)`, accessing `card.ID` in the error path panics. Save the card ID before the `Update` call. This same bug exists in both `newCardsCreateCmd` and `NewCardCmd`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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: 9528348cb1
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Adds assignee support to card creation flows in the Basecamp CLI, allowing users to specify an assignee by ID or name during cards create and the card shortcut.
Changes:
- Introduces
resolveAssigneeIDhelper and reuses it across card update/assignee parsing. - Adds
--assignee(and--toalias) tocards createandcardshortcut, implementing a post-create assignment update call. - Extends command tests and updates
.surfaceto reflect the new flags.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/commands/cards.go | Adds assignee resolution helper, new --assignee/--to flags for create/shortcut, and assignment-on-create behavior. |
| internal/commands/cards_test.go | Adds tests for new flags, assignment-on-create request behavior, and resolveAssigneeID validation. |
| .surface | Updates surface snapshot with new flags for card and cards create. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Adds first-class assignee support to card creation commands, including early validation/name resolution and shell completion, with accompanying surface snapshot and tests.
Changes:
- Introduces
resolveAssigneeIDhelper to validate/resolve assignees (ID or person name). - Adds
--assignee(and--toalias) tocards createandcard(shortcut) and performs a post-create update to apply the assignment. - Updates
.surfaceand adds tests covering flag presence, assignment update behavior, and assignee ID validation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/commands/cards.go | Adds assignee resolution helper and wires --assignee/--to into card create flows (plus reuse in assignee list parsing). |
| internal/commands/cards_test.go | Adds tests for new flags and verifies assignment triggers an update request; adds validation tests for assignee IDs. |
| .surface | Records new public CLI flags for surface snapshot testing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
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="internal/commands/cards.go">
<violation number="1" location="internal/commands/cards.go:555">
P2: The no-changes guard at line 527 still checks `assignee == ""`, which short-circuits before the new `cmd.Flags().Changed("assignee")` check can run. When only `--assignee ""` is passed, the user gets a confusing "no changes specified" error instead of "Assignee cannot be empty". Update the guard to also consider `cmd.Flags().Changed("assignee")`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Support assigning a person during card creation via --assignee/--to flags on both `basecamp card` and `basecamp cards create`. Since the SDK's CreateCardRequest lacks AssigneeIDs, assignment is a post-create Cards().Update() call. The assignee is pre-resolved before creation to fail early on bad input, and the error on update failure preserves structured SDK error metadata (code/hint/retryable) while noting the card was already created. Extract resolveAssigneeID helper (rejects non-positive IDs) and refactor the inline resolve in cards update to use it.
- Save card ID before Update() call to prevent nil-pointer dereference on assignment failure (cubic, codex) - TrimSpace flag input in resolveAssigneeID (copilot) - Validate resolved ID > 0 after ResolvePerson (copilot) - Refactor resolveAssigneeIDs (plural) to delegate to resolveAssigneeID so validation logic stays in one place (copilot)
- Assignment failure fallback now wraps sdkErr (not raw err) for
consistent CLI error formatting
- Update command's noChanges guard uses Flags().Changed("assignee")
so --assignee "" reaches resolveAssigneeID validation
There was a problem hiding this comment.
Pull request overview
Adds first-class assignee support when creating cards (both basecamp cards create and the basecamp card shortcut) by introducing --assignee (and --to alias), validating/normalizing assignee input, and exercising the behavior in tests.
Changes:
- Add
--assigneeand--toflags for card creation, plus shell completion for people names. - Introduce
resolveAssigneeIDhelper and reuse it for step assignee resolution and card update assignee parsing. - Add tests covering the new flags and the “create then assign via update” request flow; update
.surfacefor the new flags.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/commands/cards.go | Adds assignee flags + helper resolver; performs post-create assignment via an update call; reuses resolver in other assignee parsing paths. |
| internal/commands/cards_test.go | Adds tests for flag presence, request behavior (PUT with assignee_ids), and resolveAssigneeID validation. |
| .surface | Records the newly added CLI flags for basecamp card and basecamp cards create. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98a26cba75
ℹ️ 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".
Resolve assignee before resolveLocalImages (which uploads attachments) so a bad --assignee fails before any uploads happen.
Summary
--assigneeand--toflags tobasecamp cardandbasecamp cards createfor assigning a person during card creationresolveAssigneeIDhelper that accepts numeric IDs or names, rejects non-positive IDsCards().Update()assigns the person (SDK'sCreateCardRequestlacksAssigneeIDs)cards updateto use the sharedresolveAssigneeIDhelperTest plan
TestCardsCreateHasAssigneeFlag— flag presence oncards createTestCardShortcutHasAssigneeFlag— flag presence oncardTestCardsCreateWithAssigneeSendsUpdate— mock transport verifies create→update flow sendsassignee_idsand output includes assigneeTestResolveAssigneeIDRejectsZero— zero ID rejectedTestResolveAssigneeIDRejectsNegative— negative ID rejectedTestResolveAssigneeIDAcceptsPositive— positive ID acceptedbin/cipasses (lint, vet, unit tests, e2e, surface snapshot, skill drift, bare groups)Summary by cubic
Add
--assignee/--totobasecamp cardandbasecamp cards createto assign a person at creation. The assignee is resolved before content processing and applied via a post-createCards().Update().New Features
--assignee/--toonbasecamp cardandbasecamp cards create(accepts ID or name).Bug Fixes
Flags().Changedto catch explicit empty--assignee/--to; trim input and require resolved IDs > 0 inresolveAssigneeID.Update()to avoid nil-pointer on assignment failure.cards updateandresolveAssigneeIDsdelegate toresolveAssigneeID;cards updateno-changes check usesFlags().Changed("assignee")so empty values are validated.Written for commit 458eeac. Summary will update on new commits.