Add --on-hold flag to cards move command#325
Conversation
github-actions
bot
added
commands
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 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="go.mod">
<violation number="1" location="go.mod:80">
P1: Do not merge a `replace` directive pointing to a personal fork. Per project conventions, SDK dependency changes should go through `make bump-sdk` which updates both `go.mod` and `sdk-provenance.json`. This `replace` directive should be removed before merging — wait for basecamp/basecamp-sdk#188 to be merged and tagged, then use `make bump-sdk` to pull in the official release.</violation>
</file>
<file name="internal/commands/cards.go">
<violation number="1" location="internal/commands/cards.go:690">
P2: `--position` is silently ignored when combined with `--on-hold`. Reject the unsupported combination early so users don't assume their card landed at a specific position.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Adds support for moving Card Table cards into a column’s on-hold section via the basecamp cards move command, with accompanying CLI docs and an SDK dependency update.
Changes:
- Add
--on-holdflag tobasecamp cards move, including a newmoveCardOnHoldcode path. - Update Basecamp skill documentation with on-hold move examples.
- Temporarily switch the Basecamp SDK dependency via a
go.modreplace(and correspondinggo.sumupdates).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| skills/basecamp/SKILL.md | Documents new --on-hold usage for card moves. |
| internal/commands/cards.go | Implements --on-hold flag handling and on-hold move logic. |
| go.mod | Adds a temporary replace to a forked basecamp-sdk version. |
| go.sum | Updates checksums to match the forked SDK dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
tests
There was a problem hiding this comment.
Pull request overview
Adds CLI support for moving Basecamp cards into a column’s on-hold section, aligning the CLI with upcoming SDK support for CardColumn.on_hold.
Changes:
- Add
--on-hold(andmvalias) tobasecamp cards move, including new move logic for current vs target column. - Update docs/examples for the new flag and adjust for an SDK API signature change.
- Temporarily point
github.com/basecamp/basecamp-sdk/goto a fork viago.modreplace + updatedgo.sum.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/basecamp/SKILL.md | Documents cards move --on-hold usage and examples. |
| internal/commands/tools.go | Updates Tools.Create call to new SDK signature (adds nil options). |
| internal/commands/cards.go | Implements --on-hold behavior for cards move and adds mv alias. |
| internal/commands/cards_test.go | Adds tests for --position + --on-hold incompatibility and on-hold behavior. |
| go.mod | Adds temporary replace to SDK fork (pseudo-version). |
| go.sum | Updates checksums to match forked SDK dependency. |
💡 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 support for moving cards into a column’s on-hold section via the CLI, relying on the SDK exposing CardColumn.on_hold.
Changes:
- Add
--on-hold(andmvalias) tobasecamp cards move, including validation that rejects--positionwith--on-hold. - Add/adjust unit tests and CLI docs/examples for the new flag.
- Update module files (including a local
replacefor the SDK) and add a large mention-support implementation plan doc.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
internal/commands/cards.go |
Implements --on-hold behavior via moveCardOnHold and updates command help/flags. |
internal/commands/cards_test.go |
Adds tests for --on-hold validation and a mocked on-hold move flow. |
skills/basecamp/SKILL.md |
Documents the new --on-hold usage with examples. |
go.mod |
Adds a local filesystem replace for the SDK (temporary but currently merge-blocking). |
go.sum |
Removes prior SDK checksums (likely a side effect of the local replace). |
docs/superpowers/plans/2026-03-13-mention-support.md |
Adds an unrelated implementation plan document (appears out of scope for this PR). |
💡 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 support in the Basecamp CLI for moving cards into a column’s on-hold section via a new --on-hold flag, updating docs and test coverage accordingly.
Changes:
- Add
--on-holdmode (andmvalias) tobasecamp cards move, including validation that--positioncannot be combined with--on-hold. - Implement on-hold move flow by resolving the target column’s on-hold section ID and moving the card there.
- Update Basecamp skill docs and add unit tests for the new flag/behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| skills/basecamp/SKILL.md | Documents new cards move --on-hold usage and examples. |
| internal/commands/cards.go | Implements --on-hold flag handling, alias, validation, and on-hold move helper. |
| internal/commands/cards_test.go | Adds tests and HTTP transport mocks for on-hold move flows and flag validation. |
💡 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 5 files (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="go.mod">
<violation number="1" location="go.mod:9">
P1: This pins the SDK to a pseudo-version (untagged commit) which should not be merged. Once basecamp/basecamp-sdk#188 is tagged, run `make bump-sdk` to update to the proper release version — per AGENTS.md, go.mod should not be edited directly.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| charm.land/bubbletea/v2 v2.0.2 | ||
| charm.land/lipgloss/v2 v2.0.0 | ||
| github.com/basecamp/basecamp-sdk/go v0.6.0 | ||
| github.com/basecamp/basecamp-sdk/go v0.6.1-0.20260318172136-4784bb2fda18 |
There was a problem hiding this comment.
P1: This pins the SDK to a pseudo-version (untagged commit) which should not be merged. Once basecamp/basecamp-sdk#188 is tagged, run make bump-sdk to update to the proper release version — per AGENTS.md, go.mod should not be edited directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At go.mod, line 9:
<comment>This pins the SDK to a pseudo-version (untagged commit) which should not be merged. Once basecamp/basecamp-sdk#188 is tagged, run `make bump-sdk` to update to the proper release version — per AGENTS.md, go.mod should not be edited directly.</comment>
<file context>
@@ -6,7 +6,7 @@ require (
charm.land/bubbletea/v2 v2.0.2
charm.land/lipgloss/v2 v2.0.0
- github.com/basecamp/basecamp-sdk/go v0.6.0
+ github.com/basecamp/basecamp-sdk/go v0.6.1-0.20260318172136-4784bb2fda18
github.com/basecamp/cli v0.1.1
github.com/charmbracelet/bubbles v1.0.0
</file context>
There was a problem hiding this comment.
The pseudo-version is intentional — SDK PR 188 merged to main but no tag has been cut yet. The bump was done via make bump-sdk REF=4784bb2fda18 which is the standard workflow per AGENTS.md. Once a tagged release is available, a follow-up make bump-sdk will pin to it.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
There was a problem hiding this comment.
Pull request overview
Adds an --on-hold mode to basecamp cards move so cards can be moved into the on-hold section of either their current column or a specified target column, leveraging the SDK’s new CardColumn.on_hold support.
Changes:
- Add
--on-holdflag (andmvalias) tobasecamp cards move, making--tooptional in on-hold mode and rejecting--positionwith--on-hold. - Implement
moveCardOnHold(...)to resolve the correct on-hold section ID and perform the move, with an error when on-hold is unavailable. - Update docs/surface snapshots and bump the Basecamp SDK dependency to a revision that exposes
CardColumn.on_hold.
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 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
internal/commands/cards.go |
Adds the new --on-hold flow to cards move, including alias and validation logic. |
internal/commands/cards_test.go |
Adds test coverage for on-hold moves and validates --position + --on-hold rejection. |
skills/basecamp/SKILL.md |
Documents new on-hold move usage examples. |
.surface |
Records the new --on-hold flag in CLI surface snapshots. |
go.mod |
Bumps Basecamp SDK to a revision that includes CardColumn.on_hold. |
go.sum |
Updates checksums for the SDK version bump. |
internal/version/sdk-provenance.json |
Captures the updated SDK provenance metadata (version/revision/timestamp). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move a card to the on-hold section of its current column (--on-hold) or a target column (--to <column> --on-hold). Requires the SDK to expose the on_hold property on CardColumn (basecamp/basecamp-sdk#188).
- Reject --position when combined with --on-hold - Fix help examples to use numeric column IDs - Rename annotation key from "notes" to "agent_notes" - Use CardColumns().Get() for numeric column IDs and current column to avoid requiring --card-table in multi-table projects - Fix SKILL.md example to use numeric column ID - Add tests for --position + --on-hold rejection and --on-hold without --card-table - Update go.mod replace to latest SDK fork commit
- Fix usage hint to match actual command syntax (no --enable flag) - Update --to flag description to note it's optional with --on-hold - Add SKILL.md example for --card-table with named column + --on-hold - Replace weak on-hold test with mock transport that verifies full flow
- Fix agent_notes on cards move: replace irrelevant assignee filtering note with --position/--on-hold incompatibility note - Fix mock transport comment to reflect actual request sequence - Add tests for --on-hold with numeric --to and disabled on-hold error
- Mock transports now error on unexpected requests instead of returning empty 200s, catching unintended API calls in tests - On-hold error hint now includes the concrete column ID instead of a generic placeholder
Pin to 4784bb2fda18 which adds CardColumnOnHold types, EnableOnHold, and DisableOnHold service methods.
Cover the named-column + --on-hold code path with TestCardsMoveOnHoldWithNamedColumn. Fix mock JSON to match real CardColumnOnHold struct shape. Regenerate .surface for --on-hold flag.
Use ErrUsageHint with a concrete --to suggestion instead of a bare ErrUsage, so users know how to recover when the API returns a card without a parent column reference.
jeremy
force-pushed
the
feat/cards-move-on-hold
branch
from
ae9627b to
7f30f0a
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the Basecamp CLI’s card-moving workflow by adding an --on-hold mode to basecamp cards move, enabling moves into a column’s on-hold section (either for the current column or a specified target column). It also updates the pinned Basecamp SDK version to one that exposes CardColumn.OnHold, which is required to resolve the on-hold section ID.
Changes:
- Add
--on-holdtocards move(withmvalias), make--tooptional in that mode, and reject--positionwhen--on-holdis set. - Implement on-hold move resolution logic (current column vs numeric
--tovs named--tovia card table lookup) with helpful usage hints. - Add/extend tests for the new flag behavior and update docs + CLI surface/dependency metadata.
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 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
internal/commands/cards.go |
Adds --on-hold flag/alias and implements move-to-on-hold behavior, including validation and user-facing hints. |
internal/commands/cards_test.go |
Adds coverage for --on-hold flows (no --to, numeric --to, named --to) and validates flag incompatibilities. |
skills/basecamp/SKILL.md |
Documents new cards move --on-hold usage examples. |
.surface |
Updates CLI surface snapshot to include the new --on-hold flag. |
go.mod / go.sum |
Bumps github.com/basecamp/basecamp-sdk/go to a revision that includes CardColumn.OnHold. |
internal/version/sdk-provenance.json |
Records the updated SDK version and revision provenance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Depends on basecamp/basecamp-sdk#188
What
Add
--on-holdflag tobasecamp cards moveto move cards into the on-hold section of a column.basecamp cards move <id> --on-hold— move to on-hold of current columnbasecamp cards move <id> --to <column> --on-hold— move to on-hold of target columnWhy
There was no way to move cards to/from on-hold sections via the CLI. The SDK was also missing the
on_holdproperty onCardColumn, which is required to resolve the on-hold section ID.Testing
go build ./...passes (with SDK replace pointing to fork branch)make check— blocked until SDK PR is merged and dep bumpedNote:
go.modcontains a temporaryreplacedirective pointing to the SDK fork. This should be replaced with a proper version bump once basecamp/basecamp-sdk#188 is merged and tagged.Summary by cubic
Adds
--on-holdtobasecamp cards moveso you can move a card into the on‑hold section of its current or a target column. Also adds anmvalias, blocks--positionwith--on-hold, and makes--tooptional in that mode.New Features
basecamp cards move <id> --on-holdmoves to on‑hold of the current column.basecamp cards move <id> --to <column> --on-holdmoves to on‑hold of the target column (use--card-tablewhen<column>is a name).mvalias;--tooptional with--on-hold; rejects--positionwith--on-hold; no--card-tableneeded for numeric IDs or when using the current column.--position+--on-holdrejection; mock transports now fail on unexpected API calls.Dependencies
github.com/basecamp/basecamp-sdk/gotov0.6.1-0.20260318172136-4784bb2fda18withCardColumn.OnHoldand on‑hold enable/disable APIs.basecamp-sdk/go.Written for commit 7f30f0a. Summary will update on new commits.