Merge pull request #10289 from dolthub/tim/fix-rerquires-repo

Consistently implement which commands require a repo

go/cmd/dolt/cli/command.go

@@ -60,6 +60,22 @@ func hasHelpFlag(args []string) bool {
 	return false
 }
 
+// globalArgsSpecifyDB checks if the global arguments contain --use-db or --host flags,
+// which indicate that the user is manually specifying a database connection
+// rather than relying on the current directory being a dolt repository.
+func globalArgsSpecifyDB(cliCtx CliContext) bool {
+	if cliCtx == nil {
+		return false
+	}
+	globalArgs := cliCtx.GlobalArgs()
+	if globalArgs == nil {
+		return false
+	}
+	_, hasUseDb := globalArgs.GetValue("use-db")
+	_, hasHost := globalArgs.GetValue(HostFlag)
+	return hasUseDb || hasHost
+}
+
 // Command is the interface which defines a Dolt cli command
 type Command interface {
 	// Name returns the name of the Dolt cli command. This is what is used on the command line to invoke the command
@@ -220,7 +236,9 @@ func (hc SubCommandHandler) handleCommand(ctx context.Context, commandStr string
 		cmdRequiresRepo = rnrCmd.RequiresRepo()
 	}
 
-	if cmdRequiresRepo && !hasHelpFlag(args) {
+	// If --use-db or --host is specified, the user is manually specifying a database/server
+	// connection, so we don't require the current directory to be a valid dolt repository.
+	if cmdRequiresRepo && !hasHelpFlag(args) && !globalArgsSpecifyDB(cliCtx) {
 		isValid := CheckEnvIsValid(dEnv)
 		if !isValid {
 			return 2

go/cmd/dolt/commands/add.go

@@ -53,12 +53,6 @@ The dolt status command can be used to obtain a summary of which tables have cha
 
 type AddCmd struct{}
 
-var _ cli.RepoNotRequiredCommand = AddCmd{}
-
-func (cmd AddCmd) RequiresRepo() bool {
-	return false
-}
-
 // Name is returns the name of the Dolt cli command. This is what is used on the command line to invoke the command
 func (cmd AddCmd) Name() string {
 	return "add"

go/cmd/dolt/commands/blame.go

@@ -65,12 +65,6 @@ func (cmd BlameCmd) ArgParser() *argparser.ArgParser {
 	return ap
 }
 
-func (cmd BlameCmd) RequiresRepo() bool {
-	return false
-}
-
-var _ cli.RepoNotRequiredCommand = BlameCmd{}
-
 // EventType returns the type of the event to log
 func (cmd BlameCmd) EventType() eventsapi.ClientEventType {
 	return eventsapi.ClientEventType_BLAME

go/cmd/dolt/commands/clean.go

@@ -66,10 +66,6 @@ func (cmd CleanCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateCleanArgParser()
 }
 
-func (cmd CleanCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd CleanCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int {
 	ap := cli.CreateCleanArgParser()

go/cmd/dolt/commands/commit.go

@@ -80,10 +80,6 @@ func (cmd CommitCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateCommitArgParser(false)
 }
 
-func (cmd CommitCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd CommitCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int {
 	res, skipped := performCommit(ctx, commandStr, args, cliCtx, dEnv)

go/cmd/dolt/commands/config.go

@@ -91,8 +91,6 @@ func (cmd ConfigCmd) Description() string {
 	return "Dolt configuration."
 }
 
-// RequiresRepo should return false if this interface is implemented, and the command does not have the requirement
-// that it be run from within a data repository directory
 func (cmd ConfigCmd) RequiresRepo() bool {
 	return false
 }

go/cmd/dolt/commands/debug.go

@@ -89,14 +89,6 @@ func (cmd DebugCmd) EventType() eventsapi.ClientEventType {
 	return eventsapi.ClientEventType_SQL
 }
 
-// RequiresRepo indicates that this command does not have to be run from within a dolt data repository directory.
-// In this case it is because this command supports the DataDirFlag which can pass in a directory.  In the event that
-// that parameter is not provided there is additional error handling within this command to make sure that this was in
-// fact run from within a dolt data repository directory.
-func (cmd DebugCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 // Unlike other commands, sql doesn't set a new working root directly, as the SQL layer updates the working set as
 // necessary when committing work.

go/cmd/dolt/commands/diff.go

@@ -294,10 +294,6 @@ func (cmd DiffCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateDiffArgParser(false)
 }
 
-func (cmd DiffCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd DiffCmd) Exec(ctx context.Context, commandStr string, args []string, _ *env.DoltEnv, cliCtx cli.CliContext) int {
 	ap := cmd.ArgParser()

go/cmd/dolt/commands/fetch.go

@@ -70,10 +70,6 @@ func (cmd FetchCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateFetchArgParser()
 }
 
-func (cmd FetchCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd FetchCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int {
 	ap := cli.CreateFetchArgParser()

go/cmd/dolt/commands/log.go

@@ -89,10 +89,6 @@ func (cmd LogCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateLogArgParser(false)
 }
 
-func (cmd LogCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd LogCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int {
 	return cmd.logWithLoggerFunc(ctx, commandStr, args, dEnv, cliCtx)

go/cmd/dolt/commands/merge.go

@@ -86,10 +86,6 @@ func (cmd MergeCmd) EventType() eventsapi.ClientEventType {
 	return eventsapi.ClientEventType_MERGE
 }
 
-func (cmd MergeCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd MergeCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int {
 	ap := cli.CreateMergeArgParser()

go/cmd/dolt/commands/read_tables.go

@@ -66,8 +66,6 @@ func (cmd ReadTablesCmd) Docs() *cli.CommandDocumentation {
 	return cli.NewCommandDocumentation(readTablesDocs, ap)
 }
 
-// RequiresRepo should return false if this interface is implemented, and the command does not have the requirement
-// that it be run from within a data repository directory
 func (cmd ReadTablesCmd) RequiresRepo() bool {
 	return false
 }

go/cmd/dolt/commands/reflog.go

@@ -72,10 +72,6 @@ func (cmd ReflogCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateReflogArgParser()
 }
 
-func (cmd ReflogCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd ReflogCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int {
 	ap := cmd.ArgParser()

go/cmd/dolt/commands/reset.go

@@ -84,10 +84,6 @@ func (cmd ResetCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateResetArgParser()
 }
 
-func (cmd ResetCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd ResetCmd) Exec(ctx context.Context, commandStr string, args []string, _ *env.DoltEnv, cliCtx cli.CliContext) int {
 	ap := cli.CreateResetArgParser()

go/cmd/dolt/commands/revert.go

@@ -52,10 +52,6 @@ func (cmd RevertCmd) Name() string {
 	return "revert"
 }
 
-func (cmd RevertCmd) RequiresRepo() bool {
-	return false
-}
-
 // Description implements the interface cli.Command.
 func (cmd RevertCmd) Description() string {
 	return "Undo the changes introduced in a commit."

go/cmd/dolt/commands/rm.go

@@ -43,10 +43,6 @@ The dolt status command can be used to obtain a summary of which tables have cha
 
 type RmCmd struct{}
 
-func (cmd RmCmd) RequiresRepo() bool {
-	return false
-}
-
 // Name is returns the name of the Dolt cli command. This is what is used on the command line to invoke the command
 func (cmd RmCmd) Name() string {
 	return "rm"

go/cmd/dolt/commands/roots.go

@@ -54,12 +54,6 @@ func (cmd RootsCmd) Hidden() bool {
 	return true
 }
 
-// RequiresRepo should return false if this interface is implemented, and the command does not have the requirement
-// that it be run from within a data repository directory
-func (cmd RootsCmd) RequiresRepo() bool {
-	return false
-}
-
 // Description returns a description of the command
 func (cmd RootsCmd) Description() string {
 	return "Displays store root values (or potential store root values) that we find in the current database."

go/cmd/dolt/commands/send_metrics.go

@@ -54,12 +54,6 @@ func (cmd SendMetricsCmd) Description() string {
 	return "Send events logs to server."
 }
 
-// RequiresRepo should return false if this interface is implemented, and the command does not have the requirement
-// that it be run from within a data repository directory
-func (cmd SendMetricsCmd) RequiresRepo() bool {
-	return false
-}
-
 // Hidden should return true if this command should be hidden from the help text
 func (cmd SendMetricsCmd) Hidden() bool {
 	return true

go/cmd/dolt/commands/show.go

@@ -97,10 +97,6 @@ func (cmd ShowCmd) ArgParser() *argparser.ArgParser {
 	return ap
 }
 
-func (cmd ShowCmd) RequiresRepo() bool {
-	return false
-}
-
 // Exec executes the command
 func (cmd ShowCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int {
 	ap := cmd.ArgParser()

go/cmd/dolt/commands/status.go

@@ -68,11 +68,6 @@ var statusDocs = cli.CommandDocumentationContent{
 
 type StatusCmd struct{}
 
-func (cmd StatusCmd) RequiresRepo() bool {
-	return false
-}
-
-var _ cli.RepoNotRequiredCommand = StatusCmd{}
 var _ cli.EventMonitoredCommand = StatusCmd{}
 
 // Name is returns the name of the Dolt cli command. This is what is used on the command line to invoke the command

integration-tests/bats/helper/local-remote.bash

@@ -143,6 +143,7 @@ SKIP_SERVER_TESTS=$(cat <<-EOM
 ~nonlocal.bats~
 ~branch-activity.bats~
 ~mutual-tls-auth.bats~
+~requires-repo.bats~
 EOM
 )
 

integration-tests/bats/init.bats

@@ -273,7 +273,7 @@ teardown() {
 
     # Assert that the current directory has NOT been initialized
     run dolt status
-    [ $status -eq 1 ]
+    [ $status -eq 2 ]
     [[ $output =~ "The current directory is not a valid dolt repository" ]] || false
     [ ! -d "$baseDir/not_a_repo/.dolt" ]
 

integration-tests/bats/requires-repo.bats

@@ -0,0 +1,223 @@
+#!/usr/bin/env bats
+# Tests for CLI behavior when running commands from a parent directory
+# that contains a child dolt database directory.
+# See https://github.com/dolthub/dolt/issues/10230
+
+load $BATS_TEST_DIRNAME/helper/common.bash
+load $BATS_TEST_DIRNAME/helper/query-server-common.bash
+
+setup() {
+    skiponwindows "tests are flaky on Windows"
+    if [ "$SQL_ENGINE" = "remote-engine" ]; then
+      skip "This test tests local CLI behavior, SQL_ENGINE is not needed."
+    fi
+    setup_no_dolt_init
+    # Create a child database directory
+    mkdir child_db
+    cd child_db
+    dolt init
+    dolt sql -q "CREATE TABLE test_table (pk INT PRIMARY KEY, value VARCHAR(100))"
+    dolt add .
+    dolt commit -m "Initial commit"
+    cd ..
+    # We are now in the parent directory with a child dolt database
+}
+
+teardown() {
+    stop_sql_server 1 && sleep 0.5
+    teardown_common
+}
+
+NOT_VALID_REPO_ERROR="The current directory is not a valid dolt repository."
+
+# =============================================================================
+# Tests WITHOUT a running SQL server
+# All commands that require a repo should fail consistently from parent directory
+# =============================================================================
+
+@test "requires-repo: dolt status from parent dir without server fails" {
+    run dolt status
+    [ "$status" -ne 0 ]
+    [ "${lines[0]}" = "$NOT_VALID_REPO_ERROR" ]
+}
+
+@test "requires-repo: dolt checkout from parent dir without server fails" {
+    run dolt checkout -b new_branch
+    [ "$status" -ne 0 ]
+    [ "${lines[0]}" = "$NOT_VALID_REPO_ERROR" ]
+}
+
+@test "requires-repo: dolt branch from parent dir without server fails" {
+    run dolt branch
+    [ "$status" -ne 0 ]
+    [ "${lines[0]}" = "$NOT_VALID_REPO_ERROR" ]
+}
+
+@test "requires-repo: dolt log from parent dir without server fails" {
+    run dolt log
+    [ "$status" -ne 0 ]
+    [ "${lines[0]}" = "$NOT_VALID_REPO_ERROR" ]
+}
+
+@test "requires-repo: dolt diff from parent dir without server fails" {
+    run dolt diff
+    [ "$status" -ne 0 ]
+    [ "${lines[0]}" = "$NOT_VALID_REPO_ERROR" ]
+}
+
+@test "requires-repo: dolt add from parent dir without server fails" {
+    run dolt add .
+    [ "$status" -ne 0 ]
+    [ "${lines[0]}" = "$NOT_VALID_REPO_ERROR" ]
+}
+
+@test "requires-repo: dolt commit from parent dir without server fails" {
+    run dolt commit -m "test"
+    [ "$status" -ne 0 ]
+    [ "${lines[0]}" = "$NOT_VALID_REPO_ERROR" ]
+}
+
+# =============================================================================
+# Tests WITH a running SQL server
+# Commands should still fail from parent directory - they require a local repo
+# =============================================================================
+
+@test "requires-repo: dolt status from parent dir with running server fails" {
+    start_multi_db_server child_db
+
+    # Status should fail - it requires being in a dolt repo directory
+    run dolt status
+    [ "$status" -ne 0 ]
+    [[ "$output" =~ "$NOT_VALID_REPO_ERROR" ]] || false
+}
+
+@test "requires-repo: dolt checkout from parent dir with running server fails" {
+    start_multi_db_server child_db
+
+    run dolt checkout -b new_branch
+    [ "$status" -ne 0 ]
+    [[ "$output" =~ "$NOT_VALID_REPO_ERROR" ]] || false
+}
+
+@test "requires-repo: dolt branch from parent dir with running server fails" {
+    start_multi_db_server child_db
+
+    run dolt branch
+    [ "$status" -ne 0 ]
+    [[ "$output" =~ "$NOT_VALID_REPO_ERROR" ]] || false
+}
+
+@test "requires-repo: dolt log from parent dir with running server fails" {
+    start_multi_db_server child_db
+
+    run dolt log
+    [ "$status" -ne 0 ]
+    [[ "$output" =~ "$NOT_VALID_REPO_ERROR" ]] || false
+}
+
+@test "requires-repo: dolt diff from parent dir with running server fails" {
+    start_multi_db_server child_db
+
+    run dolt diff
+    [ "$status" -ne 0 ]
+    [[ "$output" =~ "$NOT_VALID_REPO_ERROR" ]] || false
+}
+
+# =============================================================================
+# Commands that do NOT require a repo should work from parent directory
+# =============================================================================
+
+@test "requires-repo: dolt sql from parent dir with running server succeeds" {
+    start_multi_db_server child_db
+
+    # SQL commands should work by connecting to the running server
+    run dolt sql -q "SELECT * FROM child_db.test_table"
+    [ "$status" -eq 0 ]
+}
+
+@test "requires-repo: dolt version from parent dir succeeds" {
+    run dolt version
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "dolt version" ]] || false
+}
+
+@test "requires-repo: dolt init in new dir succeeds" {
+    # Test that dolt init works in a new directory (doesn't require existing repo)
+    mkdir new_init_test
+    cd new_init_test
+    run dolt init
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "Successfully initialized dolt data repository" ]] || false
+    cd ..
+    rm -rf new_init_test
+}
+
+# =============================================================================
+# Commands should work correctly when run from within the child database directory
+# =============================================================================
+
+@test "requires-repo: commands in child directory work normally" {
+    cd child_db
+
+    run dolt status
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "On branch main" ]] || false
+
+    run dolt checkout -b new_branch
+    [ "$status" -eq 0 ]
+
+    run dolt branch
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "new_branch" ]] || false
+    [[ "$output" =~ "main" ]] || false
+
+    run dolt log
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "Initial commit" ]] || false
+
+    run dolt diff
+    [ "$status" -eq 0 ]
+}
+
+# =============================================================================
+# Test consistent behavior - all repo-requiring commands should fail the same way
+# =============================================================================
+
+@test "requires-repo: all repo-requiring commands fail consistently" {
+    # All these commands should fail with the same error
+    for cmd in "status" "branch" "log" "diff" "add ." "commit -m test" "checkout -b test"; do
+        run dolt $cmd
+        [ "$status" -ne 0 ]
+        [[ "${lines[0]}" = "$NOT_VALID_REPO_ERROR" ]] || false
+    done
+}
+
+# =============================================================================
+# Tests with --use-db or --host flags bypass repo requirement
+# These flags indicate a manual database/server connection, so the local
+# directory check should be bypassed.
+# =============================================================================
+
+@test "requires-repo: --host and --use-db flags bypass repo requirement for log" {
+    start_multi_db_server child_db
+
+    # With --host and --use-db, log should work from parent directory
+    run dolt --host 127.0.0.1 --port $PORT --no-tls --use-db child_db log
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "Initial commit" ]] || false
+}
+
+@test "requires-repo: --host and --use-db flags bypass repo requirement for branch" {
+    start_multi_db_server child_db
+
+    run dolt --host 127.0.0.1 --port $PORT --no-tls --use-db child_db branch
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "main" ]] || false
+}
+
+@test "requires-repo: --host and --use-db flags bypass repo requirement for diff" {
+    start_multi_db_server child_db
+
+    run dolt --host 127.0.0.1 --port $PORT --no-tls --use-db child_db diff
+    [ "$status" -eq 0 ]
+}

integration-tests/bats/shallow-clone.bats

@@ -63,6 +63,7 @@ seed_and_start_serial_remote() {
     cd clones
 
     dolt sql -q "call dolt_clone('--depth', '1','http://localhost:50051/test-org/test-repo')"
+    cd test-repo
 
     run dolt log --oneline --decorate=no
     [ "$status" -eq 0 ]
@@ -85,6 +86,7 @@ seed_and_start_serial_remote() {
     cd clones
     run dolt sql -q "call dolt_clone('--depth', '2','http://localhost:50051/test-org/test-repo')"
     [ "$status" -eq 0 ]
+    cd test-repo
 
     run dolt log --oneline --decorate=no
     [ "$status" -eq 0 ]
@@ -111,6 +113,7 @@ seed_and_start_serial_remote() {
     cd clones
     run dolt sql -q "call dolt_clone('--depth', '1','file://../file-remote')"
     [ "$status" -eq 0 ]
+    cd file-remote
 
     run dolt log --oneline --decorate=no
     [ "$status" -eq 0 ]

integration-tests/bats/sql-local-remote.bats

@@ -199,9 +199,11 @@ get_commit_hash_at() {
     dolt sql -q "insert into test values (2), (3)"
     dolt add test
     dolt commit -m "insert more values into test"
-    cd ..
 
+    cd ..
     start_sql_server altDB
+    cd altDB
+
     run dolt blame test
     [ "$status" -eq 0 ]
     export out="$output"
@@ -269,6 +271,7 @@ get_commit_hash_at() {
 
 @test "sql-local-remote: test 'status' and switch between server/no server" {
   start_sql_server defaultDB
+  cd defaultDB
 
   run dolt status
   [ "$status" -eq 0 ] || false
@@ -323,9 +326,10 @@ get_commit_hash_at() {
     dolt sql -q "create table test1 (pk int primary key)"
     dolt sql -q "create table test2 (pk int primary key)"
     dolt add test1
-    cd ..
 
+    cd ..
     start_sql_server altDB
+    cd altDB
 
     run dolt --verbose-engine-setup commit -m "committing remotely"
     [ "$status" -eq 0 ]
@@ -333,20 +337,16 @@ get_commit_hash_at() {
 
     stop_sql_server 1
 
-    cd altDB
     run dolt log
     [ "$status" -eq 0 ]
     [[ "$output" =~ "committing remotely" ]] || false
 
     run dolt add test2
     [ "$status" -eq 0 ]
-    cd ..
 
     run dolt --verbose-engine-setup commit -m "committing locally"
     [ "$status" -eq 0 ]
     [[ "$output" =~ "starting local mode" ]] || false
-
-    cd altDB
     run dolt log
     [ "$status" -eq 0 ]
     [[ "$output" =~ "committing locally" ]] || false
@@ -691,6 +691,8 @@ SQL
 
 @test "sql-local-remote: verify simple dolt reset behavior" {
     start_sql_server altDB
+    cd altDB
+
     dolt sql -q "create table test1 (pk int primary key)"
     dolt add test1
     dolt commit -m "create table test1"
@@ -781,20 +783,23 @@ SQL
     [ $status -eq 0 ]
     [[ "$output" =~ 'Revert "Commit ABCDEF"' ]] || false
 
-    dolt reset --hard HEAD~1
+    dolt --use-db altDB reset --hard HEAD~1
 
     stop_sql_server 1
 
-    run dolt revert HEAD
+    run dolt --use-db altDB revert HEAD
     [ $status -eq 0 ]
     [[ $output =~ 'Revert "Commit ABCDEF"' ]] || false
 }
 
 @test "sql-local-remote: Ensure that dolt clean works for each mode" {
+    cd altDB
     dolt reset --hard
     dolt sql -q "create table tbl (pk int primary key)"
 
+    cd ..
     start_sql_server altDB
+    cd altDB
 
     run dolt --verbose-engine-setup clean --dry-run
     [ $status -eq 0 ]