Skip to content

feat(mcp): add ttlMs + cacheScope to ReadResource contents (SEP-2549) - #201

Merged
niallroche merged 1 commit into
accordproject:mainfrom
JayDS22:jay/feat/mcp-cacheable-results
Jul 10, 2026
Merged

feat(mcp): add ttlMs + cacheScope to ReadResource contents (SEP-2549)#201
niallroche merged 1 commit into
accordproject:mainfrom
JayDS22:jay/feat/mcp-cacheable-results

Conversation

@JayDS22

@JayDS22 JayDS22 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Annotate every ReadResource response with the per-entry caching hints proposed in SEP-2549 ("CacheableResult") so the wire shape matches the MCP 2026-07-28 RC ahead of SDK support:

https://blog.modelcontextprotocol.io/posts/2026-07-28-release-candidate

Per-resource defaults are picked by mutability:

URI ttlMs cacheScope Reason
apap://templates 60_000 private list, per-client view
apap://templates/{id} 300_000 public hash-immutable single row
apap://agreements 30_000 private list, mutates on trigger
apap://agreements/{id} 30_000 private single agreement, mutable
apap://schema/protocol.cto 86_400_000 public bundled .cto, ships with build (entry for when the schema resource lands; cf. #185)

The fields are emitted alongside uri/mimeType/text. The SDK's ReadResourceResult type does not yet declare them, but the SDK's response path is pass-through (no schema-strip), so caching proxies see them today as forward-compatible hints. Once SEP-2549 lands in the SDK the spread becomes a plain field assignment with no behavioural change.

Mirrors the POC change at JayDS22/apap-mcp-poc#4.

Test plan

  • npm test -- --testPathPattern='mcp\\.test' (7/7 pass)
  • npm test full suite (71/71 pass)
  • npx tsc --noEmit -p . clean
  • Manual: hit POST /mcp with resources/read for apap://agreements, confirm ttlMs/cacheScope present in JSON-RPC response

Notes

  • The new server/handlers/mcp.test.ts is the first jest file for mcp.ts; it pins the cache defaults so future edits cannot silently change a per-resource TTL or scope.
  • No DB schema change. No public REST API change. Only the MCP resources/read payload gains two optional fields.

@JayDS22
JayDS22 marked this pull request as draft June 25, 2026 04:55
@JayDS22

JayDS22 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Moving to draft pending audit follow-ups: replacing the as unknown as Array<...> per-callsite cast with one module-augmentation declare module '@modelcontextprotocol/sdk/types.js' { interface TextResourceContents { ttlMs?: number; cacheScope?: 'public' | 'private' } }, dropping the dead CACHE_HINTS.schema key until #199 lands (or rebasing this branch on top of #199), and broadening the integration test coverage past just apap://agreements. Will re-mark ready once those land.

@niallroche

Copy link
Copy Markdown
Contributor

The CACHE_HINTS design and TTL values look well-reasoned, and the pass-through behaviour of the current SDK makes this forward-compatible. Two things needed before this can land:

parse-endpoint.js and parse.js in the repo root look like debugging scripts from exploring the OpenAPI spec. Please remove them from this branch — they shouldn't be committed.
This creates server/handlers/mcp.test.ts, which will conflict with #196, #200, and #199 (all creating the same file). The intended merge order is #196#200#199#201. After #196 merges, @JayDS22 please rebase this branch and append the SEP-2549 test block to the existing file rather than creating a new one.

JayDS22 added a commit to JayDS22/apap that referenced this pull request Jun 25, 2026
…p dead schema key)

Two changes from the code-review audit on PR accordproject#201:

1. SDK type augmentation. Replace the implicit reliance on the SDK's
   loose contents[] typing with an explicit module augmentation in
   server/types/mcp-augmentation.d.ts that declares ttlMs + cacheScope
   as optional fields on TextResourceContents and BlobResourceContents.
   This documents the SEP-2549 (CacheableResult) fields as intentional
   forward-compatible extensions of the wire shape rather than relying
   on the type checker not noticing them. See:
     blog.modelcontextprotocol.io/posts/2026-07-28-release-candidate
   tsconfig.json already picks up the file via include: ["**/*"].

