Skip to content

Comments

Remove RemoteProjectName#1919

Merged
lionello merged 6 commits intomainfrom
jordan/remove-remote-project-name
Feb 11, 2026
Merged

Remove RemoteProjectName#1919
lionello merged 6 commits intomainfrom
jordan/remove-remote-project-name

Conversation

@jordanstephens
Copy link
Member

@jordanstephens jordanstephens commented Feb 10, 2026

Description

Coderabbit suggested this issue in #1914 (comment)_

The "too many projects error" is only returned by playground, and playground has its own implementation of RemoteProjectName, so coderabbit wasn't entirely correct. However, this function is reachable if the project name is not available from the loader, or in the flagset, and it is potentially confusing in that it returns the first project out of a seemingly unsorted list retrieved from the cd s3 bucket. This function has been marked deprecated for the base byoc client for a while now, so this PR aims to rip it out by taking the following steps:

  • Handle "too many projects" error explicitly with a playground provider
  • Remove LoadProjectNameWithFallback and load project name directly from the loader
  • I renamed the CLIInterface method LoadProjectNameWithFallback to LoadProjectName, but I did not inline it, because it is still used for mocking in tests.

Linked Issues

Checklist

  • I have performed a self-review of my code
  • I have added appropriate tests
  • I have updated the Defang CLI docs and/or README to reflect my changes, if necessary

Summary by CodeRabbit

Release Notes

  • Refactor
    • Simplified project name resolution logic by removing fallback mechanisms and consolidating project discovery methods. Commands now use a more direct approach to identify projects, which may affect how the CLI resolves project names in certain scenarios.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

📝 Walkthrough

Walkthrough

This PR removes the fallback-based project name loading mechanism (LoadProjectNameWithFallback) across the codebase and replaces it with a direct loader-based approach (LoadProjectName). The changes include removing the fallback function, updating all call sites in CLI commands and agent tools, removing the deprecated RemoteProjectName method from the provider interface, and updating corresponding test mocks.

Changes

Cohort / File(s) Summary
CLI Commands
src/cmd/cli/command/cd.go, src/cmd/cli/command/compose.go, src/cmd/cli/command/config.go
Updated call sites to use session.Loader.LoadProjectName(ctx) instead of LoadProjectNameWithFallback; introduced consistent local ctx variable usage throughout; adjusted error handling in configDeleteCmd for not-found cases.
Agent Tools Implementation
src/pkg/agent/tools/default_tool_cli.go, src/pkg/agent/tools/destroy.go, src/pkg/agent/tools/listConfig.go, src/pkg/agent/tools/logs.go, src/pkg/agent/tools/removeConfig.go, src/pkg/agent/tools/services.go, src/pkg/agent/tools/setConfig.go
Replaced method calls from LoadProjectNameWithFallback(ctx, loader, provider) to LoadProjectName(ctx, loader); updated corresponding debug log statements.
Agent Tools Tests
src/pkg/agent/tools/destroy_test.go, src/pkg/agent/tools/listConfig_test.go, src/pkg/agent/tools/removeConfig_test.go, src/pkg/agent/tools/services_test.go, src/pkg/agent/tools/setConfig_test.go
Updated mock implementations to define LoadProjectName(ctx, loader) method signature removing provider parameter; adjusted logging and test expectations accordingly.
Agent Tools Interface
src/pkg/agent/tools/interfaces.go
Removed LoadProjectNameWithFallback method from CLIInterface; added LoadProjectName method with simplified signature.
Client Layer
src/pkg/cli/client/projectName.go, src/pkg/cli/client/projectName_test.go, src/pkg/cli/client/provider.go, src/pkg/cli/client/byoc/baseclient.go
Deleted projectName.go and projectName_test.go files containing old fallback implementation; removed RemoteProjectName method from provider interface and BYOC client implementation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #1820: Updates Loader.LoadProjectName return signature and adjusts all callers to handle the modified return values, paralleling this PR's call-site updates.
  • PR #1721: Refactors loader/CLI interfaces and project-name loading mechanisms alongside Loader.TargetDirectory addition and related API updates across the same subsystems.
  • PR #1754: Replaces LoadProjectNameWithFallback with session-based Loader.LoadProjectName wiring across the same command call sites.

Suggested reviewers

  • lionello

Poem

🐰 Hop away, old fallback friend,
Direct paths now lead to the end,
Provider's grip we've gently freed,
Loader's clarity is all we need,
Simple, pure, and refactored clean! 🎯

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Remove RemoteProjectName' directly and accurately describes the primary change in the changeset, which removes the RemoteProjectName method from the Provider interface and associated fallback logic.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jordan/remove-remote-project-name

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/pkg/agent/tools/services_test.go (1)

26-26: Rename field to match the new method name.

The field LoadProjectNameWithFallbackError should be renamed to LoadProjectNameError to match the renamed method LoadProjectName. This inconsistency reduces code readability and could cause confusion.

♻️ Proposed fix
 type MockCLI struct {
 	CLIInterface
 	ConnectError                     error
-	LoadProjectNameWithFallbackError error
+	LoadProjectNameError             error
 	MockClient                       *client.GrpcClient

And update the reference in the method (line 50):

 func (m *MockCLI) LoadProjectName(ctx context.Context, loader client.Loader) (string, error) {
-	if m.LoadProjectNameWithFallbackError != nil {
-		return "", m.LoadProjectNameWithFallbackError
+	if m.LoadProjectNameError != nil {
+		return "", m.LoadProjectNameError
 	}

And update the test case (line 175):

 		{
 			name: "load_project_name_error",
 			mockCLI: &MockCLI{
 				MockClient:                       &client.GrpcClient{},
 				MockProvider:                     &client.PlaygroundProvider{},
-				LoadProjectNameWithFallbackError: errors.New("failed to load project name"),
+				LoadProjectNameError:             errors.New("failed to load project name"),
 			},

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter"
level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

@lionello lionello merged commit 98a5212 into main Feb 11, 2026
14 checks passed
@lionello lionello deleted the jordan/remove-remote-project-name branch February 11, 2026 22:18
lionello added a commit that referenced this pull request Feb 12, 2026
lionello added a commit that referenced this pull request Feb 12, 2026
lionello added a commit that referenced this pull request Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants