Skip to content

fix(mcp): summarize_containers counts with one /v2/containers/summary call - #418

Merged
alosec merged 9 commits into
mainfrom
feat/mcp-restore-summary-endpoint
Oct 15, 2026
Merged

alosec merged 9 commits into
mainfrom
feat/mcp-restore-summary-endpoint

Conversation

@alosec

@alosec alosec commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Why

The summarize_containers tool released in MCP 1.1.0/1.1.1 (#411, built from umbrella PR #398) pages the entire filtered container list and counts inside the MCP server, capped at max_rows (3000 by default). On large accounts that means many API calls, counts that hit the cap, and a "total" that can be null.

That isn't how it was meant to ship. #406 moved the tool onto Rails GET /v2/containers/summary (Terminal49/t49#4208, now live in production): one query, the same filters as list_containers, and an exact total. But #406 merged into #405's branch, and the #398 version reached main instead. Nobody clearly owned the final merge, so the difference went unnoticed.

What

The rule is no counting in the MCP. Every dimension is counted by Rails.

  • packages/mcp/src/tools/summarize-containers.ts makes one client.containers.summary(group_by, filters) call. Paging, max_rows and the in-process groupKeys counting are gone.
  • group_by keeps every dimension the released tool had and adds one: current_status, pod_terminal, shipping_line, port_of_discharge, hold_type, pickup_lfd_date, pod_arrival_date.
    • The last three need Terminal49/t49#4294, which adds them to the endpoint. Until that's deployed, Rails returns 400 for them.
  • SDK: containers.summary() reuses the list's filter validation and checks the response shape. A hold_type response may add up to more than total, since a container with two holds counts twice. Every other dimension must add up exactly.
  • OpenAPI: GET /containers/summary lists all seven dimensions, and the SDK types are regenerated from it.
  • Server output schema: { total, group_by, truncated, groups: [{ key, label, count }] }. A null key means the container has no value (no hold, LFD or arrival).

list_parties (#407) and the compact-row fixes on #404/#405 are out of scope here. They need their own pass.

Tests

  • MCP: 395 passed. Lint and tsc --noEmit are clean.
  • SDK: 272 passed, including new hold_type total-contract tests.

Rollout

Leave this as a draft until it's approved and #4294 is deployed. Don't release it before then.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

RetriggerConfidence Score: 4/5

The PR appears safe to merge, with a non-blocking request to preserve the compact-row regression tests.

Fix All in CodexFindings

  1. P2 Compact-row tests are lost ▶
Fix with agent prompt
### Issue 1
packages/mcp/src/tools/summarize-containers.test.ts:5-8
The rewrite deletes the only tests for `compactContainer`, even though `list_containers` still uses it by default. Those tests check active-hold filtering, `availability_known`, and the difference between empty and unreported `holds` and `fees`. Move them into a separate test file rather than dropping them; the new summary tests do not cover that behavior.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

summarize_containers now gets counts from one Rails request instead of paging and counting containers inside MCP.

  • Container summaries now get their counts from one filtered API request.

Acknowledgments: alosec explicitly states that hold_type, pickup_lfd_date, and pod_arrival_date require Terminal49/t49#4294 and that release must wait for its deployment. alosec also defers list_parties and compact-row fixes to separate work. These are not reported as findings.

Diagram

sequenceDiagram
  participant User
  participant MCP
  participant SDK
  participant Rails
  User->>MCP: summarize_containers(group_by, filters)
  MCP->>MCP: Apply list filters and tracking default
  MCP->>SDK: containers.summary(group_by, filters)
  SDK->>Rails: GET /v2/containers/summary
  Rails-->>SDK: Groups, total, truncation flag
  SDK->>SDK: Check response fields and counts
  SDK-->>MCP: Mapped summary
  MCP-->>User: Counts and applied filters
Loading

Reviews (1) · Last reviewed commit: "chore: Auto-generate Postman collection ..." · Reviewed by Greptile

@vercel

vercel Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
api Ready Ready Preview Oct 15, 2026 5:21pm UTC

Request Review

@mintlify

mintlify Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
terminal49 🟢 Ready View Preview Oct 15, 2026, 4:43 PM

Comment thread packages/mcp/src/tools/summarize-containers.test.ts
alosec and others added 6 commits October 15, 2026 11:40
…mary call

The released summarize_containers (1.1.x, from #398) pages the whole filtered
container list and counts in the MCP server, capped at max_rows. Rails already
has GET /v2/containers/summary (Terminal49/t49#4208, live in production), and
reached main.

This restores #406 on top of main and covers every dimension the released
tool offers, so nothing is lost:

- summarize_containers is one SDK call. No paging, no counting, no max_rows.
- group_by: current_status, pod_terminal, shipping_line, port_of_discharge,
  hold_type, pickup_lfd_date, pod_arrival_date. The last three need
  Terminal49/t49 feat/api-container-summary-more-groups.
- SDK: containers.summary(groupBy, filters), with the list's filter
  validation. A hold_type response may add up to more than total, since a
  container with two holds counts twice; every other dimension must add up
  exactly.
- OpenAPI: GET /containers/summary with the seven dimensions; SDK types
  regenerated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
They lived in summarize-containers.test.ts, which this PR rewrites for the
summary endpoint. list_containers still uses compactContainer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alosec

alosec commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (cc26fc5) and pushed with an explicit force-with-lease. Resolved generated-file conflicts by regenerating from OpenAPI and fixed the duplicate paths import introduced by the merge. Compact-row regression coverage is preserved.

Also aligned the tool/OpenAPI descriptions with the companion Rails implementation: display-local calendar days with UTC fallback, and current POD hold names including pending holds. Generated SDK/Postman output refreshed.

Local validation: build and full lint/type checks pass; 403 MCP, 359 SDK, and 31 CLI tests pass (one SDK live test skipped), plus two Postman-generation tests. Hosted CI is running. Still requires t49 #4294 to be deployed before release.

@alosec

alosec commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Final verification at 35e846c: no merge conflicts; 21 hosted checks pass, including workspace checks, SDK/CLI matrices, MCP tests, all six protocol checks, and Vercel. One live-preview smoke check remains red: MCP preview 2026-07-28 cannot discover shipment/container fixtures with the configured CI eval credentials. Reproduced on one retry; the initial preview-URL race is no longer occurring. No tests were skipped or weakened to hide this failure. The failing fixture discovery uses list_shipments/list_containers, not summarize_containers; those handlers and eval code are unchanged by this PR. Investigate the CI eval account/credential and returned tool errors before calling this fully green. No merge performed.

@alosec

alosec commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Pushed aa97804: fixture discovery now includes stopped-tracking history and fails explicitly on HTTP/tool errors rather than misreporting them as an empty account. Added four regression tests; seven local eval-support tests and the full monorepo check pass. No live credentials were used locally, and no CI gate was bypassed.

Latest CI run 38776533674 isolates the remaining failure: list_shipments returns an MCP tool error. Read-only preview runtime inspection confirms SDK AuthenticationError on the upstream call, with caller authorization passed through. The same fixture-discovery failure also exists on main. The CI eval credential needs to be replaced with a valid key for the intended backend/account before the live gate can pass. Other CI checks pass. This PR is not yet fully green.

@alosec
alosec merged commit 6b56eae into main Oct 15, 2026
23 of 24 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 17, 2026
dodeja added a commit that referenced this pull request Oct 17, 2026
#421 edited the title and description in the generated list-filters.mdx
but not in the script that generates it. The next regeneration (#418)
put the old metadata back. This change moves #421's metadata into
generate-filter-types.mjs and regenerates the page.

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

This branch was successfully deployed

2 active (1 outdated) deployments
Preview — aa978044 Deployed Oct 15, 2026 by vercel[bot]
staging - docs — a9820add Deployed Oct 15, 2026 by mintlify[bot]
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