Repository navigation
fix(idalib): fall back to defaults for blank IDA_MCP_* values - #533
Merged
mrexodia merged 1 commit intoSep 21, 2026
Merged
Conversation
WORKER_OPEN_TIMEOUT_SEC and the --max-workers default converted the raw environment string themselves, so a documented variable that is present but empty (or non-numeric) raised ValueError while building the module / parser and the supervisor would not start at all - not even for --help. Read both through _env_float and a new _env_int mirror of it, the tolerant pattern every other knob in this file already uses; 0 keeps its documented meaning for both.
Lesereingrape
had a problem deploying
to
external-pr
September 21, 2026 15:52 — with
GitHub Actions
Failure
This branch had an error being deployed
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
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.
Closes #532
Problem
Two documented
IDA_MCP_*knobs insrc/ida_pro_mcp/idalib_supervisor.pyconverted theraw environment value themselves, so a variable that is present but empty (or
non-numeric) raised
ValueErrorand the supervisor did not start:WORKER_OPEN_TIMEOUT_SEC = float(os.environ.get("IDA_MCP_OPEN_TIMEOUT", "1800"))at:88, which runs at import time;default=int(os.environ.get("IDA_MCP_MAX_WORKERS", "4"))at:1485, which runs whilethe parser is built, so even
--helpaborted.Every other knob in the file is read through
_env_float(:58-62), which strips thevalue, treats empty as unset, and returns the default on
ValueError. That asymmetry isthe defect: the same operator mistake is absorbed for
IDA_MCP_WEDGED_GRACE_SEC/IDA_MCP_WORKER_CALL_TIMEOUT/IDA_MCP_HEALTH_*and isfatal for these two.
Change
IDA_MCP_OPEN_TIMEOUTnow goes through_env_float, andIDA_MCP_MAX_WORKERSthrough anew
_env_intplaced next to it - the same body withint()in place offloat(),because the file currently expresses its one integer knob as
int(_env_float("IDA_MCP_HEALTH_RETRIES", 3))and that shape is kept as is rather thanrewritten here.
0keeps its documented meaning on both paths:os.environ.get(name, "").strip() or defaulttreats the string"0"as a value, soIDA_MCP_OPEN_TIMEOUT=0still yields
0.0("wait indefinitely") andIDA_MCP_MAX_WORKERS=0still yields0("unlimited", the
max_workers <= 0branch at:424).Measurements (Windows 11, Python 3.12, base
2c0424e)Before (
PYTHONPATH=src, repo venv):After, same commands and values:
Suite A/B on the same interpreter, base worktree vs this branch:
The one failure in both runs is the pre-existing
tests/test_mcp_spec_http_e2e.py::Http11TransportTests::test_single_threaded_stop_closes_idle_keepalive_connectionthat #527 is about; this change adds no tests, so the counts are identical by design.
ruff check src/ida_pro_mcp/idalib_supervisor.py(0.14.0, repo config) reportsAll checks passed!on both base and this branch.ruff format --checkreports the fileas "would reformat" on base as well (pre-existing drift around
:33and the single blankline above
def _env_float), so I kept the file's local spacing instead of pullingunrelated reformatting into this diff.
No test file is added: #529 and #531 were both merged after the tests were dropped at
review, so this is src-only unless you would like the two CLI cases pinned in
tests/test_idalib_supervisor.py- the reload needed to re-evaluate:88isstraightforward and I can add it in a follow-up commit on request.
Relation to other open PRs
#501 (
feat(idalib): configure startup worker idle timeout) adds arguments directly afterthe
--max-workersentry at:1485, so if it lands first this one-line default needs thetrivial rebase; it does not change either read, and
IDA_MCP_OPEN_TIMEOUTis untouchedthere (
grepof its diff: 0 mentions).Authorship disclosure
This PR was authored by an AI agent (no human reviewed the diff before submission). All
measurements above were executed by the agent on the commits in this branch, on the base
commit
2c0424e, with the commands shown.Which shipped configuration forwards them
.codex-plugin/mcp.jsondeclaresIDA_MCP_MAX_WORKERSandIDA_MCP_OPEN_TIMEOUTin theenv_varslist of theidalibserver entry (that list was added by #516), so these aretwo of the knobs the shipped plugin passes through from the host environment. Every other
numeric knob in that list is read through
_env_float(:66-79) or parses its valueinside a
try/except(ida_mcp/sync.py:66-73); the string knobs are not converted atall. These two were the only forwarded variables that could abort startup when present but
empty. (Measured by setting the variables directly; the plugin expansion path itself was
not run.)