Skip to content

test(mcp): add contract tests for buildApiErrorMessage helper - #196

Merged
niallroche merged 1 commit into
accordproject:mainfrom
JayDS22:jay/mcp-error-handler-tests
Jul 1, 2026
Merged

test(mcp): add contract tests for buildApiErrorMessage helper#196
niallroche merged 1 commit into
accordproject:mainfrom
JayDS22:jay/mcp-error-handler-tests

Conversation

@JayDS22

@JayDS22 JayDS22 commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

The clean test-only follow-up to #155 (which merged Jun 13). Adds eight contract tests for the buildApiErrorMessage helper, plus a one-line export on the helper itself so the tests can import it rather than re-declare it.

This is the PR that was discussed as the replacement for the now-closed #153. The original #153 bundled the helper + error-path rewrites + the test file together; #155 took the helper and the rewrites, this PR takes the tests cleanly on top.

What the tests cover

Two describe blocks, eight cases:

buildApiErrorMessage itself:

  • HTTP 404, 400, 500, 422 status codes surface in the returned message
  • Resource identifiers ('late-delivery', 'agreement-abc-123', 'agr-1', format names like 'pdf') are preserved in the error context
  • Body-read failures (stream already consumed, network error mid-read) degrade gracefully to "No error details available" instead of throwing

Error distinctness (the original motivation):

  • A 404, 400, and 500 produce three distinct messages so MCP clients can tell them apart
  • Every message starts with the context string passed by the caller

Why the export change

The helper was added to mcp.ts as a module-local function in #155. This PR adds export to its declaration (one line) so mcp.test.ts can import it directly rather than re-declaring its logic, which was the coordination concern Niall and I discussed when planning to split #153.

Diff

server/handlers/mcp.ts      |   2 +-      (export keyword)
server/handlers/mcp.test.ts | 118 +++++   (new test file)

Coordination notes

Closes #152 (jointly with merged #155).

@JayDS22
JayDS22 force-pushed the jay/mcp-error-handler-tests branch from 4de793c to 1a8c016 Compare June 14, 2026 15:53
JayDS22 added a commit to JayDS22/apap that referenced this pull request Jun 25, 2026
Status line, workstreams, schedule, decisions, and contributions
sections updated to reflect actuals as of Jun 15, 2026 (W3 day 1):

- Slice 1 of typed errors merged via accordproject#184 (Jun 14)
- accordproject#155 (URI fix) and accordproject#190 (this roadmap) both merged Jun 13
- accordproject#153 closed Jun 13 per the agreed plan, replaced by accordproject#196 (clean
  test-only follow-up)
- Slice 2 opened as draft accordproject#197 (agreements + templatebuilder wired)
- Both alternatives spikes complete with populated NOTES.md
- Decision memo drafted on apap-mcp-poc alternatives/decision-memo
  branch, target send Jun 18-20

Adds three new pending mentor decisions surfaced from the W2
alternatives memo: MCP Tasks framing for agreement triggers,
function-calling adapter codegen target, whether to run a third
spike (CrewAI) before W9.

Both spike branches and the decision memo branch now linked from
the W1-W2 deliverables section so they are discoverable from this
doc.

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
@niallroche
niallroche self-requested a review July 1, 2026 07:18
Adds the first automated tests for the MCP layer's error-handling
helper that landed via accordproject#155. Eight test cases across two describe
blocks:

- HTTP status surfaces (404, 400, 500, 422) appear in the error message
- Resource identifiers (template/agreement IDs, format names) are
  preserved in the error context
- Body-read failures degrade gracefully to a "No error details
  available" message instead of throwing
- A 404, 400, and 500 produce distinct messages so MCP clients can
  tell them apart (the original motivation for the helper)
- Every message starts with the context string passed by the caller

Requires a one-line `export` on buildApiErrorMessage in mcp.ts so the
tests can import it directly rather than re-declaring the helper, which
was the coordination concern flagged on the now-closed accordproject#153.

Full server suite at 61/61 passing (53 prior + 8 new).

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
@JayDS22
JayDS22 force-pushed the jay/mcp-error-handler-tests branch from 1a8c016 to 98132df Compare July 1, 2026 07:24
@niallroche
niallroche merged commit c7eec7f into accordproject:main Jul 1, 2026
15 checks passed
@github-actions

github-actions Bot commented Jul 1, 2026

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 3, 2026
Status line, workstreams, schedule, decisions, and contributions
sections updated to reflect actuals as of Jun 15, 2026 (W3 day 1):

- Slice 1 of typed errors merged via accordproject#184 (Jun 14)
- accordproject#155 (URI fix) and accordproject#190 (this roadmap) both merged Jun 13
- accordproject#153 closed Jun 13 per the agreed plan, replaced by accordproject#196 (clean
  test-only follow-up)
- Slice 2 opened as draft accordproject#197 (agreements + templatebuilder wired)
- Both alternatives spikes complete with populated NOTES.md
- Decision memo drafted on apap-mcp-poc alternatives/decision-memo
  branch, target send Jun 18-20

Adds three new pending mentor decisions surfaced from the W2
alternatives memo: MCP Tasks framing for agreement triggers,
function-calling adapter codegen target, whether to run a third
spike (CrewAI) before W9.

Both spike branches and the decision memo branch now linked from
the W1-W2 deliverables section so they are discoverable from this
doc.

Signed-off-by: Jay Guwalani <guwalanijj@gmail.com>
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>
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.

MCP tool errors are generic and discard HTTP status context from the REST API

2 participants