fix(mcp): surface cloud project delete errors and support delete_notes - #1062
Conversation
Large cloud-project deletes could fail with a detail-free "Error removing project:" because httpx transport errors often stringify to an empty string. Client-side fixes for #1034: - call_get/call_put/call_patch/call_post/call_delete now wrap httpx.TransportError (including TimeoutException) in a ToolError with an actionable message that names the failure type; timeouts note the operation may still be completing server-side and point at `bm project list`. - `bm project remove` renders `str(e) or repr(e)` so the CLI never prints a blank error. - MCP delete_project gains a `delete_notes: bool = False` parameter (passed through to the typed ProjectClient, matching the CLI), and the result text now reflects reality: local vs cloud file location and deleted vs retained notes, instead of unconditionally claiming "Files remain on disk". The server-side purge half of #1034 is tracked in the cloud repo. Refs #1034 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review recheck |
4a39693 to
29a43a2
Compare
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: phernandez <paul@basicmachines.co>
|
The integration matrix failures were all the same stale retained-files assertion. I updated it to the intentional new message and verified the full MCP project-management integration file (18 passed), Ruff, formatting, and git diff checks. @codex review\n\n/recheck |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 242a4a2331
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review /recheck |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2aca7bb87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 359df9f993
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <paul@basicmachines.co>
|
/recheck |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Client-side half of #1034: deleting a large cloud project failed with a detail-free
Error removing project:because httpx transport errors (e.g.ReadTimeout) often stringify to an empty string, and the MCP delete tool both lackeddelete_notesand always claimed files remained "on disk".Defect (a): transport errors re-raised unwrapped in
call_*helperscall_delete(and its siblingscall_get/call_put/call_patch/call_posthad the same gap) only wrappedHTTPStatusErrorinToolError; transport errors escaped raw with often-empty messages. All five helpers now wraphttpx.TransportError(which includeshttpx.TimeoutException) in aToolErrorthat always names the exception type. Timeout messages note that long-running operations (such as deleting a large project) may still be completing server-side and point atbm project listbefore retrying.Defect (b): CLI printed a blank error
bm project removerenderedstr(e), which is empty for httpx timeouts. It now rendersstr(e) or repr(e)so the error line is never blank.Defect (c): MCP
delete_projectlackeddelete_notesand misreported file stateThe MCP tool now accepts
delete_notes: bool = Falseand passes it through to the typedProjectClient(which, like the CLI, already supported it). The result text now reflects reality: local vs cloud file location (mirroringget_clientrouting) and deleted vs retained note files, instead of unconditionally printing "Files remain on disk". The LLM-facing docstring was updated to match.Tests
tests/mcp/test_tool_utils.py: parametrized over all fivecall_*helpers via realhttpx.MockTransportcode paths — timeout wrapping (blankstr()), connect-error wrapping, and non-transport errors still passing through unwrapped.tests/mcp/test_tool_project_management.py: real create/delete flow assertingdelete_notes=Trueremoves the project directory on disk anddelete_notes=Falseretains it; cloud-routed delete asserts thedelete_notespassthrough and "cloud storage" wording; unit test for explicit-routing-flag handling.tests/cli/test_project_remove_errors.py(new):bm project removerendersrepr()for blank-message exceptions andstr()otherwise.tests/mcp/test_client_telemetry.py/test_tool_contracts.py: updated for the new wrapping and tool signature.ruff check,ruff format, andjust typecheckpass; 69 tests across the touched files pass, plus fulltests/mcp+tests/cliruns.Notes
The server-side half (purging large cloud projects without timing out) is tracked in the cloud repo.
Refs #1034 (server half remains)
🤖 Generated with Claude Code