From fdcdada1011c2a58ef1fea41488c61ac6ff8b8e3 Mon Sep 17 00:00:00 2001 From: AJ Alon Date: Thu, 20 Aug 2026 10:34:38 -0700 Subject: [PATCH] fix(task): name the real template markers in the not-in-template error `internal/task.IsTemplateDir` (a bare `.datarobot` os.Stat) was replaced by `repo.IsTemplateDir` in 21ff27c, which requires `.datarobot/answers`, or a `.datarobot/cli` holding something other than `state.yaml`. The error text was not updated with it, so a user tripping the stricter gate was told to create a `.datarobot` folder they were already standing in. Build the message from the same constants the predicate reads, and add a shared `repo.TemplateDetectStateFileName` so the `state.yaml` exemption is declared once instead of living as a bare literal in the predicate. Also point the user at `dr template setup` rather than leaving them to guess the remedy. Two tests pin the text to the predicate: one asserts the message names every marker `repo.IsTemplateDir` actually checks, the other asserts the directory shapes the message advertises -- and the two it excludes -- really do decide the gate that way. Both fail against the previous message. --- internal/repo/detect.go | 2 +- internal/repo/paths.go | 4 ++++ internal/task/discovery.go | 12 +++++++++-- internal/task/discovery_test.go | 37 +++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/internal/repo/detect.go b/internal/repo/detect.go index 6b29f6a99..c14bf7e3b 100644 --- a/internal/repo/detect.go +++ b/internal/repo/detect.go @@ -83,7 +83,7 @@ func IsTemplateDir(dir string) bool { // Older versions were incorrectly creating state.yaml file outside of template directories // return true if any file other than state.yaml exists in .datarobot/cli cliConfigDirPresent := slices.ContainsFunc(entries, func(entry os.DirEntry) bool { - return entry.Name() != "state.yaml" + return entry.Name() != TemplateDetectStateFileName }) if cliConfigDirPresent { diff --git a/internal/repo/paths.go b/internal/repo/paths.go index db7df1c55..86b8f7842 100644 --- a/internal/repo/paths.go +++ b/internal/repo/paths.go @@ -23,4 +23,8 @@ const ( QuickstartScriptPath = ".datarobot/cli/bin" // LocalPluginDir is the project-local plugin directory relative to CWD. LocalPluginDir = ".datarobot/cli/bin" + // TemplateDetectStateFileName is the one entry in DataRobotTemplateDetectCliPath + // that does not mark a directory as a template. Older CLI versions wrote it + // outside template directories, so its presence alone proves nothing. + TemplateDetectStateFileName = "state.yaml" ) diff --git a/internal/task/discovery.go b/internal/task/discovery.go index cd94a88d0..3f84a0828 100644 --- a/internal/task/discovery.go +++ b/internal/task/discovery.go @@ -202,9 +202,17 @@ func (d *Discovery) Discover(root string, maxDepth int) (string, error) { // and return cli.ErrSilent (or the returned error directly). func FormatDiscoveryError(err error) error { if errors.Is(err, ErrNotInTemplate) { - return fmt.Errorf("%s\n%s", + requirement := fmt.Sprintf( + "This command requires a '%s' folder, or a '%s' folder holding something other than '%s'.", + repo.DataRobotTemplateDetectAnswersPath, + repo.DataRobotTemplateDetectCliPath, + repo.TemplateDetectStateFileName, + ) + + return fmt.Errorf("%s\n%s\n%s", tui.BaseTextStyle.Render("You don't seem to be in a DataRobot Template directory."), - tui.BaseTextStyle.Render("This command requires a '.datarobot' folder to be present.")) + tui.BaseTextStyle.Render(requirement), + tui.BaseTextStyle.Render("Run 'dr template setup' to create one, or switch to an existing template directory.")) } if errors.Is(err, ErrTaskfileHasDotenv) { diff --git a/internal/task/discovery_test.go b/internal/task/discovery_test.go index c3109b552..1f29d6da0 100644 --- a/internal/task/discovery_test.go +++ b/internal/task/discovery_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "testing" + "github.com/datarobot/cli/internal/repo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" ) @@ -180,3 +181,39 @@ func (suite *DiscoveryTestSuite) TestFindComponentsSkipsHiddenDirs() { suite.Len(includes, 1) suite.Equal("visible", includes[0].Name) } + +// TestFormatDiscoveryErrorNamesRealTemplateMarkers guards the ErrNotInTemplate text +// against drifting away from repo.IsTemplateDir, which is what actually gates the +// command. The previous message promised a bare '.datarobot' folder was enough. +func (suite *DiscoveryTestSuite) TestFormatDiscoveryErrorNamesRealTemplateMarkers() { + msg := FormatDiscoveryError(ErrNotInTemplate).Error() + + suite.Contains(msg, repo.DataRobotTemplateDetectAnswersPath) + suite.Contains(msg, repo.DataRobotTemplateDetectCliPath) + suite.Contains(msg, repo.TemplateDetectStateFileName) +} + +// TestTemplateMarkersMatchAdvertisedRequirement pins the predicate to what the +// message tells the user to do: the advertised markers must pass, and the two +// shapes the message excludes must fail. +func (suite *DiscoveryTestSuite) TestTemplateMarkersMatchAdvertisedRequirement() { + bare := filepath.Join(suite.tempDir, "bare") + suite.Require().NoError(os.MkdirAll(filepath.Join(bare, ".datarobot"), 0o755)) + suite.False(repo.IsTemplateDir(bare), "a bare .datarobot folder must not satisfy the gate") + + answers := filepath.Join(suite.tempDir, "answers") + suite.Require().NoError(os.MkdirAll(filepath.Join(answers, repo.DataRobotTemplateDetectAnswersPath), 0o755)) + suite.True(repo.IsTemplateDir(answers), "the advertised answers folder must satisfy the gate") + + stateOnly := filepath.Join(suite.tempDir, "state-only") + stateCli := filepath.Join(stateOnly, repo.DataRobotTemplateDetectCliPath) + suite.Require().NoError(os.MkdirAll(stateCli, 0o755)) + suite.Require().NoError(os.WriteFile(filepath.Join(stateCli, repo.TemplateDetectStateFileName), []byte("{}"), 0o644)) + suite.False(repo.IsTemplateDir(stateOnly), "state.yaml alone must not satisfy the gate") + + withConfig := filepath.Join(suite.tempDir, "with-config") + configCli := filepath.Join(withConfig, repo.DataRobotTemplateDetectCliPath) + suite.Require().NoError(os.MkdirAll(configCli, 0o755)) + suite.Require().NoError(os.WriteFile(filepath.Join(configCli, "config.yaml"), []byte("{}"), 0o644)) + suite.True(repo.IsTemplateDir(withConfig), "cli config beyond state.yaml must satisfy the gate") +}