Configure Git identity inside container from host git config - #74
Draft
leynos wants to merge 1 commit into
Draft
Conversation
…from host to container Implement Step 4.1: read user.name and user.email from host Git config and apply inside container via git config --global. Missing fields emit warnings, do not fail. Introduce GitIdentityReader trait with SystemGitIdentityReader implementation using git config commands. Apply identity to container using ContainerExecClient exec calls. Add comprehensive unit and behavioral tests covering all cases, including partial and missing identity and container exec failures. Update design and user docs to reflect Git identity configuration behaviour. Mark Step 4.1 as done in roadmap. This feature ensures commits inside containers carry correct author info derived from host configuration, enabling smoother agent operation without requiring manual identity configuration or causing failures when identity is missing. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates Failed
(3 new files with code health below 10.00)
(1 file with Deep, Nested Complexity)
(3 files with Excess Number of Function Arguments, Code Duplication, String Heavy Function Arguments, Large Assertion Blocks, Duplicated Assertion Blocks)
Gates Passed
See analysis details in CodeScene
Reason for failure
| New code is healthy | Violations | Code Health Impact | |
|---|---|---|---|
| tests.rs | 4 rules | 6.69 | Suppress |
| mod.rs | 2 rules | 9.10 | Suppress |
| assertions.rs | 1 rule | 9.39 | Suppress |
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| mod.rs | 1 critical rule | 9.10 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| tests.rs | 4 advisory rules | 6.69 | Suppress |
| mod.rs | 1 advisory rule | 9.10 | Suppress |
| assertions.rs | 1 advisory rule | 9.39 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Comment on lines
+385
to
+414
| async fn wait_for_exec_completion<C: ContainerExecClient>( | ||
| client: &C, | ||
| exec_id: &str, | ||
| ) -> Result<(), String> { | ||
| loop { | ||
| let inspect = client | ||
| .inspect_exec(exec_id) | ||
| .await | ||
| .map_err(|e| format!("{e}"))?; | ||
|
|
||
| if let Some(running) = inspect.running { | ||
| if !running { | ||
| // Check exit code for non-zero (Git config failure). | ||
| if let Some(code) = inspect.exit_code { | ||
| if code != 0 { | ||
| return Err(format!( | ||
| "git config exited with code {code}" | ||
| )); | ||
| } | ||
| } | ||
| return Ok(()); | ||
| } | ||
| } | ||
|
|
||
| tokio::time::sleep(std::time::Duration::from_millis( | ||
| super::exec::EXEC_INSPECT_POLL_INTERVAL_MS, | ||
| )) | ||
| .await; | ||
| } | ||
| } |
There was a problem hiding this comment.
❌ New issue: Deep, Nested Complexity
wait_for_exec_completion has a nested complexity depth of 5, threshold = 4
Comment on lines
+322
to
+382
| async fn apply_single_config<C: ContainerExecClient>( | ||
| client: &C, | ||
| container_id: &str, | ||
| key: &str, | ||
| value: &str, | ||
| warnings: &mut Vec<String>, | ||
| ) -> bool { | ||
| let options = bollard::exec::CreateExecOptions::<String> { | ||
| attach_stdout: Some(false), | ||
| attach_stderr: Some(false), | ||
| attach_stdin: Some(false), | ||
| tty: Some(false), | ||
| cmd: Some(vec![ | ||
| String::from("git"), | ||
| String::from("config"), | ||
| String::from("--global"), | ||
| String::from(key), | ||
| String::from(value), | ||
| ]), | ||
| ..bollard::exec::CreateExecOptions::default() | ||
| }; | ||
|
|
||
| let create_result = client.create_exec(container_id, options).await; | ||
| let exec_id = match create_result { | ||
| Ok(result) => result.id, | ||
| Err(error) => { | ||
| warnings.push(format!( | ||
| "failed to create exec for git config {key}: {error}" | ||
| )); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| let start_result = client | ||
| .start_exec( | ||
| &exec_id, | ||
| Some(bollard::exec::StartExecOptions { | ||
| detach: true, | ||
| tty: false, | ||
| output_capacity: None, | ||
| }), | ||
| ) | ||
| .await; | ||
|
|
||
| if let Err(error) = start_result { | ||
| warnings.push(format!( | ||
| "failed to start exec for git config {key}: {error}" | ||
| )); | ||
| return false; | ||
| } | ||
|
|
||
| // Poll for completion. | ||
| if let Err(error) = wait_for_exec_completion(client, &exec_id).await { | ||
| warnings.push(format!( | ||
| "failed to inspect exec for git config {key}: {error}" | ||
| )); | ||
| return false; | ||
| } | ||
|
|
||
| true | ||
| } |
There was a problem hiding this comment.
❌ New issue: Excess Number of Function Arguments
apply_single_config has 5 arguments, max arguments = 4
Comment on lines
+292
to
+312
| async fn apply_name_only_warns_about_missing_email( | ||
| container_id: &str, | ||
| mock_client: MockExecClient, | ||
| ) { | ||
| let identity = GitIdentity::new(Some(String::from("Alice")), None); | ||
| let result = EngineConnector::apply_git_identity_async( | ||
| &mock_client, | ||
| container_id, | ||
| &identity, | ||
| ) | ||
| .await | ||
| .expect("should succeed"); | ||
|
|
||
| assert!(result.name_applied()); | ||
| assert!(!result.email_applied()); | ||
| assert_eq!(result.warnings().len(), 1); | ||
| assert!(result.warnings()[0].contains("user.email")); | ||
|
|
||
| let calls = mock_client.calls(); | ||
| assert_eq!(calls.len(), 1, "expected one exec call for name only"); | ||
| } |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 11 functions with similar structure: apply_email_only_warns_about_missing_name,apply_empty_identity_warns_and_makes_no_exec_calls,apply_name_only_warns_about_missing_email,configure_reads_identity_and_applies_it and 7 more functions
| @@ -0,0 +1,540 @@ | |||
| //! Unit tests for Git identity reading and container configuration. | |||
There was a problem hiding this comment.
❌ New issue: String Heavy Function Arguments
In this module, 40.0% of all arguments to its 33 functions are strings. The threshold for string arguments is 39.0%
Comment on lines
+191
to
+202
| fn identity_with_both_fields_is_complete() { | ||
| let identity = GitIdentity::new( | ||
| Some(String::from("Alice")), | ||
| Some(String::from("alice@example.com")), | ||
| ); | ||
| assert!(identity.is_complete()); | ||
| assert!(!identity.is_empty()); | ||
| assert!(identity.has_name()); | ||
| assert!(identity.has_email()); | ||
| assert_eq!(identity.name(), Some("Alice")); | ||
| assert_eq!(identity.email(), Some("alice@example.com")); | ||
| } |
There was a problem hiding this comment.
❌ New issue: Large Assertion Blocks
The test suite contains 11 assertion blocks with at least 4 assertions, threshold = 4
Comment on lines
+191
to
+202
| fn identity_with_both_fields_is_complete() { | ||
| let identity = GitIdentity::new( | ||
| Some(String::from("Alice")), | ||
| Some(String::from("alice@example.com")), | ||
| ); | ||
| assert!(identity.is_complete()); | ||
| assert!(!identity.is_empty()); | ||
| assert!(identity.has_name()); | ||
| assert!(identity.has_email()); | ||
| assert_eq!(identity.name(), Some("Alice")); | ||
| assert_eq!(identity.email(), Some("alice@example.com")); | ||
| } |
There was a problem hiding this comment.
❌ New issue: Duplicated Assertion Blocks
The test suite contains 4 functions with duplicated assertion blocks (create_exec_failure_produces_warning_not_error,inspect_exec_failure_produces_warning_not_error,nonzero_exit_code_produces_warning_not_error,start_exec_failure_produces_warning_not_error), threshold = 2
Comment on lines
+34
to
+43
| fn user_name_is_applied( | ||
| git_identity_state: &GitIdentityState, | ||
| ) -> StepResult<()> { | ||
| let outcome = get_outcome(git_identity_state)?; | ||
| if outcome.name_applied() { | ||
| Ok(()) | ||
| } else { | ||
| Err(String::from("expected user.name to be applied")) | ||
| } | ||
| } |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 4 functions with similar structure: user_email_is_applied,user_email_is_not_applied,user_name_is_applied,user_name_is_not_applied
Reviewer's GuideAdds a new Git identity configuration subsystem that reads user.name/user.email from the host via a pluggable reader, applies them inside containers via the existing exec seam with graceful degradation, wires the types through the engine API surface, and covers the behavior with unit, BDD tests, and documentation updates. Sequence diagram for configure_git_identity_async flowsequenceDiagram
actor User
participant HostGitConfig
participant GitIdentityReader
participant EngineConnector
participant ContainerExecClient
participant Container
User->>EngineConnector: configure_git_identity_async(client, container_id, reader)
EngineConnector->>EngineConnector: validate container_id not empty
EngineConnector->>GitIdentityReader: read_git_identity()
GitIdentityReader->>HostGitConfig: git config --get user.name
HostGitConfig-->>GitIdentityReader: optional name
GitIdentityReader->>HostGitConfig: git config --get user.email
HostGitConfig-->>GitIdentityReader: optional email
GitIdentityReader-->>EngineConnector: GitIdentity{name, email}
EngineConnector->>ContainerExecClient: apply_git_identity_async(client, container_id, identity)
alt both fields missing
ContainerExecClient-->>EngineConnector: GitIdentityResult{warnings:["no Git identity configured on the host"], applied:false}
else some fields present
alt user.name present
EngineConnector->>ContainerExecClient: create_exec(container_id, git config --global user.name value)
alt create_exec fails
ContainerExecClient-->>EngineConnector: error
EngineConnector-->>EngineConnector: record warning for user.name
else create_exec succeeds
ContainerExecClient-->>EngineConnector: exec_id_name
EngineConnector->>ContainerExecClient: start_exec(exec_id_name, detach=true)
EngineConnector->>ContainerExecClient: inspect_exec(exec_id_name) until finished
end
else user.name missing
EngineConnector-->>EngineConnector: record warning for missing user.name
end
alt user.email present
EngineConnector->>ContainerExecClient: create_exec(container_id, git config --global user.email value)
alt create_exec fails
ContainerExecClient-->>EngineConnector: error
EngineConnector-->>EngineConnector: record warning for user.email
else create_exec succeeds
ContainerExecClient-->>EngineConnector: exec_id_email
EngineConnector->>ContainerExecClient: start_exec(exec_id_email, detach=true)
EngineConnector->>ContainerExecClient: inspect_exec(exec_id_email) until finished
end
else user.email missing
EngineConnector-->>EngineConnector: record warning for missing user.email
end
end
EngineConnector-->>User: GitIdentityResult{name_applied, email_applied, warnings}
Class diagram for Git identity configuration moduleclassDiagram
class GitIdentity {
-name : Option~String~
-email : Option~String~
+new(name : Option~String~, email : Option~String~) GitIdentity
+name() Option~str~
+email() Option~str~
+has_name() bool
+has_email() bool
+is_empty() bool
+is_complete() bool
}
class GitIdentityReader {
<<interface>>
+read_git_identity() GitIdentity
}
class SystemGitIdentityReader {
+new() SystemGitIdentityReader
+read_git_identity() GitIdentity
}
class GitIdentityResult {
-name_applied : bool
-email_applied : bool
-warnings : Vec~String~
+name_applied() bool
+email_applied() bool
+warnings() Vec~String~
+is_empty() bool
}
class ContainerExecClient {
<<interface>>
+create_exec(container_id : str, options : CreateExecOptions) CreateExecResult
+start_exec(exec_id : str, options : StartExecOptions) StartExecResult
+inspect_exec(exec_id : str) InspectExecResult
}
class EngineConnector {
+configure_git_identity_async(client : ContainerExecClient, container_id : str, reader : GitIdentityReader) GitIdentityResult
+apply_git_identity_async(client : ContainerExecClient, container_id : str, identity : GitIdentity) GitIdentityResult
}
class HelperFunctions {
+read_git_config_value(key : str) Option~String~
+apply_git_identity_async(client : ContainerExecClient, container_id : str, identity : GitIdentity) GitIdentityResult
+apply_single_config(client : ContainerExecClient, container_id : str, key : str, value : str, warnings : Vec~String~) bool
+wait_for_exec_completion(client : ContainerExecClient, exec_id : str) Result~(), String~
}
GitIdentityReader <|.. SystemGitIdentityReader
ContainerExecClient <.. EngineConnector
GitIdentityReader <.. EngineConnector
GitIdentity ..> GitIdentityResult
EngineConnector ..> GitIdentityResult
EngineConnector ..> HelperFunctions
HelperFunctions ..> ContainerExecClient
HelperFunctions ..> GitIdentity
HelperFunctions ..> GitIdentityResult
SystemGitIdentityReader ..> GitIdentity
SystemGitIdentityReader ..> HelperFunctions
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Why
Implementation details
Tests
Validation and acceptance criteria
How to test locally
Notes for reviewers
◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/fb79b680-e6e5-4870-937b-b0e24407e6a1
Summary by Sourcery
Add a Git identity configuration capability that reads host Git settings and applies them inside containers, with graceful degradation and full engine wiring.
Enhancements:
Documentation:
Tests: