[AGENT-628] Unit tests

[pec1985]

Summary by CodeRabbit

• New Features

  • Automatically selects the appropriate environment file for local development vs. production.
  • Creates a development environment file when missing to streamline setup.
  • Improved parsing of environment files, including support for comments.
  • Prevents unintended syncing of environment variables to production during local development.

• Refactor

  • Centralized and simplified environment-processing logic for safer, clearer behavior.

• Tests

  • Added tests covering env selection, auto-creation, parsing, and production-sync gating.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Refactors env-file handling by adding DetermineEnvFilename, ParseAndProcessEnvFile, and ShouldSyncToProduction; ProcessEnvFiles now prefers/creates .env.development in local dev, parses and processes the chosen file, and only syncs envs to production when not in local-dev mode.

Changes

Cohort / File(s)	Summary
Env handling helpers & refactor internal/envutil/envutil.go	Added DetermineEnvFilename(dir, isLocalDev), ParseAndProcessEnvFile(logger, dir, envfilename, force), and ShouldSyncToProduction(isLocalDev). Refactored ProcessEnvFiles to use these helpers, prefer/create .env.development in local dev, and only call production sync when not local dev.
Unit tests for new behavior internal/envutil/envutil_test.go	Added tests TestShouldSyncToProduction and TestDetermineEnvFilename verifying sync gating and .env vs .env.development selection/creation and file contents.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant CLI
    participant EnvUtil as EnvUtil.ProcessEnvFiles
    participant FS as FileSystem
    participant Parser as ParseAndProcessEnvFile
    participant Deployer as HandleMissingProjectEnvs

    User->>CLI: run command
    CLI->>EnvUtil: ProcessEnvFiles(dir, isLocalDev, ...)
    EnvUtil->>FS: DetermineEnvFilename(dir, isLocalDev)
    alt local-dev & .env.development missing
        FS-->>EnvUtil: not found
        EnvUtil->>FS: create .env.development
        FS-->>EnvUtil: path -> .env.development
    else file exists
        FS-->>EnvUtil: path -> chosen env file
    end
    EnvUtil->>Parser: ParseAndProcessEnvFile(logger, dir, envfilename, force)
    Parser-->>EnvUtil: EnvFile (processed)
    alt ShouldSyncToProduction(isLocalDev=false)
        EnvUtil->>Deployer: HandleMissingProjectEnvs(...)
        Deployer-->>EnvUtil: sync result
    else isLocalDev=true
        Note over EnvUtil: skip production sync
    end
    EnvUtil-->>CLI: processed EnvFile / result

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Clarify env source usage for local dev vs production (.env vs .env.development) [AGENT-628]	✅
Avoid syncing to production when in local development [AGENT-628]	✅
Clarify prompt wording about “Would you like to set them now” to disambiguate target (project/production vs local) [AGENT-628]	❌	No changes to user-facing prompts or messaging in this diff.
Ensure local flow works without forcing production updates when declining prompts [AGENT-628]	✅

Possibly related PRs

• [AGENT-628] Unit tests #441 — Adds the same helpers (DetermineEnvFilename, ParseAndProcessEnvFile, ShouldSyncToProduction) and tests; strong overlap in modified functions.
• [AGENT-628] prevent local development env files from syncing to production #440 — Introduces guard to prevent syncing local-development envs to production; relates to the gating change here.
• When running dev mode, make sure we have a .env.development file #400 — Earlier adjustments to ProcessEnvFiles and .env.development handling; relevant historical context for this refactor.

Suggested reviewers

• potofpie
• jhaynie
• mhamidawad

Poem

I nibbled through configs soft and bright,
Found .env.development curled up at night.
“No prod sync today,” I thumped with glee—
Local burrow secrets stay just with me.
Hops of code, a tidy trail, 🥕🐇

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 01fa60b and 79069fc.

📒 Files selected for processing (1)

• internal/envutil/envutil.go (2 hunks)

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch AGENT-628-TESTS

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/envutil/envutil.go (1)

41-49: Create .env.development with 0600 perms and check write errors.

