Add Cospend tools — 16 tools for projects, members, bills (OCS)#52
Add Cospend tools — 16 tools for projects, members, bills (OCS)#52oleksandr-nc merged 6 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new Cospend MCP tool module (projects, members, bills, statistics, settlement), registers it with the MCP server, updates docs, and includes integration tests and cleanup for Cospend against a real Nextcloud instance. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant MCP as FastMCP Server
participant Cospend as apps/cospend OCS
participant NC as Nextcloud Core
Client->>MCP: call MCP tool (e.g., create_bill)
MCP->>Cospend: HTTP request to /ocs/v2.php/apps/cospend/api/v1/... (body filtered)
Cospend->>NC: read/write DB (projects/members/bills)
NC-->>Cospend: operation result (JSON)
Cospend-->>MCP: OCS JSON response
MCP-->>Client: JSON result (tool response)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #52 +/- ##
==========================================
- Coverage 95.99% 94.08% -1.91%
==========================================
Files 30 31 +1
Lines 3222 3367 +145
==========================================
+ Hits 3093 3168 +75
- Misses 129 199 +70
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
(AI) Ready for review. CI green on Lint, Type Check, Python 3.12-3.14, NC 32 (skips Cospend tests since min-version=33), NC 33, plus Session cache and User permissions on both NC versions. Summary:
Notable design notes captured in tool docstrings:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
33-36:⚠️ Potential issue | 🟡 MinorStale tool / app counts in the heading and callout.
After adding 16 Cospend tools and 1 new app (Cospend), the totals here should track
PROGRESS.md:
- "140 Tools Across 23 Nextcloud Apps" → 156 Tools Across 24 Nextcloud Apps
- "A 141st tool,
upload_file_from_path" → 157th tool📝 Proposed update
-## 140 Tools Across 23 Nextcloud Apps +## 156 Tools Across 24 Nextcloud Apps -A 141st tool, `upload_file_from_path`, is registered only when the operator sets +A 157th tool, `upload_file_from_path`, is registered only when the operator sets `NEXTCLOUD_MCP_UPLOAD_ROOT`. See [Files](`#files`) for details.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 33 - 36, Update the stale counts in the README heading and callout: change the section title string "140 Tools Across 23 Nextcloud Apps" to "156 Tools Across 24 Nextcloud Apps" and update the callout sentence "A 141st tool, `upload_file_from_path`" to "A 157th tool, `upload_file_from_path`" so the numbers match PROGRESS.md; ensure the surrounding text and anchor [Files](`#files`) remain intact.
🧹 Nitpick comments (2)
tests/integration/conftest.py (1)
221-228: Optional: short-circuit when Cospend isn't installed.
_cleanup_cospendis invoked by the autouse_clean_test_datafixture for every integration test. On instances without the Cospend app, this still issues a 404'dapps/cospend/api/v1/projectsrequest per test (silenced by the outercontextlib.suppress). Consider gating it on the same_cospend_availableprobe used intests/integration/test_cospend.py, or caching a one-shot "cospend installed" flag at session scope to skip the call entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/conftest.py` around lines 221 - 228, The _cleanup_cospend function currently issues a 404 on instances without Cospend; update it to short-circuit by checking the existing _cospend_available probe (as used in test_cospend) or a cached session-scoped boolean before calling client.ocs_get, e.g. consult the _cospend_available check (or initialize a one-shot session flag) and skip the API call entirely when false to avoid per-test 404 requests; keep the existing contextlib.suppress around delete calls for safety.PROGRESS.md (1)
53-53: Optional: split "Next Up" back into bullets.The single sentence packs Cospend follow-ups, Weather Status, and the skip list together; bulleting these (as before) reads better and makes the actual next item obvious at a glance.
📝 Suggested layout
-- Cospend follow-ups (Shares: 5 share types; Categories/Payment modes/Currencies; Statistics export). Weather Status (fully OCS, bundled). Tables, Polls, Notes, Deck, Bookmarks, Photos skipped — API not OCS or OCS incomplete. +- Cospend follow-ups: Shares (5 share types), Categories/Payment modes/Currencies, Statistics export +- Weather Status (fully OCS, bundled) +- Skipped — API not OCS or OCS incomplete: Tables, Polls, Notes, Deck, Bookmarks, Photos🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PROGRESS.md` at line 53, The "Next Up" single-line entry containing "Cospend follow-ups (Shares: 5 share types; Categories/Payment modes/Currencies; Statistics export). Weather Status (fully OCS, bundled). Tables, Polls, Notes, Deck, Bookmarks, Photos skipped — API not OCS or OCS incomplete." should be split back into separate bullet items so each next-task is visible at a glance; replace that single sentence with three bullets (1) Cospend follow-ups — list the subitems "Shares: 5 share types; Categories/Payment modes/Currencies; Statistics export", (2) Weather Status — note "fully OCS, bundled", and (3) Skipped items — "Tables, Polls, Notes, Deck, Bookmarks, Photos" with the note "API not OCS or OCS incomplete" so each item and its status are clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nc_mcp_server/tools/cospend.py`:
- Around line 471-537: The update_cospend_bill handler currently maps
payed_for=[] to payedFor="" which can unintentionally clear owers; change the
payed_for handling in update_cospend_bill so that when payed_for is an empty
list it uses None (i.e., omit the payedFor key) instead of ",". Concretely,
update the payedFor argument passed to _body (in update_cospend_bill) to be
",".join(str(m) for m in payed_for) if payed_for is not None and payed_for != []
else None; optionally add a one-line docstring note that passing [] does not
clear owers on update (or raise ValueError if you prefer explicit rejection).
Ensure references: update_cospend_bill, payed_for, payedFor, and _body.
In `@tests/integration/test_cospend.py`:
- Around line 139-220: Add a new async test that exercises update_cospend_member
with activated=False and then activated=True to cover the soft-disable lifecycle
via the update path: create a project (use _make_project), create a member
(_make_member), call nc_mcp.call("update_cospend_member", project_id=...,
member_id=..., activated=False) and assert the returned member and subsequent
list_cospend_members show activated is False (member kept but disabled), then
call update_cospend_member with activated=True and assert it is re-enabled;
ensure the test uses the same patterns as other tests and targets the
update_cospend_member code path noted in tools/cospend.py to catch the
create/update type/key asymmetry.
---
Outside diff comments:
In `@README.md`:
- Around line 33-36: Update the stale counts in the README heading and callout:
change the section title string "140 Tools Across 23 Nextcloud Apps" to "156
Tools Across 24 Nextcloud Apps" and update the callout sentence "A 141st tool,
`upload_file_from_path`" to "A 157th tool, `upload_file_from_path`" so the
numbers match PROGRESS.md; ensure the surrounding text and anchor
[Files](`#files`) remain intact.
---
Nitpick comments:
In `@PROGRESS.md`:
- Line 53: The "Next Up" single-line entry containing "Cospend follow-ups
(Shares: 5 share types; Categories/Payment modes/Currencies; Statistics export).
Weather Status (fully OCS, bundled). Tables, Polls, Notes, Deck, Bookmarks,
Photos skipped — API not OCS or OCS incomplete." should be split back into
separate bullet items so each next-task is visible at a glance; replace that
single sentence with three bullets (1) Cospend follow-ups — list the subitems
"Shares: 5 share types; Categories/Payment modes/Currencies; Statistics export",
(2) Weather Status — note "fully OCS, bundled", and (3) Skipped items — "Tables,
Polls, Notes, Deck, Bookmarks, Photos" with the note "API not OCS or OCS
incomplete" so each item and its status are clear.
In `@tests/integration/conftest.py`:
- Around line 221-228: The _cleanup_cospend function currently issues a 404 on
instances without Cospend; update it to short-circuit by checking the existing
_cospend_available probe (as used in test_cospend) or a cached session-scoped
boolean before calling client.ocs_get, e.g. consult the _cospend_available check
(or initialize a one-shot session flag) and skip the API call entirely when
false to avoid per-test 404 requests; keep the existing contextlib.suppress
around delete calls for safety.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a22a2f73-2a3f-4420-a3ad-14c833f7ebe4
📒 Files selected for processing (7)
PROGRESS.mdREADME.mdsrc/nc_mcp_server/server.pysrc/nc_mcp_server/tools/cospend.pytests/integration/conftest.pytests/integration/test_cospend.pytests/integration/test_server.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nc_mcp_server/tools/cospend.py (1)
105-105: Inconsistent boolean query parameter serialization across endpoints requires clarification.
get_cospend_project_statisticsencodesshowDisabledas"1"/"0"(line 105), whilelist_cospend_billsencodesreverseas"true"/"false"(line 225), anddeletedis passed as an integer (line 224). Integration tests pass but do not exercise these boolean parameters with true values, so the formats are not actually validated. If this inconsistency mirrors upstream Cospend API expectations (as suggested by the intentional create/update field-name asymmetry for member activation), document this assumption. Otherwise, standardize to a single format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nc_mcp_server/tools/cospend.py` at line 105, get_cospend_project_statistics currently serializes showDisabled as "1"/"0" while list_cospend_bills uses "true"/"false" for reverse and an integer for deleted; pick a single boolean serialization (either JSON-style "true"/"false" or numeric "1"/"0") or explicitly document that these formats are intentionally different to match upstream Cospend behavior. Update the serialization in get_cospend_project_statistics (params key showDisabled), list_cospend_bills (params keys reverse and deleted) to use the chosen consistent format, add a short comment in each function (get_cospend_project_statistics, list_cospend_bills) stating the expected API format if it must differ, and add/adjust unit or integration tests to exercise true/active values so the behavior is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/nc_mcp_server/tools/cospend.py`:
- Line 105: get_cospend_project_statistics currently serializes showDisabled as
"1"/"0" while list_cospend_bills uses "true"/"false" for reverse and an integer
for deleted; pick a single boolean serialization (either JSON-style
"true"/"false" or numeric "1"/"0") or explicitly document that these formats are
intentionally different to match upstream Cospend behavior. Update the
serialization in get_cospend_project_statistics (params key showDisabled),
list_cospend_bills (params keys reverse and deleted) to use the chosen
consistent format, add a short comment in each function
(get_cospend_project_statistics, list_cospend_bills) stating the expected API
format if it must differ, and add/adjust unit or integration tests to exercise
true/active values so the behavior is validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d03a344-437a-4354-8a67-bfe8a573c172
📒 Files selected for processing (2)
src/nc_mcp_server/tools/cospend.pytests/integration/test_cospend.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nc_mcp_server/tools/cospend.py`:
- Around line 192-214: The function that lists bills (the one accepting
search_term and limit) can silently ignore search when search_term is provided
but limit is None; modify the function (referencing the search_term and limit
parameters in the bills-listing function) to fail fast: when search_term is not
None and limit is None, raise a clear ValueError (or TypeError) telling callers
to provide a limit (or set a default limit), so the search behavior is enforced;
apply the same guard to the similar bills-listing variant referenced in lines
~225-237.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 379e21f9-e9b1-45d4-a6d3-6926f69651f3
📒 Files selected for processing (3)
src/nc_mcp_server/tools/cospend.pytests/integration/conftest.pytests/integration/test_cospend.py
✅ Files skipped from review due to trivial changes (1)
- tests/integration/test_cospend.py
1 participant
Summary
Adds Cospend (shared expense tracking) tool support to the MCP server. Cospend is an extension app — not bundled with Nextcloud — and exposes a clean OCS v1 API at
/ocs/v2.php/apps/cospend/api/v1/. This PR covers the foundation: enough tools to use Cospend end-to-end (create a project, add members, log expenses, see balances).Scope (this PR)
Projects (7):
list_cospend_projects,get_cospend_project,create_cospend_project,update_cospend_project,delete_cospend_project,get_cospend_project_statistics,get_cospend_project_settlementMembers (4):
list_cospend_members,create_cospend_member,update_cospend_member,delete_cospend_memberBills (5):
list_cospend_bills,get_cospend_bill,create_cospend_bill,update_cospend_bill,delete_cospend_billTotal: 16 tools, 35 integration tests (all green on master + stable33).
Out of scope (future PRs)
Notable design decisions
create_cospend_billdefaultsdateto today (UTC) when neitherdatenortimestampis provided. The controller signature marks both as optional, but the service layer raises 400 unless one is given — defaulting saves users from a guaranteed-fail call.delete_cospend_billdefaultsmove_to_trash=True, matching the API default. Passmove_to_trash=Falsefor an irreversible purge.list_cospend_bills:search_termis silently ignored unlesslimitis also set (Cospend uses different code paths for limit vs no-limit).list_cospend_bills:nb_billsis the project-wide count under filters but is not affected bysearch_term, even whensearch_termfilters the returnedbills.update_cospend_member: Settingactivated=Falseon a member with no bills deletes them rather than disabling.delete_cospend_member: Members with bills are soft-disabled (kept for bill history); bill-less members are removed.Test plan
ruff check .cleanpyrightcleanTestToolRegistration)Summary by CodeRabbit
New Features
Documentation
Progress
Tests