feat: add get_tag tool and storage_paths CRUD#108
Conversation
Adds a get_tag detail-getter to bring tags in line with correspondents,
document_types and custom_fields, all of which already expose both
list_* and get_* variants. Without get_tag, agents have to fall back to
list_tags(name__iexact=...) and the asymmetry shows up as a foot-gun
(agents try get_tag first and fail).
The tool returns the same Tag payload as the other detail-getters, with
matching_algorithm enhanced from a numeric id to {id, name} via the
existing enhanceMatchingAlgorithm helper.
E2E coverage in e2e.test.ts asserts that a freshly-created tag is
returned by get_tag with id, name, non-empty slug, and the expanded
matching_algorithm shape.
Storage paths were not exposed at all — no list/get/create/update/delete
and no bulk_edit. They appeared only as a foreign-key parameter
(storage_path) on list_documents, post_document, update_document and
bulk_edit_documents, so agents had no way to discover or manage them
via the MCP and had to fall back to raw API calls.
The new tool family mirrors the correspondents/document_types pattern:
list_storage_paths, get_storage_path, create_storage_path,
update_storage_path, delete_storage_path (with the same confirm-flag
discipline as delete_correspondent) and bulk_edit_storage_paths.
Storage paths carry an additional required `path` field, a Django
template string such as "{{ correspondent }}/{{ created_year }}/{{ title }}".
The tool descriptions document this and reference the Paperless-NGX docs
for available placeholders.
E2E coverage in e2e.test.ts walks the full lifecycle:
- create_storage_path returns the new path with id, name and template
- get_storage_path returns the same payload with expanded matching_algorithm
- list_storage_paths includes the new id
- update_storage_path renames and preserves the path template
- bulk_edit_documents method=set_storage_path assigns the path to the
uploaded test document, and get_document reflects the assignment
- delete_storage_path requires confirm=true; the deleted id then
surfaces an isError from get_storage_path (404 round-trip)
Side note: this also closes the self-evidence gap from the existing
list_documents description, which already points agents at a
list_storage_paths tool that previously did not exist.
🦋 Changeset detectedLatest commit: 50c6e33 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds StoragePath types and PaperlessAPI methods for tag detail and full storage-path CRUD/list. Implements MCP tools exposing list/get/create/update/delete/bulk_edit storage-path operations plus a get_tag tool, wires them into server initialization, and adds e2e tests covering tag detail expansion, storage-path lifecycle, document integration via bulk edit, and deletion confirmation/error flows. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/e2e.test.ts`:
- Around line 284-301: The test "list_storage_paths returns the storage path
created earlier in this run" is flaky due to pagination; modify the
client.callTool invocation for "list_storage_paths" (the call in the it block
using client.callTool and parseToolText) to request a deterministic result by
either passing a filter for the storage path name (use RUN_STORAGE_PATH) or
increasing page_size to a sufficiently large value, then assert against the
filtered result (e.g., check data.results[0] or find by id as before) rather
than relying on the default page. Ensure you update the arguments object passed
to client.callTool so the API returns the created storage path reliably when
checking state.storagePathId and RUN_STORAGE_PATH.
In `@src/tools/storagePaths.ts`:
- Around line 97-103: The update_storage_path tool schema currently requires
fields like name and path which prevents PATCH-style partial updates; modify the
Zod input schema for update_storage_path to make updatable fields optional
(e.g., change name and path and any matchingPattern/matchingAlgorithm fields in
the same schema to z.string().optional() or the appropriate optional Zod types)
and ensure id remains required, and provide sensible defaults/validation where
needed so callers can send partial updates without server-side reassignment
logic.
- Around line 189-199: Normalize args.permissions before passing to
api.bulkEditObjects to avoid sending empty arrays/objects: import and use the
arrayNotEmpty and objectNotEmpty helpers from tools/utils/empty.ts in
storagePaths.ts, compute a normalizedPermissions value (e.g., call
arrayNotEmpty(args.permissions) and/or objectNotEmpty(args.permissions) so empty
collections become undefined) and replace direct use of args.permissions in the
payload for the "set_permissions" branch of api.bulkEditObjects; keep owner and
merge as-is but only include permissions when normalizedPermissions is not
undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b52f77ef-1625-487b-9ed6-962301a51e87
📒 Files selected for processing (6)
e2e/e2e.test.tssrc/api/PaperlessAPI.tssrc/api/types.tssrc/server.tssrc/tools/storagePaths.tssrc/tools/tags.ts
| it("list_storage_paths returns the storage path created earlier in this run", async () => { | ||
| assert.ok(state.storagePathId, "storage path must be created first"); | ||
| const result = (await client.callTool({ | ||
| name: "list_storage_paths", | ||
| arguments: {}, | ||
| })) as ToolResult; | ||
| assertOk(result, "list_storage_paths"); | ||
| const data = parseToolText(result) as { | ||
| results: { id: number; name: string }[]; | ||
| }; | ||
| assert.ok(Array.isArray(data.results), "results should be an array"); | ||
| const found = data.results.find((sp) => sp.id === state.storagePathId); | ||
| assert.ok( | ||
| found, | ||
| `storage_path id=${state.storagePathId} not found in list_storage_paths` | ||
| ); | ||
| assert.strictEqual(found.name, RUN_STORAGE_PATH); | ||
| }); |
There was a problem hiding this comment.
Make list_storage_paths lookup deterministic to avoid pagination flakiness.
This test can fail when the created item is outside the default page. Filter by name (or set an explicit large page_size) before asserting presence.
💡 Suggested change
const result = (await client.callTool({
name: "list_storage_paths",
- arguments: {},
+ arguments: { name__iexact: RUN_STORAGE_PATH, page_size: 200 },
})) as ToolResult;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("list_storage_paths returns the storage path created earlier in this run", async () => { | |
| assert.ok(state.storagePathId, "storage path must be created first"); | |
| const result = (await client.callTool({ | |
| name: "list_storage_paths", | |
| arguments: {}, | |
| })) as ToolResult; | |
| assertOk(result, "list_storage_paths"); | |
| const data = parseToolText(result) as { | |
| results: { id: number; name: string }[]; | |
| }; | |
| assert.ok(Array.isArray(data.results), "results should be an array"); | |
| const found = data.results.find((sp) => sp.id === state.storagePathId); | |
| assert.ok( | |
| found, | |
| `storage_path id=${state.storagePathId} not found in list_storage_paths` | |
| ); | |
| assert.strictEqual(found.name, RUN_STORAGE_PATH); | |
| }); | |
| it("list_storage_paths returns the storage path created earlier in this run", async () => { | |
| assert.ok(state.storagePathId, "storage path must be created first"); | |
| const result = (await client.callTool({ | |
| name: "list_storage_paths", | |
| arguments: { name__iexact: RUN_STORAGE_PATH, page_size: 200 }, | |
| })) as ToolResult; | |
| assertOk(result, "list_storage_paths"); | |
| const data = parseToolText(result) as { | |
| results: { id: number; name: string }[]; | |
| }; | |
| assert.ok(Array.isArray(data.results), "results should be an array"); | |
| const found = data.results.find((sp) => sp.id === state.storagePathId); | |
| assert.ok( | |
| found, | |
| `storage_path id=${state.storagePathId} not found in list_storage_paths` | |
| ); | |
| assert.strictEqual(found.name, RUN_STORAGE_PATH); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/e2e.test.ts` around lines 284 - 301, The test "list_storage_paths returns
the storage path created earlier in this run" is flaky due to pagination; modify
the client.callTool invocation for "list_storage_paths" (the call in the it
block using client.callTool and parseToolText) to request a deterministic result
by either passing a filter for the storage path name (use RUN_STORAGE_PATH) or
increasing page_size to a sufficiently large value, then assert against the
filtered result (e.g., check data.results[0] or find by id as before) rather
than relying on the default page. Ensure you update the arguments object passed
to client.callTool so the API returns the created storage path reliably when
checking state.storagePathId and RUN_STORAGE_PATH.
| "update_storage_path", | ||
| "Update an existing storage path's name, path template, matching pattern, or matching algorithm.", | ||
| { | ||
| id: z.number(), | ||
| name: z.string(), | ||
| path: z | ||
| .string() |
There was a problem hiding this comment.
Allow partial updates in update_storage_path.
update_storage_path currently requires name, which blocks valid PATCH-style updates (e.g., updating only path or only matching fields).
💡 Suggested change
- name: z.string(),
+ name: z.string().optional(),As per coding guidelines, "Prefer similar Zod schemas to the API when possible—keep tool inputs similar to the API rather than reassignment logic in the code" and "Include optional parameters with proper defaults in tool parameter schemas".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "update_storage_path", | |
| "Update an existing storage path's name, path template, matching pattern, or matching algorithm.", | |
| { | |
| id: z.number(), | |
| name: z.string(), | |
| path: z | |
| .string() | |
| "update_storage_path", | |
| "Update an existing storage path's name, path template, matching pattern, or matching algorithm.", | |
| { | |
| id: z.number(), | |
| name: z.string().optional(), | |
| path: z | |
| .string() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tools/storagePaths.ts` around lines 97 - 103, The update_storage_path
tool schema currently requires fields like name and path which prevents
PATCH-style partial updates; modify the Zod input schema for update_storage_path
to make updatable fields optional (e.g., change name and path and any
matchingPattern/matchingAlgorithm fields in the same schema to
z.string().optional() or the appropriate optional Zod types) and ensure id
remains required, and provide sensible defaults/validation where needed so
callers can send partial updates without server-side reassignment logic.
| return api.bulkEditObjects( | ||
| args.storage_path_ids, | ||
| "storage_paths", | ||
| args.operation, | ||
| args.operation === "set_permissions" | ||
| ? { | ||
| owner: args.owner, | ||
| permissions: args.permissions, | ||
| merge: args.merge, | ||
| } | ||
| : {} |
There was a problem hiding this comment.
Normalize empty permission arrays/objects before bulk set-permissions.
Forwarding args.permissions directly can send empty arrays/objects, which may unintentionally apply empty permission sets instead of omitting unset values.
🔧 Suggested change
import { withErrorHandling } from "./utils/middlewares";
import { buildQueryString } from "./utils/queryString";
+import { arrayNotEmpty, objectNotEmpty } from "./utils/empty";
@@
- return api.bulkEditObjects(
- args.storage_path_ids,
- "storage_paths",
- args.operation,
- args.operation === "set_permissions"
- ? {
- owner: args.owner,
- permissions: args.permissions,
- merge: args.merge,
- }
- : {}
- );
+ const permissions =
+ args.permissions &&
+ objectNotEmpty({
+ view: objectNotEmpty({
+ users: arrayNotEmpty(args.permissions.view.users),
+ groups: arrayNotEmpty(args.permissions.view.groups),
+ }),
+ change: objectNotEmpty({
+ users: arrayNotEmpty(args.permissions.change.users),
+ groups: arrayNotEmpty(args.permissions.change.groups),
+ }),
+ });
+
+ const params =
+ args.operation === "set_permissions"
+ ? objectNotEmpty({
+ owner: args.owner,
+ permissions,
+ merge: args.merge,
+ }) ?? {}
+ : {};
+
+ return api.bulkEditObjects(
+ args.storage_path_ids,
+ "storage_paths",
+ args.operation,
+ params
+ );As per coding guidelines, "Use arrayNotEmpty and objectNotEmpty transformation utilities from tools/utils/empty.ts to convert empty arrays and objects to undefined".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return api.bulkEditObjects( | |
| args.storage_path_ids, | |
| "storage_paths", | |
| args.operation, | |
| args.operation === "set_permissions" | |
| ? { | |
| owner: args.owner, | |
| permissions: args.permissions, | |
| merge: args.merge, | |
| } | |
| : {} | |
| const permissions = | |
| args.permissions && | |
| objectNotEmpty({ | |
| view: objectNotEmpty({ | |
| users: arrayNotEmpty(args.permissions.view.users), | |
| groups: arrayNotEmpty(args.permissions.view.groups), | |
| }), | |
| change: objectNotEmpty({ | |
| users: arrayNotEmpty(args.permissions.change.users), | |
| groups: arrayNotEmpty(args.permissions.change.groups), | |
| }), | |
| }); | |
| const params = | |
| args.operation === "set_permissions" | |
| ? objectNotEmpty({ | |
| owner: args.owner, | |
| permissions, | |
| merge: args.merge, | |
| }) ?? {} | |
| : {}; | |
| return api.bulkEditObjects( | |
| args.storage_path_ids, | |
| "storage_paths", | |
| args.operation, | |
| params | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tools/storagePaths.ts` around lines 189 - 199, Normalize args.permissions
before passing to api.bulkEditObjects to avoid sending empty arrays/objects:
import and use the arrayNotEmpty and objectNotEmpty helpers from
tools/utils/empty.ts in storagePaths.ts, compute a normalizedPermissions value
(e.g., call arrayNotEmpty(args.permissions) and/or
objectNotEmpty(args.permissions) so empty collections become undefined) and
replace direct use of args.permissions in the payload for the "set_permissions"
branch of api.bulkEditObjects; keep owner and merge as-is but only include
permissions when normalizedPermissions is not undefined.
2 participants
Closes #107.
Adds the two missing tool families surfaced in #107.
Changes
get_tag(commit 1)Brings tags in line with
correspondents,document_typesandcustom_fields, all of which already expose bothlist_*andget_*variants. Withoutget_tagagents fell back tolist_tags(name__iexact="…"); the asymmetry was a foot-gun (agents tryget_tagfirst and fail).storage_pathsCRUD tool family (commit 2)Storage paths were not exposed at all — they appeared only as a foreign-key parameter (
storage_path) onlist_documents,post_document,update_documentandbulk_edit_documents, so agents had no way to discover or manage them.The new tools mirror the
correspondents/document_typespattern:list_storage_pathsget_storage_pathcreate_storage_path(with required Django-templatepathfield)update_storage_pathdelete_storage_path(confirm: truediscipline, same asdelete_correspondent)bulk_edit_storage_pathsThis also closes the self-evidence gap: the existing
list_documentstool description already pointed agents at alist_storage_pathstool that previously did not exist.Tests
All new functionality is covered by E2E tests in
e2e/e2e.test.ts, following the existing pattern (real Paperless-NGX viadocker-compose.e2e.yml, MCP server spawned via HTTP transport, state threaded through thedescribeblock).New
it()blocks:get_tag returns the tag by ID with full detail fieldscreate_storage_path creates a storage path and returns it with an idget_storage_path returns the storage path by ID with full detail fieldslist_storage_paths returns the storage path created earlier in this runupdate_storage_path renames the storage path and the change is visible via getbulk_edit_documents set_storage_path assigns the storage path and get_document reflects it(integration with documents)delete_storage_path requires confirm=true and then removes the storage path(asserts `confirm: false` returns `isError`, confirm-true succeeds, and a subsequent `get_storage_path` round-trips a 404)Local run against `docker compose -f docker-compose.e2e.yml up` (paperless-ngx 2.14.7): 21/21 tests pass (13 existing + 8 new).
```
tests 21
pass 21
fail 0
```
Diff stat
```
e2e/e2e.test.ts | 196 ++++++++++++++++++
src/api/PaperlessAPI.ts | 43 ++++
src/api/types.ts | 17 ++
src/server.ts | 2 +
src/tools/storagePaths.ts | 203 ++++++++++++++++++++
src/tools/tags.ts | 14 ++
6 files changed, 475 insertions(+)
```
Split into two commits so each feature can be reviewed (or reverted) independently.
Summary by CodeRabbit
New Features
Improvements
Tests