os.Create honors umask (often 022), which can leave the file world-readable. This file will likely hold secrets; default to 0600 and don’t ignore write errors.

-			// create it but don't load it -- this is required because uv expects a .env.development file to exist from the template
-			// but since its gitignore it won't get checked in and might not exist when you clone a project
-			of, err := os.Create(f)
-			if err != nil {
-				return "", err
-			}
-			defer of.Close()
-			of.WriteString("# This file is used to store development environment variables\n")
+			// Create (required by templates); safe to be empty. Use restrictive permissions.
+			if err := os.WriteFile(f, []byte("# This file is used to store development environment variables\n"), 0600); err != nil {
+				return "", err
+			}
 			envfilename = f

🧹 Nitpick comments (4)

internal/envutil/envutil_test.go (2)

82-108: Make this test parallel-safe.

Tests are table-driven and independent; run in parallel for speed.

 func TestShouldSyncToProduction(t *testing.T) {
+	t.Parallel()

---

110-189: Parallelize subtests and cover the “no files exist” case.

• Parallelization: safe, improves throughput.
• Add case where neither .env nor .env.development exists; function should still create .env.development.

 func TestDetermineEnvFilename(t *testing.T) {
+	t.Parallel()
@@
 	tests := []struct {
@@
 	}{
@@
 		{
 			name:            "local dev mode creates .env.development when it doesn't exist",
 			isLocalDev:      true,
 			existingFiles:   []string{".env"},
 			expectedFile:    ".env.development",
 			shouldCreateDev: true,
 		},
+		{
+			name:            "local dev mode creates .env.development when no files exist",
+			isLocalDev:      true,
+			existingFiles:   []string{},
+			expectedFile:    ".env.development",
+			shouldCreateDev: true,
+		},
 	}
@@
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()

internal/envutil/envutil.go (2)

41-43: Align comment with behavior (file is actually loaded).

The comment says “don’t load it,” but ProcessEnvFiles will parse it when it exists. Either update the comment or add logic to skip parsing newly created placeholder files. Given current behavior, updating the comment is simplest.

-			// create it but don't load it -- this is required because uv expects a .env.development file to exist from the template
-			// but since its gitignore it won't get checked in and might not exist when you clone a project
+			// Create it (required by templates). It's safe if empty; downstream parsing will just yield no variables.

Also applies to: 90-91, 96-99

---

83-87: Generalize the error context message.

This path can fail for reasons beyond create; make the message mode-agnostic.

-		errsystem.New(errsystem.ErrInvalidConfiguration, err, errsystem.WithContextMessage("Failed to create .env.development file")).ShowErrorAndExit()
+		errsystem.New(errsystem.ErrInvalidConfiguration, err, errsystem.WithContextMessage("Failed to prepare env file")).ShowErrorAndExit()

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 697720d and 01fa60b.

📒 Files selected for processing (2)

• internal/envutil/envutil.go (2 hunks)
• internal/envutil/envutil_test.go (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

internal/envutil/envutil_test.go (1)

internal/envutil/envutil.go (2)

• ShouldSyncToProduction (75-79)
• DetermineEnvFilename (34-53)

internal/envutil/envutil.go (5)

internal/deployer/deployer.go (1)

• EnvFile (15-18)

internal/project/project.go (1)

• ProjectData (44-53)

internal/errsystem/errorcodes.go (2)

• ErrInvalidConfiguration (25-28)
• ErrParseEnvironmentFile (41-44)

internal/errsystem/errsystem.go (1)

• WithContextMessage (100-104)

internal/util/io.go (1)

• Exists (14-19)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (3)

internal/envutil/envutil_test.go (1)

4-7: LGTM on added imports.

os and path/filepath are appropriate for the filesystem-based tests.

internal/envutil/envutil.go (2)

55-73: Helper extraction looks good.

Encapsulates template merge + parse cleanly; return type matches deployer.EnvFile usage.

---

74-80: Production sync gating is correct.

Clear, boolean-based guard avoids accidental local→prod sync.