Skip to content

fix: return structured JSON in all list-* tool responses - #96

Open
ecthelion77 wants to merge 1 commit into
yoda-digital:mainfrom
forterro:fix/list-issues-response
Open

fix: return structured JSON in all list-* tool responses#96
ecthelion77 wants to merge 1 commit into
yoda-digital:mainfrom
forterro:fix/list-issues-response

Conversation

@ecthelion77

Copy link
Copy Markdown
Contributor

Problem

All list-* tools (issues, merge requests, notes, discussions, pipelines, jobs, etc.) return their response as two separate MCP content items:

  1. A summary string: "Found N items"
  2. A JSON array of formatted items

MCP clients/gateways that only read the first content item (which is common — e.g. ContextForge, some Copilot integrations) receive zero useful data — just a count string.

Meanwhile, search_repositories already returns a single content item with { count, items } and works correctly everywhere.

Fix

Unify all 20 formatters to return a single structured JSON content item: { count, items: [...] }.

Changes

  • src/formatters.ts: extracted shared jsonResponse() helper, all formatters now return one content item with { count, items } (or a single object for entity responses like formatWikiPageResponse)
  • src/formatters.test.ts: updated all tests for new format, added coverage for formatIssuesResponse, formatEventsResponse, formatWikiPageResponse, formatWikiAttachmentResponse
  • 90 unit tests pass, zero type errors

Before/After

Before (broken for single-item clients):

{
  "content": [
    { "type": "text", "text": "Found 1 issues" },
    { "type": "text", "text": "[{\"id\": 100, ...}]" }
  ]
}

After (consistent with search_repositories):

{
  "content": [
    { "type": "text", "text": "{\"count\": 1, \"items\": [{\"id\": 100, ...}]}" }
  ]
}

Fixes #95

All formatters previously returned two content items — a summary string
('Found N items') and a separate JSON array. MCP clients/gateways that
only read the first content item received zero data.

Change all 20 formatters to return a single content item containing
structured JSON: { count, items: [...] }. This matches the format
already used by search_repositories and is parseable by all clients.

- Extracted shared jsonResponse() helper to eliminate duplication
- Added tests for formatIssuesResponse, formatEventsResponse,
  formatWikiPageResponse, formatWikiAttachmentResponse
- All existing tests updated for new single-item response format
- 90 unit tests pass

Fixes yoda-digital#95

Signed-off-by: Olivier Gintrand <olivier.gintrand@forterro.com>
@ecthelion77
ecthelion77 requested a review from nalyk as a code owner May 25, 2026 08:50

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the GitLab response formatters to provide a consistent, structured JSON output compatible with the Model Context Protocol (MCP). A central jsonResponse helper was introduced to wrap all formatter outputs into a single content item, moving metadata like item counts directly into the JSON payload. The test suite has been updated to reflect these structural changes and includes new test cases for issues, events, and wiki-related formatters. I have no feedback to provide as there were no review comments.

@nalyk

nalyk commented May 27, 2026

Copy link
Copy Markdown
Contributor

@ecthelion77 First, the genuinely good news: the architectural direction you proposed here is correct. I asked four independent reviewers (code-quality, type-design, silent-failure, and a philosophical-council review applying Postel/Hyrum/Torvalds/Carmack lenses) to walk this PR in depth, and they converged: collapsing the list-* response to a single content item with a {count, items} envelope is the right call on standalone merit, not just for ContextForge compatibility. Type-design rating moved 2/5 → 3/5 across all four axes. Tests for ten previously-untested formatters is a real coverage win. The jsonResponse() helper extraction is clean.

I'm landing this via Path B reincarnation in #103 - same pattern we used for #62#80, #63#81, and #86#99. Your four-file commit lands on main with authorship preserved via git cherry-pick -x, and five maintainer commits sit on top to close the rollout gaps:

The architectural completion (4389b53): MCP's CallToolResult schema defines BOTH content: ContentBlock[] AND structuredContent?: { [key: string]: unknown }. Your refactor populated only content[]; this commit adds structuredContent alongside on every response. Backwards-compatible for content[]-iterating clients (Claude Desktop, Continue, Cursor, Cline); forward-compatible for gateways and programmatic consumers reading the typed-data channel directly. This is the protocol-blessed slot for structured tool output - using it means ContextForge and similar gateways don't have to depend on which content[i] holds the data; they read result.structuredContent and get the typed object.

The E2E suite migration (93f210e): the extractJson<Array<T>> pattern in 37 e2e sites would crash at runtime against the new wrapper shape (type-erased cast, then data.some(...) throws TypeError). New extractListItems<T> helper + sed-migration to address it. CI's tool-coverage gate runs on PR but the actual E2E suite only runs on push-to-main, so this wasn't visible in your PR's green check.

The scope completion (92d2bc7): the 0.9.0 tool list_pipeline_jobs + include_log_tail still used {jobs, log_fetch_errors?} with a separate header text item. Renamed jobsitems and folded it into the same single-item envelope. Now the entire list-* surface is on one shape.

The contract documentation (373b3ec): README "Response shape contract" section + CHANGELOG [Unreleased] BREAKING block + migration recipes. This is the load-bearing piece for downstream consumers.

The structured-coverage extension (a8cbe47): pre-push silent-failure audit on the working tree caught a HIGH-severity finding on my OWN documentation - the README contract claim "every tool populates structuredContent" was true for the 22 formatter-routed tools + the inline include_log_tail case (23/76), but false for the other 53 tool handlers that returned content-only via inline return { content: [...] } patterns. So this commit exports jsonResponse from formatters.ts, tightens its generic to <T extends Record<string, unknown>> (blocks arrays at compile time), and sed-migrates all 48 inline JSON returns. Now every tool that returns JSON data populates both surfaces. The 6 remaining inline returns are text-only status messages ("Branch X has been deleted") that legitimately have no JSON payload.

Why Path B not Path A: the architectural completion was the load-bearing decision (whether to use structuredContent at all is a design call that fits the maintainer role; routing 48 inline sites through a shared helper is mechanical follow-through). Path A would have been six review rounds with you on items that are mostly maintainer judgement calls. Path B preserves your authorship credit via cherry-pick, lands the architectural fix in one cycle, and respects your time. Same dignity, same name on the commits.

Release plan: cut 0.10.0 immediately after #103 merges (semver-minor signals the breaking shape change under 0.x).

Closing #96 with this comment as it merges into #103. Thanks for the catch on the gateway-compatibility class of bug - your issue #95 reproduced a real problem that needed the protocol-correct fix to land properly. The shape decision lives in main with your authorship from here.

If you want to keep working on the open chart PR #97 or have other ideas, I'm here.

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.

bug: list-issues tool returns only count summary, not actual issue data

2 participants