fix(mcp): project reads list milestones without their plans, and cap done ones (#4045)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 1m0s
CI & Build / Python tests (push) Successful in 1m32s
CI & Build / Build & push image (push) Successful in 30s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 1m0s
CI & Build / Python tests (push) Successful in 1m32s
CI & Build / Build & push image (push) Successful in 30s
enter_project returned every milestone's full plan body. On a project with 39 milestones the handshake came to ~222k characters, 168k of them bodies (110k from done milestones). That is past what an MCP client accepts as a tool result, so the call meant to orient a session arrived as a file to page through. It grows with a project's history, so any long-lived project on any install gets there. - brief_milestone_summary (services/milestones.py) trims summary rows to the listing fields: id, title, description, status, order_index and progress. The plan is get_milestone's job. user_id, project_id and timestamps repeat what the caller knows. - enter_project and get_project share one block: every open milestone plus the 5 most recently updated done ones, in order. milestone_summary_omitted is attached only when older done ones were left out, and names list_milestones and get_milestone. - list_milestones lists every milestone, done included, without bodies. It is the call the omitted line points to, and it had the same size problem. - The REST project summary is unchanged; the web UI reads it. Tests: trimming, the done cap and its order, the omitted key present and absent, get_project and list_milestones, and a size ceiling on enter_project's milestone block for a 200-milestone history. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -0,0 +1,141 @@
|
||||
"""Milestone listings stay brief and bounded (#4045).
|
||||
|
||||
enter_project once carried every milestone's full plan body. On a project with
|
||||
39 milestones that came to ~222k characters, past what an MCP client accepts
|
||||
as a tool result, so the session handshake arrived as a file to page through.
|
||||
"""
|
||||
import contextlib
|
||||
import json
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.mcp.tools.milestones import list_milestones
|
||||
from scribe.mcp.tools.projects import enter_project, get_project
|
||||
from scribe.services.milestones import brief_milestone_summary
|
||||
from tests.helpers import fake_project
|
||||
|
||||
|
||||
pytestmark = pytest.mark.usefixtures("_bind_user")
|
||||
|
||||
PLAN = "A plan paragraph long enough to matter. " * 125 # ~5k chars
|
||||
|
||||
|
||||
def _row(mid: int, status: str, updated: str) -> dict:
|
||||
"""A summary row as get_project_milestone_summary returns it."""
|
||||
return {
|
||||
"id": mid, "user_id": 7, "project_id": 5, "title": f"M{mid}",
|
||||
"description": f"what M{mid} is for", "body": PLAN, "status": status,
|
||||
"order_index": mid, "created_at": "2026-01-01T00:00:00+00:00",
|
||||
"updated_at": updated, "total": 4, "completed": 2, "pct": 50.0,
|
||||
"status_counts": {"todo": 2, "in_progress": 0, "done": 2, "cancelled": 0},
|
||||
}
|
||||
|
||||
|
||||
def _history(done: int, active: int) -> list[dict]:
|
||||
"""`done` done milestones (higher id = more recently updated), then `active` open ones."""
|
||||
rows = [_row(i, "done", f"2026-08-{i + 1:02d}T00:00:00+00:00") for i in range(done)]
|
||||
rows += [_row(done + i, "active", "2026-01-01T00:00:00+00:00") for i in range(active)]
|
||||
return rows
|
||||
|
||||
|
||||
def test_brief_rows_leave_out_the_plan_and_what_the_caller_already_knows():
|
||||
brief, omitted = brief_milestone_summary([_row(1, "active", "2026-09-01")])
|
||||
assert omitted == 0
|
||||
assert brief == [{
|
||||
"id": 1, "title": "M1", "description": "what M1 is for", "status": "active",
|
||||
"order_index": 1, "total": 4, "completed": 2, "pct": 50.0,
|
||||
"status_counts": {"todo": 2, "in_progress": 0, "done": 2, "cancelled": 0},
|
||||
}]
|
||||
|
||||
|
||||
def test_cap_keeps_every_open_milestone_and_the_most_recent_done_ones_in_order():
|
||||
rows = _history(done=8, active=3)
|
||||
brief, omitted = brief_milestone_summary(rows, done_kept=2)
|
||||
assert omitted == 6
|
||||
# The two most recently updated done ones (ids 6, 7) and all open ones,
|
||||
# still in order_index order.
|
||||
assert [r["id"] for r in brief] == [6, 7, 8, 9, 10]
|
||||
|
||||
|
||||
def test_no_cap_keeps_every_row():
|
||||
brief, omitted = brief_milestone_summary(_history(done=8, active=3))
|
||||
assert omitted == 0
|
||||
assert len(brief) == 11
|
||||
|
||||
|
||||
def _enter_stubs(rows: list[dict]):
|
||||
return [
|
||||
patch("scribe.mcp.tools.projects.projects_svc.get_project",
|
||||
AsyncMock(return_value=fake_project(id=5))),
|
||||
patch("scribe.mcp.tools.projects.rulebooks_svc.get_applicable_rules",
|
||||
AsyncMock(return_value={"rules": [], "truncated": False,
|
||||
"subscribed_rulebooks": []})),
|
||||
patch("scribe.mcp.tools.projects.milestones_svc.get_project_milestone_summary",
|
||||
AsyncMock(return_value=rows)),
|
||||
patch("scribe.mcp.tools.projects.notes_svc.list_notes",
|
||||
AsyncMock(side_effect=[([], 0), ([], 0)])),
|
||||
patch("scribe.mcp.tools.projects.systems_svc.list_systems",
|
||||
AsyncMock(return_value=[])),
|
||||
patch("scribe.mcp.tools.projects.systems_tools.bootstrap_systems_ask",
|
||||
AsyncMock(return_value=None)),
|
||||
patch("scribe.mcp.tools.projects.coverage_svc.cached_coverage",
|
||||
AsyncMock(return_value=None)),
|
||||
patch("scribe.mcp.tools.projects.spawn"),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_enter_project_stays_small_however_long_the_history():
|
||||
"""The ceiling is the point: a long-lived project's handshake must not
|
||||
grow with its history. 200 milestones with 5k-character plans would be
|
||||
~1M characters if bodies rode along."""
|
||||
with contextlib.ExitStack() as stack:
|
||||
for cm in _enter_stubs(_history(done=190, active=10)):
|
||||
stack.enter_context(cm)
|
||||
out = await enter_project(project_id=5)
|
||||
|
||||
summary = out["milestone_summary"]
|
||||
assert all("body" not in m for m in summary)
|
||||
assert sum(m["status"] == "done" for m in summary) == 5
|
||||
assert sum(m["status"] == "active" for m in summary) == 10
|
||||
assert out["milestone_summary_omitted"].startswith("185 older done milestone(s)")
|
||||
assert "list_milestones(5)" in out["milestone_summary_omitted"]
|
||||
assert len(json.dumps(out["milestone_summary"], indent=2)) < 10_000
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_omitted_key_is_absent_when_nothing_was_left_out():
|
||||
"""Attached only when it applies (#2483)."""
|
||||
with contextlib.ExitStack() as stack:
|
||||
for cm in _enter_stubs(_history(done=3, active=2)):
|
||||
stack.enter_context(cm)
|
||||
out = await enter_project(project_id=5)
|
||||
assert len(out["milestone_summary"]) == 5
|
||||
assert "milestone_summary_omitted" not in out
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_project_uses_the_same_brief_block():
|
||||
with patch("scribe.mcp.tools.projects.projects_svc.get_project",
|
||||
AsyncMock(return_value=fake_project(id=5))), \
|
||||
patch("scribe.mcp.tools.projects.milestones_svc.get_project_milestone_summary",
|
||||
AsyncMock(return_value=_history(done=9, active=1))), \
|
||||
patch("scribe.mcp.tools.projects.rulebooks_svc.get_applicable_rules",
|
||||
AsyncMock(return_value={"rules": [], "truncated": False,
|
||||
"subscribed_rulebooks": []})):
|
||||
out = await get_project(project_id=5)
|
||||
assert len(out["milestone_summary"]) == 6
|
||||
assert all("body" not in m for m in out["milestone_summary"])
|
||||
assert out["milestone_summary_omitted"].startswith("4 older done milestone(s)")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_milestones_lists_every_milestone_without_plans():
|
||||
"""The call milestone_summary_omitted points to: every milestone, done
|
||||
ones included, and no bodies (get_milestone has the plan)."""
|
||||
with patch("scribe.mcp.tools.milestones.milestones_svc.get_project_milestone_summary",
|
||||
AsyncMock(return_value=_history(done=30, active=2))):
|
||||
out = await list_milestones(project_id=5)
|
||||
assert len(out["milestones"]) == 32
|
||||
assert all("body" not in m for m in out["milestones"])
|
||||
Reference in New Issue
Block a user