Trim trailing newlines when reading env var value from file

[evanphx]

Summary

• When using KEY=@filepath to set env vars from files, trailing newlines (\r\n) are now trimmed from the file contents
• Uses strings.TrimRight with "\r\n" to only strip line endings, preserving intentional leading/trailing spaces

Fixes MIR-259

Test plan

• Added test case trims_trailing_newlines_from_file_value that writes a file with \r\n\n suffix and verifies it's trimmed
• All existing tests pass (make test — 3640 tests, 0 failures)

[coderabbitai]

📝 Walkthrough

Walkthrough

This change modifies how environment variables loaded from files are processed. When an environment variable is specified using the @file syntax (e.g., KEY=@file), the file contents now have trailing carriage return and newline characters removed before the value is assigned to the variable. A corresponding test case was added to verify this behavior, confirming that values read from files are trimmed appropriately.

📝 Coding Plan

• Generate coding plan for human review comments

---

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

[coderabbitai]

🧹 Nitpick comments (1)

cli/commands/env_test.go (1)

80-97: LGTM!

Good test coverage for the trailing newline trimming behavior. The test input "\r\n\n" exercises both CR and LF character trimming.

Optionally, consider adding edge case tests for completeness:

• File containing only newlines (should result in empty string)
• File with internal newlines like "line1\nline2\n" (should preserve internal, trim trailing)

These can be deferred if the current coverage is sufficient for the use case.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cli/commands/env_test.go` around lines 80 - 97, Add two additional unit tests
for ParseEnvVarSpecs to cover edge cases: one where the temp file contains only
newlines (e.g., "\n\r\n") and assert the parsed spec Value is an empty string,
and another where the file contains internal newlines (e.g., "line1\nline2\n")
and assert internal newlines are preserved while only the trailing newline is
trimmed; add these alongside the existing t.Run("trims trailing newlines from
file value", ...) test in env_test.go so they exercise the same
file-reading/path handling code path.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cli/commands/env_test.go`:
- Around line 80-97: Add two additional unit tests for ParseEnvVarSpecs to cover
edge cases: one where the temp file contains only newlines (e.g., "\n\r\n") and
assert the parsed spec Value is an empty string, and another where the file
contains internal newlines (e.g., "line1\nline2\n") and assert internal newlines
are preserved while only the trailing newline is trimmed; add these alongside
the existing t.Run("trims trailing newlines from file value", ...) test in
env_test.go so they exercise the same file-reading/path handling code path.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a5af7337-46e8-48cc-9fbe-faecd1534323

📥 Commits

Reviewing files that changed from the base of the PR and between 915ad02 and 21e7f2e.

📒 Files selected for processing (2)

• cli/commands/env.go
• cli/commands/env_test.go