Skip to content

Expose mock repo URL rewrites to MCP gateway via shared gitconfig#535

Open
grulja wants to merge 1 commit into
packit:mainfrom
grulja:shared-git
Open

Expose mock repo URL rewrites to MCP gateway via shared gitconfig#535
grulja wants to merge 1 commit into
packit:mainfrom
grulja:shared-git

Conversation

@grulja
Copy link
Copy Markdown
Contributor

@grulja grulja commented May 28, 2026

The MCP gateway runs in a separate container and does not inherit the GIT_CONFIG_COUNT/KEY/VALUE env vars used by local tools. Write the same insteadOf rewrites to $GIT_REPO_BASEPATH/.mock_gitconfig on the shared volume, which the gateway reads via GIT_CONFIG_GLOBAL. This allows clone_repository calls made through the gateway (e.g. in ApplicabilityAgent) to be transparently redirected to local bare clones.

Assisted-by: Claude

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a helper function _write_global_gitconfig to write insteadOf rewrites to a shared global git config file, allowing the MCP gateway container to resolve mock repositories. A critical issue was identified where writing directly to the file overwrites existing configurations, which can cause failures in concurrent environments. It is recommended to use git config --file to surgically add or update the mappings instead.

Comment thread ymir/common/mock_repos.py Outdated
@grulja
Copy link
Copy Markdown
Contributor Author

grulja commented May 28, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a helper function _write_global_gitconfig to write insteadOf git configuration rewrites to a shared .mock_gitconfig file, allowing the MCP gateway container to access them. A review comment suggests cleaning up stale entries pointing to deleted temporary directories in this shared configuration file to prevent it from growing indefinitely and degrading performance.

Comment thread ymir/common/mock_repos.py
TomasTomecek
TomasTomecek previously approved these changes May 28, 2026
Copy link
Copy Markdown
Member

@TomasTomecek TomasTomecek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nicely done!

@grulja
Copy link
Copy Markdown
Contributor Author

grulja commented May 28, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new helper function _write_global_gitconfig to write insteadOf git configuration rewrites to a shared global gitconfig file, enabling the MCP gateway container to access mock repositories. It also includes a cleanup mechanism for stale configuration entries. The review highlights critical issues regarding shared volume concurrency and locking when writing to the gitconfig file, potential accessibility issues if the repository base directory is not under the shared volume path, and the need for defensive directory creation. A robust retry mechanism and validation checks were suggested to address these concerns.

Comment thread ymir/common/mock_repos.py
@grulja grulja marked this pull request as draft May 28, 2026 10:09
@grulja
Copy link
Copy Markdown
Contributor Author

grulja commented May 28, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces changes to handle mock git repositories more robustly during E2E tests. Specifically, it adds support for writing global git configurations to a shared volume file so that the mcp-gateway container can access them, and cleans up stale entries. In test_triage.py, the mock repository directory is cleaned up and recreated if a custom base path is provided. A review comment suggests adding exist_ok=True to the directory creation step to prevent potential FileExistsError failures if directory deletion is slow.

Comment thread ymir/agents/tests/e2e/test_triage.py Outdated
The MCP gateway runs in a separate container and does not inherit the
GIT_CONFIG_COUNT/KEY/VALUE env vars used by local tools. Write the same
insteadOf rewrites to $GIT_REPO_BASEPATH/.mock_gitconfig on the shared
volume, which the gateway reads via GIT_CONFIG_GLOBAL. This allows
clone_repository calls made through the gateway (e.g. in
ApplicabilityAgent) to be transparently redirected to local bare clones.

Assisted-by: Claude
@grulja
Copy link
Copy Markdown
Contributor Author

grulja commented May 28, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces changes to support writing git insteadOf rewrites to a global git configuration file (.mock_gitconfig) when GIT_REPO_BASEPATH is set. This ensures that the rewrites are visible to other containers, such as the mcp-gateway. It also includes logic to clean up stale gitconfig entries pointing to non-existent local paths. The review feedback suggests optimizing the file operations in _write_global_gitconfig by using git.GitConfigParser with read_only=False to perform all updates in-memory, avoiding the overhead of spawning multiple external git subprocesses.

Comment thread ymir/common/mock_repos.py
@grulja grulja marked this pull request as ready for review May 28, 2026 10:56
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