feat(mcp): standardise all contract tools to integer contract_id#68
Draft
sd-gh-bot wants to merge 1 commit into
Draft
feat(mcp): standardise all contract tools to integer contract_id#68sd-gh-bot wants to merge 1 commit into
sd-gh-bot wants to merge 1 commit into
Conversation
All MCP contract tools that previously required a composite ID (T-123 or
H-456) now accept a plain integer contract_id. The T-/H- prefix is
resolved deterministically inside the tool layer so agents never need to
know about this implementation detail.
Resolution algorithm (src/utils/contract_id.ts – withCompositeId):
1. Call the upstream API with the T-{n} composite ID.
2. If the API returns HTTP 404, retry with H-{n}.
3. Any other error is re-thrown immediately.
This is deterministic: each integer maps to at most one prefix per
workspace. T contracts (created from templates) are tried first because
they are the common case; historic/uploaded contracts (H) only incur one
extra API call.
Changed tools (composite string → integer):
• get_contract_status (composite_id → contract_id: integer)
• get_contract_content (composite_id → contract_id: integer)
• get_contract_download_link (composite_id → contract_id: integer)
• get_contract_key_pointers (contract_id: string → integer)
• get_contract_approvals (contract_id: string → integer)
• create_contract_approvals (contract_id: string → integer)
• send_contract_to_counterparties (contract_id: string → integer)
Unchanged tools (already integer):
• add_contract_comment
• get_contract_activity_log
• upload_contract_version
Other changes:
• get_contract_list description updated to clarify agents strip T-/H-
prefix from the response 'id' field before passing to other tools.
• compact_responses.ts: compactContractKeyPointers handles numeric
contract_id (coerces to string for the contract_id field in response).
• 4 new unit tests for withCompositeId covering T-first success,
H-fallback on 404, non-404 error propagation, and both-miss case.
Co-authored-by: Jaskaran Bhatia <jaskaran@spotdraft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
Closes the inconsistency raised in Slack: some MCP contract tools required a composite ID (e.g.
T-123orH-456) while others required a plain integer. This polluted the agent-facing contract with an internal implementation detail and caused errors when agents mixed up the two formats.After this PR, every contract tool accepts a single
contract_idinteger. The T-/H- prefix resolution is handled entirely inside the tool layer via a new shared utility.What changed
New utility —
src/utils/contract_id.tswithCompositeId(client, contractId, apiFn)wraps any API call that requires a composite ID:apiFn("T-{contractId}")first (template contracts — the common case).apiFn("H-{contractId}")(historic/uploaded contracts).This is deterministic: each integer maps to exactly one prefix per workspace.
Tools updated (composite string → integer)
get_contract_statuscomposite_id: stringcontract_id: integerget_contract_contentcomposite_id: stringcontract_id: integerget_contract_download_linkcomposite_id: stringcontract_id: integerget_contract_key_pointerscontract_id: string(T-/H- format)contract_id: integerget_contract_approvalscontract_id: string(T-/H- format)contract_id: integercreate_contract_approvalscontract_id: string(T-/H- format)contract_id: integersend_contract_to_counterpartiescontract_id: string(T-/H- format)contract_id: integerTools unchanged (already integer)
add_contract_comment,get_contract_activity_log,upload_contract_versionOther changes
get_contract_listdescription updated: agents should strip the T-/H- prefix from the responseidfield before passing to other tools.compact_responses.ts:compactContractKeyPointersnow handles numericcontract_id(coerces to string).AGENTS.mdfiles updated to document the new semantics and the utility.Testing
withCompositeId(test/contract-id-resolution.test.mjs):Live API validation was not run in this sandbox (no credentials). The resolution logic follows the same T-then-H pattern already used in the SpotDraft backend's own fallback code (
get_contract_v3_from_historic_contract_id).Generated description
Below is a concise technical summary of the changes proposed in this PR:
Standardize contract tooling so every endpoint accepts the integer
contract_idreturned byget_contract_listand let the newwithCompositeIdhelper resolve the T- or H- prefixes before calling v2.1 APIs. Document the new semantics in the AGENTS guides and adjust helpers likecompactContractKeyPointersso downstream agents stay aligned with the streamlined identifier flow.withCompositeId, its tests, and documentation updates so integercontract_idinputs can transparently resolve to the required T-/H- composite strings and compact responses keep emitting string IDs.Modified files (4)
Latest Contributors(2)
contract_id, defer composite prefix resolution towithCompositeId, and explain howget_contract_listfilters still use the composite format while responses drop the prefix.Modified files (9)
Latest Contributors(2)