Refuse a container flag that a move or copy would ignore

todos copy 456 --todolist 901 copied the to-do into its own list and
reported success: --todolist is a root flag todos copy never reads.
todolistgroups copy --list failed the same way and was fixed on its
own, as were cards copy --card-table and files move --vault.

Every command that moves or copies recordings now goes through
guardFilingFlags, which names the container flags the command
inherits and reads. Any other one that was set, the root's --todolist
or a flag the command's group adds (--card-table, --vault, --folder,
--list), is refused before anything is read or sent, with the
command's own --to-<container> flags as the hint. The three one-off
refusals go. --project and --in stay with each command, which holds a
filed recording to them.

The same-project modes are covered too: todos move --to 1 --todolist
and todolistgroups position --list, which ignored the flag, now refuse
it. cards move and cards column move read --card-table, so they keep it.

The test walks the whole command tree: every move or copy command
declares what it reads, and each container flag it inherits but does
not read is refused with no request sent.

Author: robzolkos

internal/commands/cards.go

@@ -43,7 +43,7 @@ func NewCardsCmd() *cobra.Command {
 		newCardsCreateCmd(&project, &cardTable),
 		newCardsUpdateCmd(),
 		newCardsMoveCmd(&project, &cardTable),
-		newCardsCopyCmd(&project, &cardTable),
+		newCardsCopyCmd(&project),
 		newCardsDoneCmd(&project, &cardTable),
 		newCardsColumnsCmd(&project, &cardTable),
 		newCardsColumnCmd(&project, &cardTable),
@@ -1374,7 +1374,13 @@ Basecamp to finish the move and shows the card where it landed.`,
 				if targetColumn != "" || onHold || positionSet || wormholeSet {
 					return output.ErrUsage("--to-project and --to-card-table cannot be combined with --to, --on-hold, --position, or --to-wormhole")
 				}
-				if err := validateFilingFlags(*cardTable, toProject, toCardTable); err != nil {
+				if *cardTable != "" {
+					return output.ErrUsageHint(
+						"--card-table names the card's own card table, not where it goes",
+						"Use --to-card-table for the destination card table",
+					)
+				}
+				if err := validateFilingFlags(toProject, toCardTable); err != nil {
 					return err
 				}
 			} else if wormholeSet {
@@ -1563,7 +1569,7 @@ Basecamp to finish the move and shows the card where it landed.`,
 	cmd.Flags().StringVar(&toWormhole, "to-wormhole", "", "Send the card to another project's card table through an existing wormhole (wormhole ID or destination-column URL)")
 	addFilingDestinationFlags(cmd, &toProject, &toCardTable, "Move")
 
-	return cmd
+	return guardFilingFlags(cmd, "card-table")
 }
 
 // moveCardThroughWormhole teleports a card across projects through a wormhole to
@@ -1797,7 +1803,7 @@ func parseColumnID(ref string) (int64, error) {
 	return id, nil
 }
 
