Skip to content

fix(mcp): task retention floor, drop malformed responses, clearer capacity error (#132 re-check nits) - #135

Merged
ViperJuice merged 2 commits into
mainfrom
fix/132-recheck-nits
Nov 9, 2026
Merged

ViperJuice merged 2 commits into
mainfrom
fix/132-recheck-nits

Conversation

@ViperJuice

@ViperJuice ViperJuice commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This follow-up fixes the four NITs from the delta re-check of #132. That re-check judged #132 MERGE-READY, with 0 blocking and 0 should-fix findings. #132 is merged as e193baf.

These fixes are meant to land before the 1.6.0 release PR. They change no version and release nothing.

Changed: mcp_server/storage/mcp_task_registry.py, mcp_server/cli/task_runtime.py (five lines for R4), the STDIO pre-check in mcp_server/cli/stdio_runner.py, CHANGELOG.md [Unreleased], tests/test_mcp2_stdio_wire.py, and a one-line test-isolation fix in tests/test_v13_pmcp_pilot.py (see below).

R1–R4 → fixes and tests

Nit Fix Tests (tests/test_mcp2_stdio_wire.py)
R1: a finished result could be lost before a woken tasks/result read it, either via ttl: 0 or via eviction right after the wake with 255 tasks running Task TTLs have a one-minute floor (MIN_TASK_TTL_MS; effective_ttl_ms clamps to [1 min, 1 h]). Expiry and eviction never remove a finished task that still has a tasks/result waiter, or one that finished within the floor. Waiters are counted in wait_for_update, so a waiter woken by the completion but not yet scheduled is covered. Eviction takes the oldest evictable finished task, by finished_at. If every slot holds a running, awaited or just-finished task, the new task is refused rather than a result dropped. test_ttl_zero_task_result_still_reaches_the_waiting_client (ttl: 0 end to end: echoed as 60000, and the parked tasks/result receives the payload) · test_task_ttl_is_clamped_and_defaulted[ttl 0 / ttl 1 → 60000] · test_finished_task_within_the_floor_survives_its_expiry · test_full_registry_keeps_a_just_finished_task_instead_of_evicting_it[with_waiter / without] (255 running plus one just finished; the reviewer's eviction-after-wake ordering: the new task is refused, and the result survives and is read) · test_eviction_takes_older_finished_tasks_before_recent_ones
R2: N3 answered malformed responses, echoing an id from the server's own request-id space _malformed_line_reply returns a drop verdict for a line that fails the SDK's validator and is an object with result or error and no method. _stdio_input logs it at debug and sends no reply. Malformed requests are answered as before (-32700 / -32600). test_malformed_stdio_lines_get_json_rpc_errors_and_valid_lines_pass now feeds {"id":11,"error":{"code":"x"}}, {"id":12,"result":1}, {"id":13,"error":{"message":"no code"}} and {"jsonrpc":"1.0","id":14,"result":{}}. None is answered or passed through; the -32700 and -32600 replies and the valid ping are unchanged.
R3: the CHANGELOG stated the N3 replies unconditionally The [Unreleased] bullet now says "On the POSIX STDIO transport", and adds that Windows uses the SDK's own reader, which still drops malformed lines silently. It also describes the R2 drop. tests/test_release_notes.py and tests/test_release_metadata.py pass, along with the 61 doc-contract files
R4: a capacity refusal reached the client as "details": "ValueError" The registry raises TaskCapacityError (a ValueError subclass). TaskRuntime.run_task re-raises it as MCPError, which call_tool passes through, so the tools/call adapter returns its text as the isError result: "Too many tasks (N running or awaiting retrieval); retry after one finishes". test_full_task_slots_return_the_actionable_message_through_call_tool (through the real call_tool and the handler path with a one-slot registry: the second task call gets the exact message, with no ValueError)

Two existing tests now pass explicit bounds, because the floor changes what they observe. test_task_retention_evicts_finished_tasks_and_refuses_when_all_run uses min_ttl_ms=0, since it tests eviction order rather than the floor, and expects TaskCapacityError. The max_ttl_ms=0 expiry test is unchanged, because the floor is clamped to the maximum.

Verification

dev0. Each affected file ran alone, serially, under nice, with no xdist, after the load and I/O-pressure check (1-minute load 1.0–3.3, I/O pressure ≈0):

File Result
test_mcp2_stdio_wire.py 34 passed
test_mcptasks_reindex.py 8 passed
test_mcptasks_write_summaries.py 2 passed
docs/test_mcptasks_protocol_contract.py 4 passed
test_mcp_server_cli.py 18 passed
test_mcpauth_stdio_contract.py 4 passed
docs/test_mcpstruct_call_tool_contract.py 2 passed
smoke/test_mcpeval_sdk_surface.py 1 passed
smoke/test_mcpbase_stdio_smoke.py 2 passed
test_release_notes.py 4 passed
test_release_metadata.py 27 passed
doc-contract files 61 passed

black, isort and flake8 are clean. The mypy ratchet output is identical to origin/main.

Full suite on ai. It was run rather than skipped because the change touches task_runtime.py (five lines for R4) in addition to the registry, the STDIO pre-check and the CHANGELOG. Each run used the same isolation as #132: systemd-run --user with MemoryMax=16G and CPUQuota=400%, the shared Code-Index-MCP project lock, a fresh clone, an isolated HOME, a private prefetched grammar cache, MCP_REQUIRE_GRAMMARS=1, MCP_SKIP_PLUGIN_PREINDEX=true, uv sync --locked --extra dev --extra semantic (mcp 2.3.0), and pytest tests/ -n 2 --timeout=300.

🤖 Generated with Claude Code

…acity error (#132 re-check nits)

R1: task TTLs have a one-minute floor (MIN_TASK_TTL_MS), so `ttl: 0` can no
longer expire a finished task before a woken tasks/result reads it. Expiry and
eviction never remove a finished task that still has a tasks/result waiter
(counted in wait_for_update, so a woken waiter that has not yet resumed is
covered) or that finished within the floor. Eviction takes the oldest
evictable finished task. With every slot running, awaited or just finished, a
new task is refused instead.

R2: the POSIX STDIO pre-check drops a malformed JSON-RPC response (`result` or
`error` with no `method`) with a debug log. It is not answered, so its id,
which belongs to the server's own request-id space, is never echoed.

R3: the CHANGELOG says the malformed-line replies apply to the POSIX STDIO
transport; Windows uses the SDK's reader, which still drops them silently.

R4: a full registry raises TaskCapacityError, which TaskRuntime.run_task
re-raises as MCPError. The client therefore gets "Too many tasks (N running or
awaiting retrieval); retry after one finishes" instead of
"details": "ValueError".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-09T23:12:04.515305Z 93b7e9f PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

test_fixture_server_and_workers_get_the_prefetched_cache_with_downloads_denied
(added in #132) calls create_fixture, which imports the sibling module
v13_pilot_estimate the way the pilot does when run as a script. It passed only
when an earlier test on the same xdist worker had put scripts/ on sys.path.
The ai full suite on 93b7e9f hit the other ordering (ModuleNotFoundError), and
running the test alone reproduces it. Prepend scripts/ with monkeypatch, as the
file's other create_fixture test already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ViperJuice
ViperJuice merged commit c2a3451 into main Nov 9, 2026
14 checks passed
@ViperJuice ViperJuice mentioned this pull request Nov 10, 2026
ViperJuice added a commit that referenced this pull request Nov 10, 2026
Prepare index-it-mcp 1.6.0: the mcp 2.x port (#124 with #128, #131 and #127
in #132, re-check fixes in #135), so index-it-mcp co-installs with PMCP 3.x,
and the watcher sweep, incremental sync and generation-retention fixes (#114
and #111 in #129, follow-ups in #130).

- Bump to 1.6.0 in pyproject, __version__, the project's uv.lock entry, the
  Release Automation default, both Docker installers, the release docs and the
  release-gate tests.
- Turn [Unreleased] into [1.6.0] - 2026-11-09 (Changed / Fixed / Upgrade notes
  / Known follow-ups). Upgrade notes: mcp>=2.2,<2.4 is required, so drop any
  mcp<2 pin; PMCP 3.x co-install; Claude Code negotiates 2026-07-28 without
  Tasks; the cap widens per re-tested mcp minor; retention is automatic but
  never prunes pre-upgrade generations. Follow-ups #133, #134, #121, #117.
- Docs: 1.6.0 is the prepared candidate, 1.5.1 is published with partial
  delivered acceptance, and 1.3.0 through 1.5.0 are yanked on PyPI.
- The raw JSON-RPC initialize probe now requires serverInfo.version to equal
  the installed index-it-mcp version (#128), so release smoke guards it.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant