Skip to content

fix(mcp): bound version-detection git subprocess - #3038

Open
Zenjibad wants to merge 2 commits into
headroomlabs-ai:mainfrom
Zenjibad:fix/mcp-version-detection-hang
Open

fix(mcp): bound version-detection git subprocess#3038
Zenjibad wants to merge 2 commits into
headroomlabs-ai:mainfrom
Zenjibad:fix/mcp-version-detection-hang

Conversation

@Zenjibad

@Zenjibad Zenjibad commented Aug 14, 2026

Copy link
Copy Markdown

Description

headroom_compress hangs (60s -32001 MCP timeout) in stdio MCP servers. Root cause: the compression pipeline init → OpenTelemetry tracer → get_version()_source_tree_version()list_release_tags() runs git tag -l v* via subprocess.run with no timeout and inherited stdin. In a stdio MCP server, git inherits the stdio pipe and hangs, so await run_in_executor(...) blocks until the client's 60s timeout.

Fix: pass stdin=DEVNULL (stop inheriting the stdio pipe) + timeout=10 (fail gracefully through the existing try/except and continue through the packaged, distribution-metadata, or "unknown" fallback chain) on the three version-detection git calls.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Performance improvement
  • Code refactoring (no functional changes)

Changes Made

  • headroom/release_version.py: add stdin=subprocess.DEVNULL + timeout=10 to list_release_tags, list_release_commits, and commit_height_since git subprocess calls.
  • tests/test_release_version.py: add test_version_detection_git_calls_are_hang_safe asserting the three git calls are bounded and do not inherit the caller's stdin.

Testing

  • Unit tests pass (pytest)
  • Linting passes (ruff check .)
  • Type checking passes (mypy headroom)
  • New tests added for new functionality
  • Manual testing performed

Test Output

$ uv run --no-sync python -m pytest tests/test_release_version.py tests/test_observability_metrics.py -q
collected 29 items
tests\test_release_version.py ................                           [ 55%]
tests\test_observability_metrics.py .............                        [100%]
============================= 29 passed in 6.34s ==============================

$ uv run --no-sync ruff check .
All checks passed!

$ uv run --no-sync ruff format . --check
1415 files already formatted

$ uv run --no-sync mypy headroom/release_version.py --ignore-missing-imports
Success: no issues found in 1 source file

Real Behavior Proof

  • Environment: Windows 10, Python 3.13.13, headroom 0.36.0-dev (editable install), headroom mcp serve (stdio transport), no proxy.
  • Exact command / steps: spawned headroom mcp serve and sent a raw MCP tools/call for headroom_compress with {"content":"{"name":"test","items":[1,2,3,4,5]}"}. Also verified through the dsh web UI at http://127.0.0.1:3080 (same tool, routed through a running proxy).
  • Observed result: before the fix, tools/call hung 70s and returned Error: MCP error -32001: Request timed out. After the fix, tools/call returned in 2.38s:
{"hash":"a76e1fa6b2611dc18b031c6d","original_tokens":24,"compressed_tokens":24,"tokens_saved":0,"transforms":["router:noop"]}

The dsh web UI returned the same hash a76e1fa6b2611dc18b031c6d.

  • Not tested: headroom_retrieve / headroom_stats against a live proxy; Linux/macOS stdio MCP clients; the git-hang path with a running proxy (the proxy is not in the tool-call path — the hang reproduces with no proxy).

Runtime Rollout Safety

  • Rollout-managed feature(s): none.
  • Minimum rollout channel: n/a.
  • Stable/default behavior changed: version detection now fails gracefully through the packaged, distribution-metadata, or "unknown" fallback chain instead of hanging when a git subprocess exceeds 10s. Compression behavior is unchanged.
  • Kill switch / disable path: none — the timeout is a bounded safety net; version detection is already best-effort behind try/except.
  • Unsafe override required: no.
  • Qualification impact: none (version detection is best-effort; failure already continues through that fallback chain).
  • Rollback path: revert this commit.

Review Readiness

  • I have performed a self-review
  • This PR is ready for human review

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I did not edit CHANGELOG.md — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this)

Screenshots (if applicable)

N/A

Additional Notes

  • tests/test_release_workflows.py::test_no_native_tls_in_wheel_build_tree fails locally with FileNotFoundError (missing Windows build tool) — pre-existing and unrelated to this change.
  • The commented-code and documentation checklist items are left unchecked: the change is a small, self-documenting bug fix (stdin=DEVNULL + timeout=10 are self-explanatory), the rationale is captured in the commit message and test docstring, and no API/behavior surface changed to document.
  • Not dsh-specific: this affects headroom mcp serve for any stdio client (claude/codex/etc.).

headroom_compress hangs (60s MCP timeout) because the compression pipeline initializes the OTel tracer, which calls get_version() -> list_release_tags() -> 'git tag -l v*' via subprocess.run with no timeout and inherited stdin. In a stdio MCP server, git inherits the stdio pipe and hangs.

Add stdin=DEVNULL + timeout=10 to the three version-detection git calls (list_release_tags, list_release_commits, commit_height_since).
@github-actions

Copy link
Copy Markdown
Contributor

PR governance

This PR follows the template and is marked ready for human review.

@github-actions github-actions Bot added the status: ready for review Pull request body is complete and the author marked it ready for human review label Aug 14, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the refreshed exact head adversarially. The source-tree version path can execute during telemetry initialization, and all three Git subprocesses are now isolated from MCP stdio and bounded. A timeout from the first lookup is contained by _source_tree_version, allowing get_version() to continue through its existing packaged/distribution fallback chain without affecting compression. I verified that behavior explicitly (the local editable environment returned its installed 0.35.0 metadata rather than hanging), and the focused release/observability suite passes 29/29 with Ruff and diff checks clean. I refreshed the branch to current upstream main and corrected the fallback wording in the description; no merge action was taken. No blocking correctness findings.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready for review Pull request body is complete and the author marked it ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants