Fix MarkFlagRequired rejecting alias flags

internal/commands/todolistgroups.go

@@ -347,6 +347,10 @@ func newTodolistgroupsPositionCmd() *cobra.Command {
 		Short:   "Change group position",
 		Args:    cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if position == 0 {
+				return output.ErrUsage("--position is required (1-based)")
+			}
+
 			app := appctx.FromContext(cmd.Context())
 			if app == nil {
 				return fmt.Errorf("app not initialized")
@@ -358,10 +362,6 @@ func newTodolistgroupsPositionCmd() *cobra.Command {
 
 			groupIDStr := args[0]
 
-			if position == 0 {
-				return output.ErrUsage("--position is required (1-based)")
-			}
-
 			groupID, err := strconv.ParseInt(groupIDStr, 10, 64)
 			if err != nil {
 				return output.ErrUsage("Invalid group ID")
@@ -381,7 +381,6 @@ func newTodolistgroupsPositionCmd() *cobra.Command {
 
 	cmd.Flags().IntVar(&position, "position", 0, "New position, 1-based (required)")
 	cmd.Flags().IntVar(&position, "pos", 0, "New position (alias)")
-	_ = cmd.MarkFlagRequired("position")
 
 	return cmd
 }

internal/commands/todolistgroups_test.go

@@ -0,0 +1,45 @@
+package commands
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/basecamp/basecamp-cli/internal/output"
+)
+
+// TestTodolistgroupsPositionAcceptsPosAlias tests that --pos works as an alias for --position.
+func TestTodolistgroupsPositionAcceptsPosAlias(t *testing.T) {
+	app, _ := setupTestApp(t)
+	app.Config.ProjectID = "123"
+
+	cmd := newTodolistgroupsPositionCmd()
+
+	// --pos should reach the RunE and proceed past the position guard
+	err := executeCommand(cmd, app, "456", "--pos", "2")
+
+	// Expect an API/network error — NOT "required flag" and NOT the RunE usage guard
+	require.NotNil(t, err)
+	assert.NotContains(t, err.Error(), "required flag")
+	var e *output.Error
+	if errors.As(err, &e) {
+		assert.NotEqual(t, "--position is required (1-based)", e.Message)
+	}
+}
+
+// TestTodolistgroupsPositionRequiresPosition tests the RunE guard when neither flag is given.
+func TestTodolistgroupsPositionRequiresPosition(t *testing.T) {
+	app, _ := setupTestApp(t)
+	app.Config.ProjectID = "123"
+
+	cmd := newTodolistgroupsPositionCmd()
+
+	err := executeCommand(cmd, app, "456")
+	require.NotNil(t, err)
+
+	var e *output.Error
+	require.True(t, errors.As(err, &e))
+	assert.Equal(t, "--position is required (1-based)", e.Message)
+}

internal/commands/tools.go

@@ -130,16 +130,16 @@ func newToolsCreateCmd(project *string) *cobra.Command {
 
 For example, clone a Campfire to create a second chat room in the same project.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if sourceID == "" {
+				return output.ErrUsage("--source or --clone is required (ID of tool to clone)")
+			}
+
 			app := appctx.FromContext(cmd.Context())
 
 			if err := ensureAccount(cmd, app); err != nil {
 				return err
 			}
 
-			if sourceID == "" {
-				return output.ErrUsage("--source is required (ID of tool to clone)")
-			}
-
 			sourceToolID, err := strconv.ParseInt(sourceID, 10, 64)
 			if err != nil {
 				return output.ErrUsage("Invalid source tool ID")
@@ -203,7 +203,6 @@ For example, clone a Campfire to create a second chat room in the same project.`
 
 	cmd.Flags().StringVarP(&sourceID, "source", "s", "", "Source tool ID to clone (required)")
 	cmd.Flags().StringVar(&sourceID, "clone", "", "Source tool ID (alias for --source)")
-	_ = cmd.MarkFlagRequired("source")
 
 	return cmd
 }
@@ -470,6 +469,10 @@ func newToolsRepositionCmd(project *string) *cobra.Command {
 		Long:    "Move a tool to a different position in the project dock.",
 		Args:    cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if position == 0 {
+				return output.ErrUsage("--position is required (1-based)")
+			}
+
 			app := appctx.FromContext(cmd.Context())
 
 			if err := ensureAccount(cmd, app); err != nil {
@@ -481,10 +484,6 @@ func newToolsRepositionCmd(project *string) *cobra.Command {
 				return output.ErrUsage("Invalid tool ID")
 			}
 
-			if position == 0 {
-				return output.ErrUsage("--position is required (1-based)")
-			}
-
 			// Resolve project, with interactive fallback
 			projectID := *project
 			if projectID == "" {
@@ -525,7 +524,6 @@ func newToolsRepositionCmd(project *string) *cobra.Command {
 
 	cmd.Flags().IntVar(&position, "position", 0, "New position, 1-based (required)")
 	cmd.Flags().IntVar(&position, "pos", 0, "New position (alias)")
-	_ = cmd.MarkFlagRequired("position")
 
 	return cmd
 }

internal/commands/tools_test.go

@@ -0,0 +1,85 @@
+package commands
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/basecamp/basecamp-cli/internal/output"
+)
+
+// TestToolsCreateAcceptsCloneAlias tests that --clone works as an alias for --source.
+// Previously, MarkFlagRequired("source") caused Cobra to reject --clone before RunE ran.
+func TestToolsCreateAcceptsCloneAlias(t *testing.T) {
+	app, _ := setupTestApp(t)
+	app.Config.ProjectID = "123"
+
+	project := ""
+	cmd := newToolsCreateCmd(&project)
+
+	// --clone should reach the RunE guard, not fail with "required flag not set"
+	err := executeCommand(cmd, app, "--clone", "999", "My Tool")
+
+	// Expect an API/network error — NOT "required flag" and NOT the RunE usage guard
+	require.NotNil(t, err)
+	var e *output.Error
+	if errors.As(err, &e) {
+		assert.NotEqual(t, "--source or --clone is required (ID of tool to clone)", e.Message)
+	}
+	assert.NotContains(t, err.Error(), "required flag")
+}
+
+// TestToolsCreateRequiresSourceOrClone tests that omitting both --source and --clone
+// produces a usage error from the RunE guard.
+func TestToolsCreateRequiresSourceOrClone(t *testing.T) {
+	app, _ := setupTestApp(t)
+	app.Config.ProjectID = "123"
+
+	project := ""
+	cmd := newToolsCreateCmd(&project)
+
+	err := executeCommand(cmd, app, "My Tool")
+	require.NotNil(t, err)
+
+	var e *output.Error
+	require.True(t, errors.As(err, &e))
+	assert.Equal(t, "--source or --clone is required (ID of tool to clone)", e.Message)
+}
+
+// TestToolsRepositionAcceptsPosAlias tests that --pos works as an alias for --position.
+func TestToolsRepositionAcceptsPosAlias(t *testing.T) {
+	app, _ := setupTestApp(t)
+	app.Config.ProjectID = "123"
+
+	project := ""
+	cmd := newToolsRepositionCmd(&project)
+
+	// --pos should reach the RunE and proceed past the position guard
+	err := executeCommand(cmd, app, "456", "--pos", "2")
+
+	// Expect an API/network error — NOT "required flag" and NOT the RunE usage guard
+	require.NotNil(t, err)
+	assert.NotContains(t, err.Error(), "required flag")
+	var e *output.Error
+	if errors.As(err, &e) {
+		assert.NotEqual(t, "--position is required (1-based)", e.Message)
+	}
+}
+
+// TestToolsRepositionRequiresPosition tests the RunE guard when neither flag is given.
+func TestToolsRepositionRequiresPosition(t *testing.T) {
+	app, _ := setupTestApp(t)
+	app.Config.ProjectID = "123"
+
+	project := ""
+	cmd := newToolsRepositionCmd(&project)
+
+	err := executeCommand(cmd, app, "456")
+	require.NotNil(t, err)
+
+	var e *output.Error
+	require.True(t, errors.As(err, &e))
+	assert.Equal(t, "--position is required (1-based)", e.Message)
+}