fix: stop advertising unsupported schedule mutations in SDK and MCP#48
Open
lorus-ipsum wants to merge 6 commits intoGenentech:mainfrom
Open
fix: stop advertising unsupported schedule mutations in SDK and MCP#48lorus-ipsum wants to merge 6 commits intoGenentech:mainfrom
lorus-ipsum wants to merge 6 commits intoGenentech:mainfrom
Conversation
Remove unsupported schedule mutation advertisements from SDK and MCP surfaces. Made-with: Cursor
The service layer always rejects replaceSchedule and deleteSchedule with ConflictError because schedule edges are derived from startDate. Remove these operations from the three public surfaces that still advertised them: - Move37Client: removed replaceSchedule() and deleteSchedule() - useActivityGraph hook: removed replaceSchedule and deleteSchedule - McpToolRegistry: removed activity.schedule.replace and schedule.delete tool definitions, handler entries, and handler methods REST routes are intentionally left in place per issue Genentech#23 scope. Closes Genentech#23 Made-with: Cursor
SDK client tests:
- replaceSchedule is not a property of Move37Client
- deleteSchedule is not a property of Move37Client
React hook test:
- useActivityGraph return object omits replaceSchedule/deleteSchedule
Python API tests:
- MCP tools/list response omits activity.schedule.replace and schedule.delete
- REST PUT /activities/{id}/schedule still returns 409 ConflictError
- REST DELETE /schedules/{a}/{b} still returns 409 ConflictError
Made-with: Cursor
Remove ReplaceActivityScheduleInput and ScheduleEdgeInput from
schemas.py. These were only referenced by the MCP handler methods
removed in the CORE commit and are now dead code. The REST-facing
schemas (ReplaceScheduleInput, SchedulePeerInput) remain in place.
Also add two tests proving that tools/call with the removed tool names
(activity.schedule.replace, schedule.delete) returns JSON-RPC error
code -32001 ("Unknown tool").
Made-with: Cursor
Mark both REST schedule mutation endpoints as deprecated in their FastAPI route decorators so the generated OpenAPI spec shows deprecated: true and a 409 response description. This ensures Fern docs and any auto-generated SDK clients surface the unsupported status. Add a "Schedule edge mutations" section to fern/pages/sdks.mdx explaining that the Node SDK intentionally omits these methods and directing users to updateActivity or /v1/scheduling/replan instead. Add docstrings to the replace_schedule and delete_schedule service stubs explaining they exist solely to back the legacy REST routes. Made-with: Cursor
Move prompt history, implementation plan, and issue selection recommendation to assessment/ for PR reference. Made-with: Cursor
Author
|
In hindsight, I think a better way to do this would be to write the PR with the original plan, including the core, additional and stretch chunks as well as the validation checklist for each. Then as each commit was made for each of the three chunks I could add comments detailing any deviations from the plan above and ticking off completed validation checklists. Finally add a comment such as the PR above, summarising the approach and steps taken. I think with would result in a more auditable PR and demonstrate train of thought more clearly. |
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.
Closes #23 — Stop advertising unsupported schedule mutations in SDK and MCP.
Why this issue
I reviewed all 19 open issues against three criteria: fix existing functionality over new features, no external dependency blockers, and achievable scope for a time-boxed session. Issue #23 stood out because it is a real API contract mismatch — the SDK, React hooks, and MCP tool registry all advertise schedule mutation operations that the service layer unconditionally rejects. This misleads both human developers and AI agents. It also touches Python and JavaScript across multiple architectural layers (service, transport, SDK, hooks), giving a good signal on cross-stack understanding without requiring Docker, API keys, or external services. Full analysis:
assessment/issue_selection_recommendation.md.Solution summary
The core fix removes
replaceScheduleanddeleteSchedulefrom the three public surfaces that advertised them: the Node SDK client, theuseActivityGraphReact hook, and the MCP tool registry. On top of that, I added 8 regression tests across Python and JavaScript, removed orphaned Pydantic schemas, marked the legacy REST endpoints as deprecated in OpenAPI, added a design note to the Fern SDK docs, and documented the service stubs. The work was structured as four incremental commits — each validated against a checklist before proceeding to the next. This is an intentional surface-area reduction, not a broken feature removal.Planning artifacts:
assessment/contains the implementation plan, issue selection recommendation, and prompt history.Acceptance Criteria Coverage
Completed:
replaceSchedule(...)anddeleteSchedule(...)fromMove37Clientinclient.jsreplaceScheduleanddeleteSchedulefrom the object returned byuseActivityGraph(...)activity.schedule.replaceandschedule.deletefromMcpToolRegistrydefinitions and handlersAdditionally completed (beyond acceptance criteria):
ReplaceActivityScheduleInput,ScheduleEdgeInput) that became dead code after the MCP handler removaltools/callerror-path tests proving removed tools return-32001("Unknown tool")deprecated=Truein FastAPI route decorators, flowing"deprecated": trueand a409response description into the generated OpenAPI specfern/pages/sdks.mdx)replace_scheduleanddelete_scheduleservice stubs explaining they exist solely to back legacy REST routesNot completed:
Validation
Commands run after each commit
Results
"deprecated": truemodel_rebuild()* Pre-existing Python failures (verified identical on
mainby stashing changes and re-running):test_fork_clears_scheduling_fields—StopIteration(seed data issue)test_disconnect_clears_owner_scoped_account— assertion mismatchtest_apply_updates_graph_dates_and_syncs_calendar— date-dependent (uses today's date vs hardcoded2026-03-23)** Pre-existing SDK failure (verified identical on
main):useAppleCalendarIntegration > loads status and supports connecting—connectedfield assertionPrompt History
Full history with context:
assessment/prompt-history.mdKey prompts that materially influenced the work:
AI Mistakes And Corrections
Issue: When first running the Python test suite, the assistant used
unittest discoverwhich failed withImportError: Start directory is not importablebecausesrc/move37/tests/lacked an__init__.py. The assistant created the file to fix the import path.Correction: I identified this as a local environment issue (CI uses a different test runner configuration). The temporary
__init__.pywas removed after test execution and not committed. Subsequent runs usedpytestwhich handles package discovery differently and does not require it.Issue: The assistant initially wrote the MCP
tools/listtest by accessingself.client.app.state.mcp_transportdirectly and callinghandle_requestas a method call, bypassing the HTTP layer entirely.Correction: I had the assistant rewrite the test to go through the actual HTTP endpoint (
POST /v1/mcp/ssewith a JSON-RPC payload), which exercises the full transport stack and matches how a real MCP client would interact with the API. This caught that the correct endpoint path is/v1/mcp/sse, not a direct object method call.Issue: The assistant initially included unnecessary graph setup (
GET /v1/graph) in the REST 409 tests, not realising thatreplace_scheduleanddelete_scheduleraiseConflictErrorunconditionally without touching the database.Correction: After reading the service layer stubs, I had the tests simplified to call the routes directly with arbitrary IDs, confirming the 409 is always returned regardless of graph state. This makes the tests faster and more clearly documents the stub behaviour.
Limitations And Deferred Work
replaceScheduleanddeleteScheduleis a breaking change to the SDK's public API. The SDK is pre-1.0 (nopackage.jsonversion field) so semver does not strictly require a major bump, but a changelog entry or version bump would be good practice in a follow-up.activity.schedule.replacereceived error code-32000(ConflictError from the service layer). After this PR, it receives-32001("Unknown tool" from the transport layer). This is intentional — the tool should not be discoverable — but any MCP client that was catching-32000specifically would need updating.PUT /v1/activities/{id}/schedule,DELETE /v1/schedules/{a}/{b}) remain in place and always return 409. They are now markeddeprecated=Truein OpenAPI. A future PR could remove them entirely as a breaking REST change.ReplaceScheduleInputandSchedulePeerInputinschemas.pyare only used by the deprecated REST routes. If those routes are removed in a future PR, these schemas should be removed too.main(documented in Validation above). These are unrelated to this PR and should be addressed separately.Candidate Checklist
assessmentdir containing a full list of prompts and Claude plan mds before merging.