feat: add set-env command to manage environment variables #315
Conversation
- Add SetEnvironmentVariableOptions and EnvironmentVariableService - Update Program.cs to register the command - Add unit tests for various cases - Update documentation includes inline comments, README.md, and any related markdown files.
Reviewer's GuideImplements a new Sequence diagram for set-env command executionsequenceDiagram
actor User
participant Cli as dotnet_aicommitmessage
participant Parser
participant Program
participant EnvService as EnvironmentVariableService
participant SystemEnv as Environment
participant Output
User->>Cli: set-env VAR_NAME=value --target User
Cli->>Parser: ParseArguments(args)
Parser-->>Program: SetEnvironmentVariableOptions instance
Program->>Program: Run(options)
Program->>EnvService: SetEnvironmentVariable(options)
EnvService->>EnvService: Validate Variable not null or empty
EnvService->>EnvService: Split Variable into name and value
EnvService->>EnvService: Validate variableName not empty
EnvService->>EnvService: Resolve EnvironmentVariableTarget (User or Machine)
EnvService->>SystemEnv: SetEnvironmentVariable(name, value, target)
SystemEnv-->>EnvService: success or exception
alt success
EnvService->>Output: InfoLine(Environment variable set message)
else security exception
EnvService->>Output: ErrorLine(Permission denied)
EnvService->>SystemEnv: set ExitCode = 1
else invalid argument or other error
EnvService->>Output: ErrorLine(error message)
EnvService->>SystemEnv: set ExitCode = 1
end
Class diagram for new set-env options and serviceclassDiagram
class Program {
+Main(string[] args)
-Run(object options)
}
class SetEnvironmentVariableOptions {
+string Variable
+string Target
}
class EnvironmentVariableService {
+SetEnvironmentVariable(SetEnvironmentVariableOptions setEnvironmentVariableOptions) void
-ErrorLine(string message) void
}
class Output {
+ErrorLine(string message) void
+InfoLine(string message) void
}
class Environment {
+SetEnvironmentVariable(string variableName, string variableValue, EnvironmentVariableTarget envTarget) void
+ExitCode int
}
class EnvironmentVariableTarget {
<<enumeration>>
User
Machine
}
Program ..> SetEnvironmentVariableOptions : parses
Program ..> EnvironmentVariableService : calls
EnvironmentVariableService ..> SetEnvironmentVariableOptions : uses
EnvironmentVariableService ..> Output : uses
EnvironmentVariableService ..> Environment : uses
Environment ..> EnvironmentVariableTarget : uses
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughImplements a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI Parser (Program)
participant Service as EnvironmentVariableService
participant OS as Environment (Runtime/OS)
User->>CLI: run "set-env VAR_NAME=value --target User?"
CLI->>Service: new SetEnvironmentVariableOptions(Variable, Target)
CLI->>Service: Invoke SetEnvironmentVariable(options)
Service->>Service: Validate "VAR=value" format\nParse name and value\nResolve target (User/Machine)
Service->>OS: Environment.SetEnvironmentVariable(name, value, target)
OS-->>Service: Success / throws exception
Service-->>CLI: Write success or ErrorLine + set ExitCode=1
CLI-->>User: Console output (success or error)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Here's the code health analysis summary for commits Analysis Summary
|
gstraccini
bot
added
enhancement
![gstraccini[bot]](https://avatars.githubusercontent.com/in/480132?s=60&v=4){kind=small}
gstraccini
bot
added
the
🚦 awaiting triage
penify-dev
bot
added
Tests
Review effort [1-5]: 4
PR Review 🔍
|
PR Code Suggestions ✨
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
General comments:
- Consider treating a null or empty
SetEnvironmentVariableOptions.Targetas the defaultUsertarget insideEnvironmentVariableServiceso the service is robust even when used outside the CommandLineParser context. - Relying on
Environment.ExitCodemutation insideEnvironmentVariableServicecouples the service to process-level concerns; returning a status or throwing exceptions and lettingProgramdecide the exit code would keep the service more reusable and easier to compose.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider treating a null or empty `SetEnvironmentVariableOptions.Target` as the default `User` target inside `EnvironmentVariableService` so the service is robust even when used outside the CommandLineParser context.
- Relying on `Environment.ExitCode` mutation inside `EnvironmentVariableService` couples the service to process-level concerns; returning a status or throwing exceptions and letting `Program` decide the exit code would keep the service more reusable and easier to compose.
## Individual Comments
### Comment 1
<location> `Src/AiCommitMessage/Services/EnvironmentVariableService.cs:55-64` </location>
<code_context>
+ }
+ else
+ {
+ ErrorLine($"Invalid target '{setEnvironmentVariableOptions.Target}'. Please use 'User' or 'Machine'.");
+ return;
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align target handling with the documented default-to-User behavior and be more robust to null/empty values.
The XML remarks state this should default to User scope if no target is specified, but the current logic errors on anything not exactly "User" or "Machine", including null/empty targets. To align with the docs and `[Option(..., Default = "User")]`, normalize `setEnvironmentVariableOptions.Target` first (treat null/whitespace as "User") and then validate/branch on that normalized value.
```suggestion
var target = string.IsNullOrWhiteSpace(setEnvironmentVariableOptions.Target)
? "User"
: setEnvironmentVariableOptions.Target;
if (string.Equals(target, "Machine", StringComparison.OrdinalIgnoreCase))
{
envTarget = EnvironmentVariableTarget.Machine;
}
else if (string.Equals(target, "User", StringComparison.OrdinalIgnoreCase))
{
envTarget = EnvironmentVariableTarget.User;
}
else
{
ErrorLine($"Invalid target '{setEnvironmentVariableOptions.Target}'. Please use 'User' or 'Machine'.");
return;
}
```
</issue_to_address>
### Comment 2
<location> `Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs:37-14` </location>
<code_context>
+ /// Tests that setting a User-level environment variable works when target is not specified (default).
+ /// </summary>
+ [Fact]
+ public void SetEnvironmentVariable_Should_SetUserVariable_When_TargetIsNotSpecified()
+ {
+ var options = new SetEnvironmentVariableOptions
+ {
+ Variable = "TEST_DEFAULT_VAR=default_value",
</code_context>
<issue_to_address>
**issue (testing):** This test claims to cover the default target behavior but still sets `Target` explicitly to `"User"`, so it does not actually validate the default value.
To validate the default behavior, construct `SetEnvironmentVariableOptions` without setting `Target` (or set it to `null` if you want to test defensive handling) and assert that the variable is written using `EnvironmentVariableTarget.User`. In its current form, this test just repeats the explicit-User case and doesn’t prove the default is applied.
</issue_to_address>
### Comment 3
<location> `Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs:70-79` </location>
<code_context>
+ Target = "User"
+ };
+
+ var originalExitCode = Environment.ExitCode;
+
+ try
+ {
+ EnvironmentVariableService.SetEnvironmentVariable(options);
+
+ Environment.ExitCode.Should().Be(1);
+ }
+ finally
+ {
+ Environment.ExitCode = originalExitCode;
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (testing):** Multiple tests mutate the global `Environment.ExitCode`, which can cause flakiness if tests are ever run in parallel.
Although each test locally saves/restores `Environment.ExitCode`, they still share a single global value, which is risky if tests run in parallel or more tests start using it. It would help to extract this pattern into a reusable helper (e.g., an `IDisposable` that manages the exit code) and/or mark these tests as non-parallel (e.g., via a collection fixture) to prevent cross-test interference.
Suggested implementation:
```csharp
var options = new SetEnvironmentVariableOptions
{
Variable = "INVALID_FORMAT",
Target = "User"
};
using (new ExitCodeScope())
{
EnvironmentVariableService.SetEnvironmentVariable(options);
Environment.ExitCode.Should().Be(1);
}
```
```csharp
private sealed class ExitCodeScope : IDisposable
{
private readonly int _originalExitCode;
public ExitCodeScope()
{
_originalExitCode = Environment.ExitCode;
}
public void Dispose()
{
Environment.ExitCode = _originalExitCode;
}
}
/// <summary>
/// Tests that setting a User-level environment variable works when target is not specified (default).
/// </summary>
[Fact]
```
1. Replace any other tests in `EnvironmentVariableServiceTests` (or elsewhere) that manually capture/restore `Environment.ExitCode` with `using (new ExitCodeScope()) { ... }` to centralize and harden this pattern.
2. To fully address cross-test interference, consider defining a non-parallel xUnit collection for all tests that touch `Environment.ExitCode`:
- Add a `[CollectionDefinition("EnvironmentExitCode", DisableParallelization = true)]` class in the test project.
- Annotate the relevant test class(es) with `[Collection("EnvironmentExitCode")]`.
</issue_to_address>
### Comment 4
<location> `README.md:141` </location>
<code_context>
sk-abc123xyz
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (1)
46-47: Consider whether value trimming is always appropriate.Line 47 trims the variable value, which might not be desired in all cases. For example, if a user intentionally wants to set a value with leading or trailing whitespace, this will remove it.
While trimming is generally helpful for user input, consider whether the value should be preserved as-is:
var variableName = variable[0].Trim(); -var variableValue = variable[1].Trim(); +var variableValue = variable[1];Alternatively, document this behavior in the XML remarks or README to clarify that values are trimmed.
Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs (1)
4-4: Remove extra blank line.CodeFactor flags multiple consecutive blank lines (SA1507). Remove one blank line for consistency with coding standards.
namespace AiCommitMessage.Options; - /// <summary>Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs (1)
113-136: LGTM! Test correctly validates empty variable name handling.The test properly verifies that an empty variable name (format "=value") results in exit code 1 and restores the original exit code in the finally block.
Note: CodeFactor flags extra blank lines at lines 115 and 132, but these are very minor formatting issues that can be addressed if desired:
public void SetEnvironmentVariable_Should_SetExitCode_When_VariableNameIsEmpty() { - var options = new SetEnvironmentVariableOptions} finally { - Environment.ExitCode = originalExitCode;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(2 hunks)Src/AiCommitMessage/AiCommitMessage.csproj(1 hunks)Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs(1 hunks)Src/AiCommitMessage/Program.cs(4 hunks)Src/AiCommitMessage/Services/EnvironmentVariableService.cs(1 hunks)Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:46.785Z
Learning: In the dotnet-aicommitmessage project, the EnvironmentLoader.GetEnvironmentVariable() method only reads environment variables from EnvironmentVariableTarget.User scope first, then EnvironmentVariableTarget.Machine as fallback. It does NOT read from EnvironmentVariableTarget.Process scope, so tests must set environment variables using EnvironmentVariableTarget.User to be properly detected by the application code.
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:02.619Z
Learning: In the dotnet-aicommitmessage project, the EnvironmentLoader class reads environment variables from EnvironmentVariableTarget.User first, then falls back to EnvironmentVariableTarget.Machine. It never reads from EnvironmentVariableTarget.Process scope, so test setups should use EnvironmentVariableTarget.User to match the application's behavior.
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:02.619Z
Learning: In the dotnet-aicommitmessage project, the application code does not read environment variables from the EnvironmentVariableTarget.Process scope, so setting environment variables with this target in tests would be ineffective.
📚 Learning: 2025-07-06T08:14:46.785Z
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:46.785Z
Learning: In the dotnet-aicommitmessage project, the EnvironmentLoader.GetEnvironmentVariable() method only reads environment variables from EnvironmentVariableTarget.User scope first, then EnvironmentVariableTarget.Machine as fallback. It does NOT read from EnvironmentVariableTarget.Process scope, so tests must set environment variables using EnvironmentVariableTarget.User to be properly detected by the application code.
Applied to files:
Src/AiCommitMessage/Services/EnvironmentVariableService.csTests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.csREADME.mdSrc/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs
📚 Learning: 2025-07-06T08:14:02.619Z
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:02.619Z
Learning: In the dotnet-aicommitmessage project, the application code does not read environment variables from the EnvironmentVariableTarget.Process scope, so setting environment variables with this target in tests would be ineffective.
Applied to files:
Src/AiCommitMessage/Services/EnvironmentVariableService.csTests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.csREADME.mdSrc/AiCommitMessage/Options/SetEnvironmentVariableOptions.csSrc/AiCommitMessage/Program.cs
📚 Learning: 2025-07-06T08:14:02.619Z
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:02.619Z
Learning: In the dotnet-aicommitmessage project, the EnvironmentLoader class reads environment variables from EnvironmentVariableTarget.User first, then falls back to EnvironmentVariableTarget.Machine. It never reads from EnvironmentVariableTarget.Process scope, so test setups should use EnvironmentVariableTarget.User to match the application's behavior.
Applied to files:
Src/AiCommitMessage/Services/EnvironmentVariableService.csTests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.csREADME.mdSrc/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs
🧬 Code graph analysis (3)
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (1)
Src/AiCommitMessage/Utility/Output.cs (1)
Output(5-70)
Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs (2)
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (2)
EnvironmentVariableService(9-88)SetEnvironmentVariable(31-87)Src/AiCommitMessage/Utility/EnvironmentLoader.cs (1)
GetEnvironmentVariable(169-188)
Src/AiCommitMessage/Program.cs (1)
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (2)
EnvironmentVariableService(9-88)SetEnvironmentVariable(31-87)
🪛 GitHub Check: CodeFactor
Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs
[notice] 115-115: Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs#L115
An opening brace should not be followed by a blank line. (SA1505)
[notice] 132-132: Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs#L132
An opening brace should not be followed by a blank line. (SA1505)
Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs
[notice] 4-4: Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs#L4
Code should not contain multiple blank lines in a row. (SA1507)
🪛 Gitleaks (8.30.0)
README.md
[high] 141-141: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (3)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Sourcery review
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (12)
README.md (1)
113-162: LGTM! Comprehensive documentation for the new set-env command.The documentation is well-structured with clear syntax, parameter descriptions, target scope explanations, and practical examples covering both User and Machine scopes. The warning about administrator privileges for Machine scope is helpful.
Note: The Gitleaks warning on line 141 is a false positive—it's example documentation showing the command syntax, not an actual API key.
Src/AiCommitMessage/Program.cs (2)
21-36: LGTM! Clean integration of SetEnvironmentVariableOptions into the parser.The integration follows the existing pattern for other options classes. Documentation is updated appropriately, and the new option is correctly added to the ParseArguments generic parameter list.
55-81: LGTM! Proper routing for the set-env command.The new case statement correctly routes SetEnvironmentVariableOptions to EnvironmentVariableService.SetEnvironmentVariable, following the same pattern as other commands. Documentation accurately describes the behavior.
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (3)
15-19: LGTM! Helper method for consistent error handling.The ErrorLine helper correctly outputs errors and sets the exit code to 1, providing consistent error handling throughout the service.
33-52: LGTM! Robust input validation.The validation logic correctly handles:
- Null or empty variables
- Invalid format (missing '=')
- Split with limit 2 to preserve '=' characters in values
- Trimmed names and values
- Empty variable names after trimming
54-67: LGTM! Case-insensitive target validation.The target validation correctly handles case-insensitive matching for "User" and "Machine", with appropriate error messaging for invalid targets.
Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs (1)
1-25: LGTM! Options class correctly configured for CommandLineParser.The class is properly structured with:
- Verb attribute for "set-env"
- Required positional Value parameter for the variable
- Optional Target option with sensible default ("User")
- Clear help text for both properties
Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs (5)
11-31: LGTM! Test correctly validates User target behavior.The test properly sets an environment variable with User target, verifies it was set correctly, and cleans up in the finally block. The use of EnvironmentVariableTarget.User is consistent with the application's environment variable handling.
61-82: LGTM! Test correctly validates invalid format handling.The test properly verifies that an invalid format (missing '=') results in exit code 1 and restores the original exit code in the finally block.
87-108: LGTM! Test correctly validates null variable handling.The test properly verifies that a null variable results in exit code 1 and restores the original exit code in the finally block.
141-162: LGTM! Test correctly validates invalid target handling.The test properly verifies that an invalid target value results in exit code 1 and restores the original exit code in the finally block.
167-191: LGTM! Excellent test coverage for case-insensitive targets.The Theory test with InlineData correctly verifies that target matching is case-insensitive for various input cases ("user", "USER", "User", "uSeR"), and properly cleans up the test variable. This ensures the implementation is user-friendly.
|
@gstraccini csharpier |
|
Running CSharpier on this branch! 🔧 |
|
❌ CSharpier failed! |
|
Hi @guibranco Thank you so much for reviewing and approving my PR! . I've now addressed everything: ✅ Fixed:
Everything should be green now! Thanks again for maintaining this excellent project and for your guidance throughout the review process. I've learned a lot! |
guibranco
removed
the
🚦 awaiting triage
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Src/AiCommitMessage/Program.cs (1)
43-58: Doc comment still says “three” cases but Run now handles four.Update the remarks to say “four specific cases” so the documentation matches the actual switch cases.
- /// This method uses a switch statement to determine the type of action to perform based on the - /// runtime type of the <paramref name="options"/> parameter. It handles three specific cases: + /// This method uses a switch statement to determine the type of action to perform based on the + /// runtime type of the <paramref name="options"/> parameter. It handles four specific cases:
♻️ Duplicate comments (3)
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (2)
57-84: Normalize null/whitespace Target to"User"to match remarks and intended default.Right now, anything other than
"User"/"Machine"(includingnull/whitespace) is treated as invalid, even though the remarks say it should default to User scope when not specified. Normalizing to"User"first makes this method safer for non-CLI callers and aligns it with the documented behavior.- EnvironmentVariableTarget envTarget; - if ( - string.Equals( - setEnvironmentVariableOptions.Target, - "Machine", - StringComparison.OrdinalIgnoreCase - ) - ) + var target = string.IsNullOrWhiteSpace(setEnvironmentVariableOptions.Target) + ? "User" + : setEnvironmentVariableOptions.Target; + + EnvironmentVariableTarget envTarget; + if ( + string.Equals( + target, + "Machine", + StringComparison.OrdinalIgnoreCase + ) + ) { envTarget = EnvironmentVariableTarget.Machine; } else if ( string.Equals( - setEnvironmentVariableOptions.Target, + target, "User", StringComparison.OrdinalIgnoreCase ) ) { envTarget = EnvironmentVariableTarget.User; } else { ErrorLine( $"Invalid target '{setEnvironmentVariableOptions.Target}'. Please use 'User' or 'Machine'." ); return; }
86-92: Avoid logging full environment variable values (possible secret leak).The success message currently prints the entire value, which is risky for secrets (API keys, tokens, passwords, etc.). It’s safer to log only the name and scope, or a masked value.
- Output.InfoLine( - $"Environment variable '{variableName}' set to '{variableValue}' for {envTarget} scope." - ); + Output.InfoLine( + $"Environment variable '{variableName}' set successfully for {envTarget} scope." + );Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs (1)
41-71: “Target not specified” test currently sets Target explicitly, so it doesn’t cover the default behavior.The test name and XML comment say it verifies the default User scope when target is not specified, but it still sets
Target = "User". Either remove the explicit assignment to actually test the default, or rename the test. Removing the assignment is preferable, especially ifSetEnvironmentVariableis updated to treat null/whitespace Target as User as suggested earlier.- var options = new SetEnvironmentVariableOptions - { - Variable = "TEST_DEFAULT_VAR=default_value", - Target = "User", - }; + var options = new SetEnvironmentVariableOptions + { + Variable = "TEST_DEFAULT_VAR=default_value", + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs(1 hunks)Src/AiCommitMessage/Program.cs(4 hunks)Src/AiCommitMessage/Services/Cache/CommitMessageCacheService.cs(2 hunks)Src/AiCommitMessage/Services/EnvironmentVariableService.cs(1 hunks)Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:02.619Z
Learning: In the dotnet-aicommitmessage project, the EnvironmentLoader class reads environment variables from EnvironmentVariableTarget.User first, then falls back to EnvironmentVariableTarget.Machine. It never reads from EnvironmentVariableTarget.Process scope, so test setups should use EnvironmentVariableTarget.User to match the application's behavior.
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:46.785Z
Learning: In the dotnet-aicommitmessage project, the EnvironmentLoader.GetEnvironmentVariable() method only reads environment variables from EnvironmentVariableTarget.User scope first, then EnvironmentVariableTarget.Machine as fallback. It does NOT read from EnvironmentVariableTarget.Process scope, so tests must set environment variables using EnvironmentVariableTarget.User to be properly detected by the application code.
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:02.619Z
Learning: In the dotnet-aicommitmessage project, the application code does not read environment variables from the EnvironmentVariableTarget.Process scope, so setting environment variables with this target in tests would be ineffective.
📚 Learning: 2025-07-06T08:14:02.619Z
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:02.619Z
Learning: In the dotnet-aicommitmessage project, the application code does not read environment variables from the EnvironmentVariableTarget.Process scope, so setting environment variables with this target in tests would be ineffective.
Applied to files:
Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.csSrc/AiCommitMessage/Program.csSrc/AiCommitMessage/Services/EnvironmentVariableService.csTests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs
📚 Learning: 2025-07-06T08:14:46.785Z
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:46.785Z
Learning: In the dotnet-aicommitmessage project, the EnvironmentLoader.GetEnvironmentVariable() method only reads environment variables from EnvironmentVariableTarget.User scope first, then EnvironmentVariableTarget.Machine as fallback. It does NOT read from EnvironmentVariableTarget.Process scope, so tests must set environment variables using EnvironmentVariableTarget.User to be properly detected by the application code.
Applied to files:
Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.csSrc/AiCommitMessage/Program.csSrc/AiCommitMessage/Services/EnvironmentVariableService.csTests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs
📚 Learning: 2025-07-06T08:14:02.619Z
Learnt from: krisbiradar
Repo: guibranco/dotnet-aicommitmessage PR: 251
File: Tests/AiCommitMessage.Tests/Services/GenerateCommitMessageServiceTests.cs:16-16
Timestamp: 2025-07-06T08:14:02.619Z
Learning: In the dotnet-aicommitmessage project, the EnvironmentLoader class reads environment variables from EnvironmentVariableTarget.User first, then falls back to EnvironmentVariableTarget.Machine. It never reads from EnvironmentVariableTarget.Process scope, so test setups should use EnvironmentVariableTarget.User to match the application's behavior.
Applied to files:
Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.csSrc/AiCommitMessage/Program.csSrc/AiCommitMessage/Services/EnvironmentVariableService.csTests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs
🧬 Code graph analysis (3)
Src/AiCommitMessage/Program.cs (1)
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (2)
EnvironmentVariableService(9-109)SetEnvironmentVariable(32-108)
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (1)
Src/AiCommitMessage/Utility/Output.cs (5)
ErrorLine(24-24)ErrorLine(36-37)Output(5-70)InfoLine(7-7)InfoLine(21-22)
Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs (2)
Src/AiCommitMessage/Services/EnvironmentVariableService.cs (2)
EnvironmentVariableService(9-109)SetEnvironmentVariable(32-108)Src/AiCommitMessage/Utility/EnvironmentLoader.cs (1)
GetEnvironmentVariable(169-188)
⏰ 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). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (3)
Src/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs (1)
5-29: Options shape forset-envlooks solid.Verb name, required
VAR_NAME=valueargument, and optional--targetdefaulting to"User"are consistent with the rest of the CLI and the PR requirements.Src/AiCommitMessage/Program.cs (1)
65-81:set-envdispatch wiring looks correct.Routing
SetEnvironmentVariableOptionstoEnvironmentVariableService.SetEnvironmentVariablefits the existing pattern for other verbs and keeps Program.cs nicely centralized.Tests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs (1)
7-203: Good coverage of success and error paths.These tests exercise happy paths, invalid formats, null/empty variables, invalid targets, and case-insensitive target handling, and they clean up both environment variables and
Environment.ExitCodeproperly.
|
Hi! 👋 Hope you're doing well! Just checking in on the PR status. Current situation:
Question: Happy to help troubleshoot if needed! 😊 Thanks for your time! |
|
Hi! I just wanted to check in and see if there’s anything blocking this on my end, or if you’re waiting for anything from me. Please let me know if any changes are needed. I noticed the workflows are still pending approval, and I’m happy to help troubleshoot if required. Thanks for your time! |
2 participants
User description
📋 Description
Implements the feature requested in issue #38 to add a
set-envcommand for setting environment variables from the CLI. This enhancement allows users to easily configure environment variables for User or Machine scope directly from the command line, which is particularly useful for development setup and CI/CD pipelines.✨ Changes Made
1. New Options Class:
SetEnvironmentVariableOptions.csSrc/AiCommitMessage/Options/[Verb("set-env")]VARIABLE_NAME=valueformat--target/-toption for User/Machine scope2. New Service Class:
EnvironmentVariableService.csSrc/AiCommitMessage/Services/VARIABLE_NAME=valueformat usingSplit('=', 2)to support=in valuesSecurityExceptionfor Machine scope without admin privilegesOutput.ErrorLine()3. Updated:
Program.csSetEnvironmentVariableOptionstoParseArguments<>generic typesRun()method to callEnvironmentVariableService.SetEnvironmentVariable()4. Unit Tests:
EnvironmentVariableServiceTests.csTests/AiCommitMessage.Tests/Services/5. Unit Tests:
SetEnvironmentVariableOptionsTests.csTests/AiCommitMessage.Tests/Options/6. Documentation: Updated
README.mdset-envto Commands table🧪 Testing
Manual Testing Results
All tests performed using:
✅ Success Cases:
✅ Error Handling:
🔗 Closes
Closes #38
🙏 Thank You
Thank you for reviewing this contribution! I'm happy to address any feedback or make requested changes.
Description
set-envcommand for managing environment variables from the CLI.README.mdfor usage and examples.SetEnvironmentVariableOptionsclass for command-line options.EnvironmentVariableServiceto handle setting environment variables with error handling.Changes walkthrough 📝
README.md
Documentation for `set-env` commandREADME.md
set-envcommand.SetEnvironmentVariableOptions.cs
New options class for setting environment variablesSrc/AiCommitMessage/Options/SetEnvironmentVariableOptions.cs
SetEnvironmentVariableOptionsclass.EnvironmentVariableService.cs
Service for managing environment variablesSrc/AiCommitMessage/Services/EnvironmentVariableService.cs
SetEnvironmentVariablemethod.Program.cs
Updated program to include new command handlingSrc/AiCommitMessage/Program.cs
SetEnvironmentVariableOptionsin command-line parser.Runmethod to handle new command.EnvironmentVariableServiceTests.cs
Unit tests for environment variable serviceTests/AiCommitMessage.Tests/Services/EnvironmentVariableServiceTests.cs
SetEnvironmentVariablemethod.Summary by CodeRabbit
New Features
set-envcommand to configure environment variables with User or Machine scope.Tests
Refactor
Documentation
set-env, including syntax, parameters, scopes, and examples.✏️ Tip: You can customize this high-level summary in your review settings.