2. Drop the dead schema key from CACHE_HINTS. The
   apap://schema/protocol.cto resource only exists on
   jay/feat/mcp-concerto-context (PR accordproject#199). On main, no handler emits
   that URI, so the CACHE_HINTS.schema entry was dead code. The test
   that pinned it has been retired too. A TODO comment in mcp.ts notes
   that the key should be re-added (ttlMs: 86_400_000, cacheScope:
   'public') in a one-line follow-up once accordproject#199 merges.

No behaviour change for the four resource URIs (apap://templates,
apap://templates/{id}, apap://agreements, apap://agreements/{id});
ttlMs and cacheScope land on every contents[] entry exactly as before.

Tests: 70/70 jest pass (npm test); mcp.test alone 6/6.
Typecheck: npx tsc --noEmit -p . clean.

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
JayDS22 added a commit to JayDS22/apap-mcp-poc that referenced this pull request Jun 25, 2026
Mirrors the upstream change at accordproject/apap#201. Two items from
the code-review audit:

1. SDK type augmentation. Replace four per-callsite
   `as unknown as Array<{ uri; mimeType; text; ttlMs?; cacheScope? }>`
   casts in src/handlers/mcp.ts with a single module augmentation in
   src/types/mcp-augmentation.d.ts that adds optional ttlMs and
   cacheScope fields to TextResourceContents and BlobResourceContents.
   The augmentation documents the SEP-2549 (CacheableResult) fields as
   intentional forward-compatible extensions of the wire shape:
     blog.modelcontextprotocol.io/posts/2026-07-28-release-candidate
   tsconfig.json picks up the file via include: ["src/**/*"].

2. Drop the dead schema key from CACHE_HINTS. The
   apap://schema/protocol.cto resource only exists on
   jay/feat/mcp-concerto-context (upstream PR #199). On main, no
   handler emits that URI, so the CACHE_HINTS.schema entry was dead.
   A TODO comment in mcp.ts notes that the key should be re-added
   (ttlMs: 86_400_000, cacheScope: 'public') in a one-line follow-up
   once #199 merges upstream and the schema resource ports here.

No behaviour change for the four resource URIs already emitted; ttlMs
and cacheScope land on every contents[] entry exactly as before, just
without the cast.

Tests: 54/54 vitest pass (npm test).
Typecheck: tsc --noEmit clean (npm run typecheck).

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
@JayDS22
JayDS22 marked this pull request as ready for review June 25, 2026 14:22
@JayDS22

JayDS22 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed b7e6c8f addressing the two audit items, @niallroche:

  1. Module augmentation in place of per-callsite casts. Added server/types/mcp-augmentation.d.ts that declares ttlMs?: number and cacheScope?: 'public' | 'private' on both TextResourceContents and BlobResourceContents. The spread sites in mcp.ts stay clean and the SEP-2549 fields are documented as intentional forward-compatible extensions rather than relying on the type-checker not noticing.
  2. Dropped the dead schema key from CACHE_HINTS. apap://schema/protocol.cto only exists on jay/feat/mcp-concerto-context (PR feat(mcp): Concerto typed-context hint + protocol.cto schema resource #199), not on main, so the entry was unreachable. Test that pinned it is retired too. A TODO comment in mcp.ts records that the key gets re-added ({ ttlMs: 86_400_000, cacheScope: 'public' }) in a one-line follow-up once feat(mcp): Concerto typed-context hint + protocol.cto schema resource #199 merges and the schema resource lands here.

No wire-format change for the four resource URIs already covered; ttlMs and cacheScope still land on every contents[] entry.

Verification: npm test 70/70 pass, mcp.test alone 6/6, npx tsc --noEmit -p . clean.

Marked ready for review.

@github-actions github-actions Bot added the maintainer-engaged A maintainer has commented or reviewed this item label Jun 26, 2026
JayDS22 added a commit to JayDS22/apap that referenced this pull request Jul 1, 2026
…p dead schema key)

Two changes from the code-review audit on PR accordproject#201:

1. SDK type augmentation. Replace the implicit reliance on the SDK's
   loose contents[] typing with an explicit module augmentation in
   server/types/mcp-augmentation.d.ts that declares ttlMs + cacheScope
   as optional fields on TextResourceContents and BlobResourceContents.
   This documents the SEP-2549 (CacheableResult) fields as intentional
   forward-compatible extensions of the wire shape rather than relying
   on the type checker not noticing them. See:
     blog.modelcontextprotocol.io/posts/2026-07-28-release-candidate
   tsconfig.json already picks up the file via include: ["**/*"].

2. Drop the dead schema key from CACHE_HINTS. The
   apap://schema/protocol.cto resource only exists on
   jay/feat/mcp-concerto-context (PR accordproject#199). On main, no handler emits
   that URI, so the CACHE_HINTS.schema entry was dead code. The test
   that pinned it has been retired too. A TODO comment in mcp.ts notes
   that the key should be re-added (ttlMs: 86_400_000, cacheScope:
   'public') in a one-line follow-up once accordproject#199 merges.

No behaviour change for the four resource URIs (apap://templates,
apap://templates/{id}, apap://agreements, apap://agreements/{id});
ttlMs and cacheScope land on every contents[] entry exactly as before.

Tests: 70/70 jest pass (npm test); mcp.test alone 6/6.
Typecheck: npx tsc --noEmit -p . clean.

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
@JayDS22
JayDS22 force-pushed the jay/feat/mcp-cacheable-results branch from b7e6c8f to 957fe03 Compare July 1, 2026 07:36
@niallroche

Copy link
Copy Markdown
Contributor

Some suggestions:

  1. remove the TODO after [feat(mcp): Concerto typed-context hint + protocol.cto schema resource #199]is merged.

  2. this test doesn't assert anything meaningful:
    it('exports the same CACHE_HINTS reference for both list and item resources', () => {
    expect(CACHE_HINTS.agreementList.ttlMs).toBe(CACHE_HINTS.agreementItem.ttlMs);
    expect(CACHE_HINTS.agreementList).not.toBe(CACHE_HINTS.agreementItem); // trivially true
    });
    The second assertion — that agreementList and agreementItem are different object references — is always true for distinct keys on any object. It's not testing anything. Should either be removed or replaced with something that actually catches a regression (e.g. asserting both have cacheScope: 'private').

  3. there is an edge case for templateItem: { ttlMs: 300_000, cacheScope: 'public' } when the template will serve the old version

JayDS22 added a commit to JayDS22/apap that referenced this pull request Jul 8, 2026
Addresses one of Niall's audit items on accordproject#201. The outer describe
'MCP handler emits cache hints on the wire (smoke)' claimed a
resources/read smoke test that mocks global.fetch, but the actual
test just re-read two CACHE_HINTS values. One of the assertions
(agreementList !== agreementItem by reference) is trivially true
for distinct object keys and catches nothing.

The regression this block could catch is already covered by the
per-key value assertions above (both agreementList and agreementItem
are pinned to ttlMs 30_000 and cacheScope 'private'). Removing the
block rather than replacing it keeps the file honest about what is
actually tested.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
@JayDS22
JayDS22 force-pushed the jay/feat/mcp-cacheable-results branch from 5835a08 to 9d61150 Compare July 9, 2026 03:17
@niallroche
niallroche self-requested a review July 9, 2026 13:39
@JayDS22
JayDS22 force-pushed the jay/feat/mcp-cacheable-results branch from 9d61150 to 0576ab7 Compare July 9, 2026 14:54
…549)

Forward-looking cache hints for `ReadResourceResult.contents[]` mirroring
the SEP-2549 CacheableResult shape in the MCP 2026-07-28 RC. Defaults
chosen by mutability of each resource:

- template lists are volatile and per-client (60s, private)
- single templates are hash-immutable (5min, public)
- agreements are short-lived because the row can be triggered/updated
  (30s, private)
- the bundled Concerto schema is immutable per deploy (24h, public)

Fields are spread alongside uri/mimeType/text so the SEP wire shape lands
unchanged once the SDK accepts them at the top level; the current SDK's
pass-through path lets caching proxies see them today as forward-
compatible hints.

Rebased on top of de87943 (post-accordproject#199 and post-accordproject#202). Previous branch had
a TODO deferring the `schema` key until accordproject#199 landed. accordproject#199 is now merged,
so `CACHE_HINTS.schema` is added with { ttlMs: 86_400_000, cacheScope:
'public' } and spread into the schema resource callback. TODO removed.

Wire-format tests pin per-resource defaults so a value change is a
visible-to-every-client change and CI catches it. Module augmentation in
server/types/mcp-augmentation.d.ts declares the two fields on
TextResourceContents / BlobResourceContents so the spread typechecks
without per-callsite casts.

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>

@niallroche niallroche left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good to go

@niallroche
niallroche merged commit 6c742e6 into accordproject:main Jul 10, 2026
15 checks passed
@github-actions

Copy link
Copy Markdown

🎉 Thank you for your contribution! 🎉

Dear @JayDS22,

Your pull request has been successfully merged into the project! We greatly appreciate your efforts and the time you've dedicated to improving our repository.

What happens next?

  • Your changes will be included in the next release
  • Your name will be added to our contributors list
  • Feel free to take on another issue or suggest new features

Once again, thank you for being part of our community!

Best regards,
The APAP Team

JayDS22 added a commit to JayDS22/apap that referenced this pull request Jul 13, 2026
Brings the roadmap current through the first-half deliverables:

- Status header now reflects W7 start, first-half PRs (accordproject#184, accordproject#196, accordproject#199, accordproject#200,
  accordproject#201, accordproject#202) all merged upstream, midterm dispatch published, Thursday sync
  slot confirmed with Niall.
- Workstream table reflects Proposal Core first-half complete, MCP RC migration
  on track through SEP-2549, Alternatives Evaluation complete with Medium
  publication.
- 12-week schedule marks W4–W6 Done, W7 Active (subscriptions/listen slice
  scoped in apap-mcp-poc#6), W8 covers upstream port + JSON-RPC error mapping.
- Milestones: adds Jul 12 midterm dispatch published and Jul 13 midterm eval
  submitted.
- Open decisions: 2, 3, and 4 all resolved (alternatives shipped, MCP RC
  transport parallel via accordproject#201, Thursday sync slot confirmed).
- Contributions to date: adds accordproject#200, accordproject#201, accordproject#202, accordproject#208, and the peer review
  posted on accordproject#194. accordproject#196 marked merged; accordproject#197 marked superseded.
- Adds Comms deliverables section for the Medium/LinkedIn/Discord surfaces.

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
JayDS22 added a commit to JayDS22/apap that referenced this pull request Jul 14, 2026
Per Niall's Discord feedback on Jul 14 that MCP-native tool retrieval
(ACE-Router paper, ACL 2026) "might be a future work element to explore
if it is worthwhile" — parking the thread in the roadmap doc so it's
discoverable during the W12 handoff writeup rather than lost in Discord
scrollback.

Captures the peer-reviewed prior art (ACE-Router paper + code), the
ecosystem validation (Anthropic Tool Search production numbers, Red Hat
ToolScope, MCP community issue #2808), and how it composes with the
SEP-2549 cache hints landed in accordproject#201.

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
JayDS22 added a commit to JayDS22/apap that referenced this pull request Jul 21, 2026
… review)

Niall flagged that the un-paged `listTemplates(db)` was a regression on
the `apap://templates` MCP resource path: `parseQueryParams` used to clamp
list reads to ≤100 rows, and the service function returns the whole
table on every call. That's a token-budget + scalability regression, and
it means the per-resource `ttlMs` / `cacheScope` hints from accordproject#201 now
annotate an unbounded payload.

Add `{ limit?: number; offset?: number }` opts to `listTemplates`
directly on the primitive, matching the ≤100 cap the REST layer already
applied. Callers with no opts (the current MCP resource path) get the
same effective bound as before. Slice 3 REST unification will pass
`limit` / `offset` through from `parseQueryParams` and inherit the
bounded primitive rather than having to re-add paging in memory.

Also updates the fluent Drizzle mock in templateService.test.ts to
handle both `.limit(N)` and `.limit(N).offset(M)` chains, and adds four
tests pinning the clamp behavior (limit>100 → 100, limit<1 → 1,
offset<0 → 0, no-opts → 100/0).

Validation:
- npm test: 9/9 suites pass, 126 tests total (4 new)
- npx tsc --noEmit clean

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-engaged A maintainer has commented or reviewed this item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants