feat(tools): Add kind parameter to upload_document for template uploads - #70
Merged
Conversation
Let upload_document optionally store the upload as a reusable template via a make_template flag, instead of a separate tool. make_template defaults to false, so plain document upload is unchanged; when true the returned document_id is a template id and next_steps switch to the template follow-ups (create_from_template / create_embedded_editor). Because adding a parameter changes a released tool contract, the flag ships as a new upload_document v3.0 registration in tools/signnow_v3.py; the v2.0 contract stays frozen and unchanged. FastMCP serves the highest version by default, so clients get the new flag transparently while a client pinning v2.0 keeps the old contract. Establish the governance rule in AGENTS.md: every tool contract change or new tool goes through the latest MCP version only (currently v3.0), never edits v1.0/v2.0 in place. CLAUDE.md now points to AGENTS.md and surfaces the rule. Supporting changes: - signnow_client.upload_document / CreateDocumentFromUrlRequest gain make_template (and an optional name for URL uploads); the flag is omitted when false so the API's PHP !empty() check can't treat a "false" string as truthy. - URL-upload filenames are validated leniently — a caller-supplied display name need not carry an extension, since SignNow types the fetched file server-side. - signnow101 skill and README document the flag; tests cover the make_template document/template/URL paths and the shared MCP-resource upload helper.
Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
src/sn_mcp_server/tools/signnow.pyLines 1052-1060 1052 raise ValueError("Provide exactly one of resource_uri, file_path, or file_url — not multiple")
1053 if provided == 0:
1054 raise ValueError("Provide one of: resource_uri, file_path, or file_url")
1055
! 1056 resource_bytes, filename = await _resolve_upload_resource(ctx, resource_uri, filename)
1057
1058 # H-3: Run synchronous _upload_document off the async event loop
1059 return await asyncio.to_thread(
1060 _upload_document,src/sn_mcp_server/tools/signnow_v3.pyLines 136-146 136
137 # Validate mutually-exclusive source inputs before any I/O
138 provided = sum(x is not None for x in (resource_uri, file_path, file_url))
139 if provided > 1:
! 140 raise ValueError("Provide exactly one of resource_uri, file_path, or file_url — not multiple")
141 if provided == 0:
! 142 raise ValueError("Provide one of: resource_uri, file_path, or file_url")
143
144 resource_bytes, filename = await _resolve_upload_resource(ctx, resource_uri, filename)
145
146 # Run synchronous _upload_document off the async event loop |
hvenhlovskyi
marked this pull request as draft
July 14, 2026 13:10
Swap the boolean make_template argument on the v3.0 upload_document tool for a kind parameter accepting "document" (default) or "template". This reads better as an agent-facing contract than a boolean and leaves room for future upload kinds without another flag. kind lives only at the tool boundary (signnow_v3.py); it maps to make_template=(kind == "template") before calling the business logic. The signnow_client / CreateDocumentFromUrlRequest layers keep make_template unchanged, since that is the actual SignNow API field. Since the make_template flag was only introduced earlier in this same branch and never released, this edits the v3.0 contract in place rather than adding a new version. Update the tool description, docstring, and UploadDocumentResponse field descriptions, plus the README and signnow101 skill, to speak in terms of kind. Existing tests for the shared upload helper still exercise make_template at the business-logic layer and pass unchanged.
There was a problem hiding this comment.
Pull request overview
Adds support for uploading a file as a reusable template via the existing upload_document tool flow, implemented as a new v3.0 tool registration while keeping the v2.0 contract available.
Changes:
- Introduces
upload_documentv3.0 registration with a new selector (kind=document|template) and wires it into tool registration/version invariants. - Extends upload plumbing to support template uploads end-to-end (tool → business logic → SignNow client), including omission of
make_templatewhen false to avoid PHP!empty()pitfalls. - Adds/updates unit, integration, and API tests for resource/URL/local uploads and template-specific follow-ups; updates docs/skills/README accordingly.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/sn_mcp_server/tools/test_upload_resource.py | New unit coverage for _resolve_upload_resource resource attachment handling. |
| tests/unit/sn_mcp_server/tools/test_document.py | Expands upload tests (URL naming rules, make_template behavior, template next_steps). |
| tests/unit/sn_mcp_server/test_tool_versions.py | Adds v3/v2 coexistence invariant for contract changes (upload_document). |
| tests/integration/test_upload_document.py | Integration coverage for template uploads over resource bytes and URL paths. |
| tests/api/test_upload_document.py | API-layer assertions for make_template omission/presence and URL request optional fields. |
| src/sn_mcp_server/tools/signnow.py | Extracts resource attachment resolution into _resolve_upload_resource; keeps v2 upload orchestration. |
| src/sn_mcp_server/tools/signnow_v3.py | New v3.0 tool registrations, including upload_document with `kind=document |
| src/sn_mcp_server/tools/models.py | Clarifies UploadDocumentResponse semantics for template uploads. |
| src/sn_mcp_server/tools/document.py | Refactors upload logic, adds template next_steps/guidance, and supports passing make_template/name to client. |
| src/sn_mcp_server/tools/init.py | Registers v3 tool binder alongside v1 and v2. |
| src/sn_mcp_server/skills/signnow101.md | Documents template upload flow via upload_document(kind='template'). |
| src/signnow_client/models/templates_and_documents.py | Extends CreateDocumentFromUrlRequest with make_template and name. |
| src/signnow_client/client_documents.py | Adds make_template to multipart upload and uses exclude_none=True for URL payloads. |
| README.md | Updates upload_document docs to describe template uploads via kind='template'. |
| Makefile | Adds uv-up convenience target. |
| CLAUDE.md | New pointer file directing governance rules to AGENTS.md. |
| AGENTS.md | Adds explicit tool-versioning governance rule and updates last-updated date. |
Comment on lines
231
to
235
| @@ -135,70 +235,85 @@ def _upload_document( | |||
| raise ValueError("Provide one of: resource_uri, file_path, or file_url") | |||
…eview notes Add tests/unit/sn_mcp_server/tools/test_upload_document_v3.py exercising the v3.0 upload_document tool body end to end (token, client, and resource resolution patched): kind='template' forwards make_template=True and returns template follow-ups, while the default 'document' forwards make_template=False. This covers the tool-boundary mapping the shared _upload_document helper tests don't reach. Address PR review comments on document.py: - _upload() validation messages named resource_uri, but the helper takes resource_bytes; rename them so direct callers/tests see accurate arguments. - _upload_document() filename docstring said "document" only; reword to entity (document or template) and note URL uploads transmit an explicit name. Update the corresponding no-source test assertion to the resource_bytes wording.
mihasicehcek
approved these changes
Jul 21, 2026
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.
Let
upload_documentoptionally store the upload as a reusable template via akindparameter ("document"|"template"), instead of a separate tool.kinddefaults to"document", so plain document upload is unchanged; withkind="template"the returneddocument_idis a template id andnext_stepsswitch to the template follow-ups (create_from_template / create_embedded_editor).Because adding a parameter changes a released tool contract, it ships as a new
upload_documentv3.0 registration intools/signnow_v3.py; the v2.0 contract stays frozen and unchanged. FastMCP serves the highest version by default, so clients get the new contract transparently while a client pinning v2.0 keeps the old one.The
kindselector lives only at the tool boundary; it maps tomake_template = kind == "template"before calling the shared business logic. Thesignnow_client/CreateDocumentFromUrlRequestlayers keepmake_template, since that is the actual SignNow API field.Establish the governance rule in AGENTS.md: every tool contract change or new tool goes through the latest MCP version only (currently v3.0), never edits v1.0/v2.0 in place. CLAUDE.md now points to AGENTS.md and surfaces the rule.
Supporting changes:
signnow_client.upload_document/CreateDocumentFromUrlRequestgainmake_template(and an optionalnamefor URL uploads); the flag is omitted when false so the API's PHP!empty()check can't treat a"false"string as truthy.kind='template'; tests cover the document / template / URL paths and the shared MCP-resource upload helper.