Skip to content

fix(idalib): fall back to defaults for blank IDA_MCP_* values - #533

Merged
mrexodia merged 1 commit into
mrexodia:mainfrom
Lesereingrape:fix/supervisor-env-fallback
Sep 21, 2026
Merged

mrexodia merged 1 commit into
mrexodia:mainfrom
Lesereingrape:fix/supervisor-env-fallback

Conversation

@Lesereingrape

@Lesereingrape Lesereingrape commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Closes #532

Problem

Two documented IDA_MCP_* knobs in src/ida_pro_mcp/idalib_supervisor.py converted the
raw environment value themselves, so a variable that is present but empty (or
non-numeric) raised ValueError and 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 while
    the parser is built, so even --help aborted.

Every other knob in the file is read through _env_float (:58-62), which strips the
value, treats empty as unset, and returns the default on ValueError. That asymmetry is
the defect: the same operator mistake is absorbed for
IDA_MCP_WEDGED_GRACE_SEC / IDA_MCP_WORKER_CALL_TIMEOUT / IDA_MCP_HEALTH_* and is
fatal for these two.

Change

IDA_MCP_OPEN_TIMEOUT now goes through _env_float, and IDA_MCP_MAX_WORKERS through a
new _env_int placed next to it - the same body with int() in place of float(),
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 than
rewritten here. 0 keeps its documented meaning on both paths: os.environ.get(name, "").strip() or default treats the string "0" as a value, so IDA_MCP_OPEN_TIMEOUT=0
still yields 0.0 ("wait indefinitely") and IDA_MCP_MAX_WORKERS=0 still yields 0
("unlimited", the max_workers <= 0 branch at :424).

Measurements (Windows 11, Python 3.12, base 2c0424e)

Before (PYTHONPATH=src, repo venv):

$ IDA_MCP_OPEN_TIMEOUT="" python -c "import ida_pro_mcp.idalib_supervisor"
ValueError: could not convert string to float: ''          (idalib_supervisor.py:88)
$ IDA_MCP_MAX_WORKERS="" python -m ida_pro_mcp.idalib_supervisor --help
ValueError: invalid literal for int() with base 10: ''     (idalib_supervisor.py:1485)
$ IDA_MCP_OPEN_TIMEOUT="abc" python -c "import ida_pro_mcp.idalib_supervisor"
ValueError: could not convert string to float: 'abc'

After, same commands and values:

IDA_MCP_OPEN_TIMEOUT=""   -> imported ok, WORKER_OPEN_TIMEOUT_SEC = 1800.0
IDA_MCP_OPEN_TIMEOUT=abc  -> imported ok, WORKER_OPEN_TIMEOUT_SEC = 1800.0
IDA_MCP_OPEN_TIMEOUT=0    -> WORKER_OPEN_TIMEOUT_SEC = 0.0        (still "wait indefinitely")
IDA_MCP_MAX_WORKERS=""    -> --help prints usage and exits 0
IDA_MCP_MAX_WORKERS=0     -> _env_int(...) == 0                   (still "unlimited")

Suite A/B on the same interpreter, base worktree vs this branch:

python -m pytest tests -q
  base    2c0424e: 1 failed, 248 passed, 114 subtests passed in 20.09s
  this branch    : 1 failed, 248 passed, 114 subtests passed in 19.86s

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_connection
that #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) reports
All checks passed! on both base and this branch. ruff format --check reports the file
as "would reformat" on base as well (pre-existing drift around :33 and the single blank
line above def _env_float), so I kept the file's local spacing instead of pulling
unrelated 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 :88 is
straightforward 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 after
the --max-workers entry at :1485, so if it lands first this one-line default needs the
trivial rebase; it does not change either read, and IDA_MCP_OPEN_TIMEOUT is untouched
there (grep of 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.json declares IDA_MCP_MAX_WORKERS and IDA_MCP_OPEN_TIMEOUT in the
env_vars list of the idalib server entry (that list was added by #516), so these are
two 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 value
inside a try/except (ida_mcp/sync.py:66-73); the string knobs are not converted at
all. 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.)

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.
@mrexodia
mrexodia merged commit a671a5b into mrexodia:main Sep 21, 2026
1 check failed

This branch had an error being deployed

1 failed deployment
external-pr — e67d32cc Deployed Sep 21, 2026 by Lesereingrape via approval #406
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.

[idalib] a blank IDA_MCP_OPEN_TIMEOUT or IDA_MCP_MAX_WORKERS stops the supervisor from starting

2 participants