Skip to content

Add MCP payload/response reduction (read + write) and b5/b6 verification fixes - #277

Closed
yakky wants to merge 55 commits into
feature/issue-267-add-mcpfrom
issue/mcp-write-payload-reduction
Closed

yakky wants to merge 55 commits into
feature/issue-267-add-mcpfrom
issue/mcp-write-payload-reduction

Conversation

@yakky

@yakky yakky commented Oct 4, 2026

Copy link
Copy Markdown
Member

Stacks on feature/issue-267-add-mcp (#268). Adds opt-in payload-reduction for the MCP server's read and write tools, plus bugfixes found during two rounds of live verification against taiga.nephila.it.

Read-side (payload/fields/strip_media/expand/resolve_assigned_users/strict_filters on every list_*/get_* tool) and write-side (return_representation on every create_*/update_* tool, plus a new update_work_items batch tool) - both opt-in, byte-identical by default.

Bugfixes: malformed-batch-item isolation, stale-version merge-order leak, resolve_assigned_users field-name bug, several MINIMAL_FIELDS gaps, generic error messages now surface the real message, invited_by now collapses under compact/minimal, expand now also strips media.

Also documents three issues found with no code fix available: a client-side MCP validation quirk, a Taiga-server-side list-filter collapse, and a genuine Taiga API gap in embedded user_stories.

🤖 Generated with Claude Code

yakky and others added 30 commits September 12, 2026 10:09
Neither Milestones.list() nor WikiPages.list() require project at the
client/API level (confirmed live against taiga.nephila.it), unlike
project-scoped tools such as search or the by_ref lookups. Bring
list_milestones and list_wiki_pages in line with the sibling
list_user_stories/list_tasks/list_issues/list_epics pattern, which
already treat project as optional.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…estone

Milestone.user_stories is always fully expanded by python-taiga's parser,
making the embedded field potentially large. Add include_user_stories
(default True, preserving current output) to both tools so callers can
opt into a trimmed response with that key stripped.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tests

Minor code-review follow-ups: replace the Any -> Any signature with the
actual dict[str, Any] | list[dict[str, Any]] shape it's always called
with, and add unit tests exercising the helper directly (dict, list,
and no-op-when-absent cases) rather than only through list_milestones/
get_milestone.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…es toggle

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
…in userstory minimal set

Addresses final-review findings: `payload` was typed `str` instead of the
spec-mandated `Literal["full","compact","minimal"]` on all 19 tool
signatures, allowing an invalid value to silently pass through as
`payload="full"` with no error. `MINIMAL_FIELDS["userstory"]` didn't
include `assigned_users_extra_info`, so `resolve_assigned_users=True`
combined with `payload="minimal"` silently discarded the enrichment the
caller just paid an extra API call for.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
Per user decision after final review: verbose per-tool docstrings for
payload/fields/strip_media/expand/resolve_assigned_users/strict_filters
were repeated near-verbatim across 19 tools, growing the combined
tool-schema text 2.2x (7,844 -> 17,335 chars) - paid by every session
regardless of whether the new parameters are used. Full semantics remain
documented once in docs/mcp.rst; tool docstrings now point there.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
…mpression

The prior docstring-compression commit removed the only place that listed
exactly which fields payload="minimal" keeps for user stories, issues,
epics and milestones, and docs/mcp.rst never had this detail either -
leaving it unrecoverable without reading MINIMAL_FIELDS source. Adds the
four field lists to the existing payload/fields/strip_media/expand tip
block, sourced from serialize.py's MINIMAL_FIELDS.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
doc-sync pass flagged this new private helper as missing a docstring -
its dotted-path grouping and bare-wins-over-dotted collision logic
isn't obvious from the name/type hints alone.
yakky and others added 25 commits September 17, 2026 12:29
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
A task review found that entity_type/ref/fields were extracted from each
item before the try/except block, so a malformed item (missing a
required key) raised KeyError outside the per-item handler - aborting
the whole batch and discarding results already built for earlier
successful items, contradicting the tool's own "not atomic, one row per
item" contract. Moves the extraction inside the try block so a
malformed item now produces its own error row instead of crashing the
batch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
…prove batch error messages

Addresses final-review findings: `_represent`'s "minimal" branch merged
`{**base, **written}`, letting a caller-supplied stale `version` inside
`fields` (used for the write's own optimistic locking) silently
overwrite the correct post-write version - fixed by reversing the merge
order so `base` always wins. `docs/mcp.rst` claimed `version` is "not a
separate parameter anywhere in this server", which is false -
`set_custom_attribute_value` has one - scoped the claim to
`create_*`/`update_*` tools. `update_work_items`'s error rows used bare
`str(exc)`, producing opaque messages like `"'wiki'"` for structural
failures (unsupported entity_type, missing keys) - now includes the
exception type name.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
…display

resolve_assigned_users always returned null names — confirmed against a
live Taiga instance that membership records expose the display name as
full_name, not full_name_display (the field name every other Taiga user
block in this codebase uses, which this code wrongly assumed memberships
shared too). The output key stays full_name_display for consistency with
those other blocks; only the source field read from the membership
record changes. Also corrects an unrelated test's example field name
(list_memberships' generic fields-projection test) to use a real
membership field instead of the same wrong assumption.
…issue

- milestone: add project_extra_info.name/.slug so a cross-project report
  can display/link each board without a second call (M4)
- userstory: add assigned_users so minimal never under-reports secondary
  assignees (M5)
- userstory/issue: drop the redundant per-project status id in favour of
  status_extra_info.name, which is comparable across boards (M8b)
- docs: document verified filter keys and the is_closed/
  status_extra_info.is_closed contradiction caveat (M11)

Ref: artifacts/specs/2026-09-17-taiga-mcp-fix-recommendations.md

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
…set fixes

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
strict_filters only catches this server's own parameter names nested
inside filters by mistake. It gives no protection against a misspelled
or unsupported Taiga filter key (e.g. milestone__in, silently ignored
by Taiga's REST backend) - that class of mistake returns a normal-
looking but wrong result set with no error either way. Document this
prominently so filter-based narrowing is re-asserted client-side
rather than trusted purely because the call didn't raise.

Recommendation 2 (echoing the filters Taiga actually applied) was
considered and dropped: Taiga's list API gives no such signal back to
this client, so the only thing this server could honestly echo is the
query it sent, not what Taiga did with it - materially weaker than
what the finding asked for, so it's left undone rather than shipped
with an implied guarantee it can't deliver.

Ref: artifacts/specs/2026-09-17-taiga-mcp-fix-recommendations.md (M10)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
Anticipated failures - a strict_filters violation naming the offending
keys, a stale/missing version on a single-item update - were reaching
callers as a bare "Error executing tool <name>" with no detail. The mcp
SDK replaces any exception that isn't its own ToolError with a fixed
generic message, treating it as an unanticipated crash; only ToolError
(or a ValueError raised inside a pydantic validator) preserves the real
text.

Add a _patch() helper wrapping every single-item update_*/update_*_by_id
tool's resource.patch() call, re-raising as ToolError with the original
message, and change _check_strict_filters to raise ToolError directly.
update_work_items already avoided this by catching exceptions itself and
returning them as structured rows; single-item tools now surface the
same detail.

Ref: artifacts/specs/verify-writeside-b5.md,
artifacts/specs/taiga-mcp-2.0.0b5-verification.md (N1/N3)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
collapse_extra_info() only matched keys ending in _extra_info, so
invited_by (seen on membership records) - a full nested user block -
escaped the same shrinking every other user/project block gets, even
though it carries the same id + full_name_display shape.

Ref: artifacts/specs/verify-readside-b5.md,
artifacts/specs/taiga-mcp-2.0.0b5-verification.md (M7)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
apply_payload() previously merged expand's raw content after the strip
pass, guaranteeing an expanded block was byte-identical to the raw
resource - including avatar/logo fields, which strip_media otherwise
removes from everywhere else in the response. Measured as a 2.81x size
penalty in practice, with over half of it one duplicated signed logo
URL.

Reorders the merge before the strip pass instead: expand still adds
back every field of the named block that MINIMAL_FIELDS/fields would
otherwise drop, but the block now goes through the same strip_media
semantics as the rest of the response. This reverses a Day 1 ruling
that took the opposite position; usage measurement since then shows
the media leak costs more than the completeness guarantee is worth.

Ref: artifacts/specs/verify-readside-b5.md,
artifacts/specs/taiga-mcp-2.0.0b5-verification.md (M6)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
… issue

Two independent live verification sessions found that some MCP clients
reject a tool call outright when a parameter carrying a schema-level
default is omitted, even though it is absent from the schema's
required array - affecting every such parameter, including ones that
predate this server's payload-reduction work (e.g. list_milestones'
include_user_stories). Confirmed this server's own schema and its own
protocol-level validation are correct (live tools/list shows no
required entries for these parameters, and a raw stdio JSON-RPC call
omitting them succeeds); the rejection traces to the calling client's
own schema-to-validator bridge, not to this server, and there is no
hook in this server's dependencies to change what gets emitted to work
around it.

Ref: artifacts/specs/verify-readside-b5.md,
artifacts/specs/taiga-mcp-2.0.0b5-verification.md (B1/N1),
artifacts/specs/2026-09-17-b5-verification-spike-report.md

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
Confirmed via a raw, unfiltered probe: a milestone-embedded user_story
has no assigned_users key at all - Taiga's embedded serializer omits
it entirely, unlike the top-level list_user_stories/get_user_story
shape. Not something fields or resolve_assigned_users can produce,
since there's no bare id list in the embedded payload to project or
resolve from. The one-call "everything in a sprint" route is real and
valuable, but yields primary assignees only.

Ref: artifacts/specs/taiga-mcp-2.0.0b5-verification.md (N4)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
…the last id (N2)

filters={"milestone": [1444, 1446]} returns a small, clean, plausible-
looking result set - but only for the last id, with every other id's
items silently dropped. Traced to taiga/requestmaker.py: query is
passed straight to requests.get(..., params=query), and requests
already serializes a list value as repeated same-name query params
correctly - this client is not the one collapsing it. The collapse
happens in Taiga's own REST backend (standard Django QueryDict.get()
semantics on repeated params), outside this repo's control - the only
available mitigation is documenting it, alongside the existing
milestone__in caveat.

Ref: artifacts/specs/taiga-mcp-2.0.0b5-verification.md (N2)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBEycLqZR68HpZF5jCdq7Y
@codecov

codecov Bot commented Oct 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.75627% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.55%. Comparing base (07a59ff) to head (2db7322).

Files with missing lines Patch % Lines
taiga/mcp_server/server.py 90.47% 10 Missing and 10 partials ⚠️
taiga/mcp_server/serialize.py 95.58% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@                      Coverage Diff                      @@
##           feature/issue-267-add-mcp     #277      +/-   ##
=============================================================
- Coverage                      97.68%   96.55%   -1.14%     
=============================================================
  Files                             12       12              
  Lines                           1340     1539     +199     
  Branches                          92      146      +54     
=============================================================
+ Hits                            1309     1486     +177     
- Misses                            20       31      +11     
- Partials                          11       22      +11     
Flag Coverage Δ
unittests 96.55% <91.75%> (-1.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 97.986% (-0.5%) from 98.507% — issue/mcp-write-payload-reduction into feature/issue-267-add-mcp

@yakky

yakky commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Superseded by #280 (head branch renamed to satisfy the towncrier branch-name check; also adds coverage tests and the epic-link/version fixes).

@yakky yakky closed this Oct 4, 2026
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.

2 participants