Skip to content

Commit 9dd7962

Browse files
committed
fix(collectives): surface failed requests instead of passing them on
Three findings from the Copilot review on #159, all the same shape: a failed request was treated as a successful one. get_page_content only special-cased 404. Any other failure was handed back as if the response body were the page's markdown - in practice a DAV error document, which the agent would then summarize as page content. The 404 contract is unchanged and still comes first; every 4xx and 5xx now raises. update_page_content discarded the PUT response entirely and always reported success, so a refused write looked like a completed one to the agent and to the user. The realistic case is the one the tool's own docstring warns about: a page held open in the real-time editor is locked, the write comes back 423, and the tool said it had gone through. Both use response.raise_for_status(). The review suggested it verbatim for the read; the write is the same problem and gets the same treatment. It is also what nc_py_api's own check_error() uses internally. Its message carries status, reason and URL, and graph.py's handle_tool_error hands repr() of the exception back to the model, so the agent sees what failed rather than a bare status code. 2xx multi-status responses do not raise. is_available caught bare, which also swallows asyncio.CancelledError and turns a cancellation during tool discovery into "Collectives unavailable" while discovery carries on - freezing an incomplete tool list in the 60-second cache around it. It now catches Exception, so cancellation propagates. Signed-off-by: Pavlinchen <69079839+Pavlinchen@users.noreply.github.com>
1 parent 2df6b3d commit 9dd7962

1 file changed

Lines changed: 4 additions & 2 deletions

File tree

‎ex_app/lib/all_tools/collectives.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ async def get_page_content(collective_id: int, page_id: int):
116116
})
117117
if response.status_code == 404:
118118
return ''
119+
response.raise_for_status()
119120
return response.text
120121

121122
@tool
@@ -171,9 +172,10 @@ async def update_page_content(collective_id: int, page_id: int, content: str):
171172
url = await _page_webdav_url(user_id, page)
172173
body = _strip_ai_disclaimer(content).rstrip()
173174
stamped = f"{body}\n\n{_AI_DISCLAIMER}\n" if body else f"{_AI_DISCLAIMER}\n"
174-
await nc._session._create_adapter(True).request('PUT', url, headers={
175+
response = await nc._session._create_adapter(True).request('PUT', url, headers={
175176
'Content-Type': 'text/markdown',
176177
}, data=stamped)
178+
response.raise_for_status()
177179
return json.dumps({'status': 'success', 'page_id': page_id})
178180

179181
@tool
@@ -281,6 +283,6 @@ def get_category_name():
281283
async def is_available(nc: AsyncNextcloudApp):
282284
try:
283285
await nc.ocs('GET', '/ocs/v2.php/apps/collectives/api/v1.0/collectives')
284-
except:
286+
except Exception:
285287
return False
286288
return True

0 commit comments

Comments
 (0)