-func newCardsCopyCmd(project, cardTable *string) *cobra.Command {
+func newCardsCopyCmd(project *string) *cobra.Command {
 	var toProject, toCardTable string
 	var includeComments bool
 
@@ -1826,8 +1832,8 @@ the new card.`,
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			filingSet := cmd.Flags().Changed("to-project") || cmd.Flags().Changed("to-card-table")
-			if filingSet || *cardTable != "" {
-				if err := validateFilingFlags(*cardTable, toProject, toCardTable); err != nil {
+			if filingSet {
+				if err := validateFilingFlags(toProject, toCardTable); err != nil {
 					return err
 				}
 			}
@@ -1874,7 +1880,7 @@ the new card.`,
 	addFilingDestinationFlags(cmd, &toProject, &toCardTable, "Copy")
 	cmd.Flags().BoolVar(&includeComments, "include-comments", false, "Copy the card's comments too")
 
-	return cmd
+	return guardFilingFlags(cmd)
 }
 
 // addFilingDestinationFlags adds --to-project and --to-card-table, which name
@@ -1885,15 +1891,8 @@ func addFilingDestinationFlags(cmd *cobra.Command, toProject, toCardTable *strin
 }
 
 // validateFilingFlags checks --to-project and --to-card-table before any
-// request. The group's --card-table names the card's own table, so it is
-// refused here rather than mistaken for the destination.
-func validateFilingFlags(sourceCardTable, toProject, toCardTable string) error {
-	if sourceCardTable != "" {
-		return output.ErrUsageHint(
-			"--card-table names the card's own card table, not where it goes",
-			"Use --to-card-table for the destination card table",
-		)
-	}
+// request.
+func validateFilingFlags(toProject, toCardTable string) error {
 	if toProject == "" && toCardTable == "" {
 		return output.ErrUsage("--to-project or --to-card-table needs a value")
 	}
@@ -2748,7 +2747,7 @@ You can pass either a column ID or a Basecamp URL:
 	cmd.Flags().IntVar(&position, "position", 0, "Target position (1-indexed)")
 	cmd.Flags().IntVar(&position, "pos", 0, "Target position (alias for --position)")
 
-	return cmd
+	return guardFilingFlags(cmd, "card-table")
 }
 
 func newCardsColumnWatchCmd() *cobra.Command {

internal/commands/cards_filing_test.go

@@ -444,16 +444,6 @@ func TestCardsCopyStillRunningSaysNotToCopyAgain(t *testing.T) {
 	assert.Len(t, transport.posts, 1)
 }
 
-func TestCardsCopyRefusesTheSourceCardTableFlag(t *testing.T) {
-	transport := &mockFilingTransport{}
-	run := runFilingCommand(t, transport, "copy", "456", "--card-table", "555")
-	e := requireFilingError(t, run)
-
-	assert.Equal(t, output.CodeUsage, e.Code)
-	assert.Contains(t, e.Hint, "--to-card-table")
-	assert.Empty(t, transport.posts)
-}
-
 func TestCardsMoveToAForeignColumnPointsAtCrossProjectMoves(t *testing.T) {
 	transport := &mockFilingTransport{}
 	run := runFilingCommand(t, transport, "move", "456", "--to", "8888", "--in", "123")

internal/commands/files.go

@@ -2295,11 +2295,6 @@ You can pass IDs or Basecamp URLs for both the item and the folder:
 		Annotations: map[string]string{"agent_notes": "Moves in place within one project: the ID, URL, and comments stay the same. --position is 1-indexed (default: first). A folder in another project is refused."},
 		Args:        cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// --vault/--folder filter the files group's listings; here they would be
-			// silently ignored, and reading them as the destination is a guess.
-			if cmd.Flags().Changed("vault") || cmd.Flags().Changed("folder") {
-				return output.ErrUsageHint("files move takes its destination from --to", "basecamp files move <id|url> --to <folder-id|url>")
-			}
 			if to == "" {
 				return missingArg(cmd, "--to")
 			}
@@ -2386,7 +2381,7 @@ You can pass IDs or Basecamp URLs for both the item and the folder:
 	cmd.Flags().StringVar(&to, "to", "", "Destination folder ID or URL")
 	cmd.Flags().IntVar(&position, "position", 0, "Position in the folder (1-indexed, default: first; past the end means last)")
 
-	return cmd
+	return guardFilingFlags(cmd)
 }
 
 // filesMoveRef is an ID or URL given to files move, reduced to what the move

internal/commands/files_test.go

@@ -1748,8 +1748,6 @@ func TestFilesMoveWithoutDestinationShowsHelp(t *testing.T) {
 
 func TestFilesMoveRefusesBadArgumentsBeforeTheWire(t *testing.T) {
 	cases := map[string][]string{
-		"--folder for --to":         {"123", "--folder", "456"},
-		"--vault beside --to":       {"123", "--to", "456", "--vault", "789"},
 		"zero position":             {"123", "--to", "456", "--position", "0"},
 		"negative position":         {"123", "--to", "456", "--position", "-1"},
 		"non-numeric destination":   {"123", "--to", "Designs"},

internal/commands/filing.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 
 	"github.com/basecamp/basecamp-sdk/go/pkg/basecamp"
 
@@ -466,3 +467,72 @@ func rejectProjectConflict(cmd *cobra.Command, app *appctx.App, noun, flagProjec
 	}
 	return check(urlProjectID, "the "+noun+" URL")
 }
+
+// AnnotationFilingReads lists, space-separated, the container flags that a
+// command moving or copying recordings inherits and reads. guardFilingFlags
+// sets it.
+const AnnotationFilingReads = "filing_reads"
+
+// guardFilingFlags makes a command that moves or copies recordings refuse,
+// before it reads or sends anything, a container flag it inherits but never
+// reads. Such a flag would be ignored: todos copy --todolist 901 copied the
+// to-do into its own list and reported success. reads names the inherited
+// container flags the command does read, in some mode.
+//
+// A container flag is the root's --todolist or a flag that a command group
+// adds for its commands (--card-table, --vault, --list, …). --project and
+// --in are left to each command, which holds a filed recording to them; the
+// root's account, profile and output flags are not containers.
+func guardFilingFlags(cmd *cobra.Command, reads ...string) *cobra.Command {
+	if cmd.Annotations == nil {
+		cmd.Annotations = map[string]string{}
+	}
+	cmd.Annotations[AnnotationFilingReads] = strings.Join(reads, " ")
+	run := cmd.RunE
+	cmd.RunE = func(cmd *cobra.Command, args []string) error {
+		if err := refuseUnreadContainerFlags(cmd, reads); err != nil {
+			return err
+		}
+		return run(cmd, args)
+	}
+	return cmd
+}
+
+func refuseUnreadContainerFlags(cmd *cobra.Command, reads []string) error {
+	var unread *pflag.Flag
+	cmd.InheritedFlags().VisitAll(func(f *pflag.Flag) {
+		if unread == nil && f.Changed && isContainerFlag(cmd, f) && !slices.Contains(reads, f.Name) {
+			unread = f
+		}
+	})
+	if unread == nil {
+		return nil
+	}
+	hint := fmt.Sprintf("%s takes no destination; drop --%s", cmd.CommandPath(), unread.Name)
+	if dests := destinationFlags(cmd); len(dests) > 0 {
+		hint = fmt.Sprintf("Name where it goes with %s (see %s --help)", strings.Join(dests, " or "), cmd.CommandPath())
+	}
+	return output.ErrUsageHint(fmt.Sprintf("%s does not read --%s, so it would ignore it", cmd.CommandPath(), unread.Name), hint)
+}
+
+func isContainerFlag(cmd *cobra.Command, f *pflag.Flag) bool {
+	switch f.Name {
+	case "project", "in":
+		return false
+	case "todolist":
+		return true
+	}
+	return cmd.Root().PersistentFlags().Lookup(f.Name) != f
+}
+
+// destinationFlags lists a command's own flags that name where it files a
+// recording: --to (a folder or column) and the --to-<container> flags.
+func destinationFlags(cmd *cobra.Command) []string {
+	var dests []string
+	cmd.LocalNonPersistentFlags().VisitAll(func(f *pflag.Flag) {
+		if strings.HasPrefix(f.Name, "to-") || (f.Name == "to" && f.Value.Type() == "string") {
+			dests = append(dests, "--"+f.Name)
+		}
+	})
+	return dests
+}