From f28833a39c523f5e5db3b301f79497d489bf9a01 Mon Sep 17 00:00:00 2001 From: Ewan Wallace Date: Thu, 2 Apr 2026 19:49:08 +0100 Subject: [PATCH 1/6] chore: open branch for issue #23 Remove unsupported schedule mutation advertisements from SDK and MCP surfaces. Made-with: Cursor From 3ea09e56972552e76a332f67a15d7f6ce8eca359 Mon Sep 17 00:00:00 2001 From: Ewan Wallace Date: Thu, 2 Apr 2026 20:19:47 +0100 Subject: [PATCH 2/6] fix: remove unsupported schedule mutations from SDK and MCP 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 #23 scope. Closes #23 Made-with: Cursor --- src/move37/api/tool_registry.py | 28 ------------------- src/move37/sdk/node/src/client.js | 15 ---------- .../sdk/node/src/hooks/useActivityGraph.js | 4 --- 3 files changed, 47 deletions(-) diff --git a/src/move37/api/tool_registry.py b/src/move37/api/tool_registry.py index 3fd6189..4486c56 100644 --- a/src/move37/api/tool_registry.py +++ b/src/move37/api/tool_registry.py @@ -47,13 +47,7 @@ def __init__(self, services: ServiceContainer) -> None: "Replace an activity's dependencies.", schemas.ReplaceActivityDependenciesInput, ), - ToolDefinition( - "activity.schedule.replace", - "Replace derived schedule edges for an activity.", - schemas.ReplaceActivityScheduleInput, - ), ToolDefinition("dependency.delete", "Delete a dependency edge.", schemas.DependencyEdgeInput), - ToolDefinition("schedule.delete", "Delete a schedule edge.", schemas.ScheduleEdgeInput), ToolDefinition("note.create", "Create a note and linked graph node.", schemas.NotePayload), ToolDefinition("note.update", "Update a note and linked graph node.", schemas.UpdateNoteInput), ToolDefinition("note.get", "Fetch a note.", schemas.NoteIdInput), @@ -75,9 +69,7 @@ def __init__(self, services: ServiceContainer) -> None: "activity.fork": self._handle_activity_fork, "activity.delete": self._handle_activity_delete, "activity.dependencies.replace": self._handle_activity_dependencies_replace, - "activity.schedule.replace": self._handle_activity_schedule_replace, "dependency.delete": self._handle_dependency_delete, - "schedule.delete": self._handle_schedule_delete, "note.create": self._handle_note_create, "note.update": self._handle_note_update, "note.get": self._handle_note_get, @@ -193,16 +185,6 @@ def _handle_activity_dependencies_replace(self, subject: str, arguments: dict[st ) ).model_dump() - def _handle_activity_schedule_replace(self, subject: str, arguments: dict[str, Any]) -> dict[str, Any]: - payload = schemas.ReplaceActivityScheduleInput.model_validate(arguments) - return schemas.ActivityGraphOutput( - **self._services.activity_graph_service.replace_schedule( - subject, - payload.activityId, - [], - ) - ).model_dump() - def _handle_dependency_delete(self, subject: str, arguments: dict[str, Any]) -> dict[str, Any]: payload = schemas.DependencyEdgeInput.model_validate(arguments) return schemas.ActivityGraphOutput( @@ -213,16 +195,6 @@ def _handle_dependency_delete(self, subject: str, arguments: dict[str, Any]) -> ) ).model_dump() - def _handle_schedule_delete(self, subject: str, arguments: dict[str, Any]) -> dict[str, Any]: - payload = schemas.ScheduleEdgeInput.model_validate(arguments) - return schemas.ActivityGraphOutput( - **self._services.activity_graph_service.delete_schedule( - subject, - payload.earlierId, - payload.laterId, - ) - ).model_dump() - def _handle_note_create(self, subject: str, arguments: dict[str, Any]) -> dict[str, Any]: payload = schemas.NotePayload.model_validate(arguments) response = self._services.note_service.create_note(subject, title=payload.title, body=payload.body) diff --git a/src/move37/sdk/node/src/client.js b/src/move37/sdk/node/src/client.js index 17b37de..33fd9b2 100644 --- a/src/move37/sdk/node/src/client.js +++ b/src/move37/sdk/node/src/client.js @@ -162,13 +162,6 @@ export class Move37Client { }); } - replaceSchedule(activityId, peers) { - this.logOperation("replaceSchedule", { activityId, peers }); - return this.request("PUT", `/v1/activities/${encodeURIComponent(activityId)}/schedule`, { - body: { peers }, - }); - } - deleteDependency(parentId, childId) { this.logOperation("deleteDependency", { parentId, childId }); return this.request( @@ -177,14 +170,6 @@ export class Move37Client { ); } - deleteSchedule(earlierId, laterId) { - this.logOperation("deleteSchedule", { earlierId, laterId }); - return this.request( - "DELETE", - `/v1/schedules/${encodeURIComponent(earlierId)}/${encodeURIComponent(laterId)}`, - ); - } - async request(method, path, opts = {}) { const query = toQueryString(opts.query); const url = `${this.baseUrl}${path}${query ? `?${query}` : ""}`; diff --git a/src/move37/sdk/node/src/hooks/useActivityGraph.js b/src/move37/sdk/node/src/hooks/useActivityGraph.js index bf2e343..48fa3e0 100644 --- a/src/move37/sdk/node/src/hooks/useActivityGraph.js +++ b/src/move37/sdk/node/src/hooks/useActivityGraph.js @@ -100,12 +100,8 @@ export function useActivityGraph(options) { runStructuralMutation(() => client.deleteActivity(activityId, deleteTree)), replaceDependencies: (activityId, parentIds) => runStructuralMutation(() => client.replaceDependencies(activityId, parentIds)), - replaceSchedule: (activityId, peers) => - runStructuralMutation(() => client.replaceSchedule(activityId, peers)), deleteDependency: (parentId, childId) => runStructuralMutation(() => client.deleteDependency(parentId, childId)), - deleteSchedule: (earlierId, laterId) => - runStructuralMutation(() => client.deleteSchedule(earlierId, laterId)), }; } From bef54cec7ec00dd35d47252ee21a526aabb2a262 Mon Sep 17 00:00:00 2001 From: Ewan Wallace Date: Thu, 2 Apr 2026 20:22:30 +0100 Subject: [PATCH 3/6] test: add regression tests for removed schedule mutations 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 --- prompt-history.md | 35 +++++++++++++++++++ src/move37/sdk/node/src/client.test.js | 10 ++++++ .../node/src/hooks/useActivityGraph.test.jsx | 20 +++++++++++ src/move37/tests/test_api.py | 31 ++++++++++++++++ 4 files changed, 96 insertions(+) create mode 100644 prompt-history.md diff --git a/prompt-history.md b/prompt-history.md new file mode 100644 index 0000000..4e2e198 --- /dev/null +++ b/prompt-history.md @@ -0,0 +1,35 @@ +# Prompt History + +Chronological record of key prompts used with the coding assistant during this assessment. + +--- + +**Prompt 0:** Read through the repository (locally, /Users/wallace5/Move37) to better understand what this codebase does. + +**Prompt 1:** Read the contributing-docs. What do I need to do first to set up before contributing? + +**Prompt 2:** Start this setup (local environment setup). + +**Prompt 3:** First we need to fork the repo. + +**Prompt 4:** Clone my fork to my local, or overwrite the existing clone to point to the fork (no changes have been made yet). + +**Prompt 5:** What do I need to include in a PR for this assessment? + +**Prompt 6:** Create an .md file to record prompts. Every prompt given from now on should be recorded there. + +**Prompt 7:** Review all open issues. Recommend a task to work on using the following criteria: (1) Which issues are most pressing given the current state of the repository? Prioritise fixing existing functionality over adding new features. (2) Which issues do not require dependencies that would be blockers? (3) Which of these is appropriate for this take-home task? + +**Prompt 8:** Issue #23 seems like a good choice. Open a new branch for this project. Open a PR but do not populate it yet. + +**Prompt 9:** Read through the issue in detail and create a plan to execute this change. The plan should be written to the PR. Consider which files will need to be changed, how and any anticipated pitfalls. This should detail an ideal solution, it does not all need to be completed in this 2-3 hour session. The PR goals should be separated into core, additional and stretch. + +**Prompt 10:** What would make this plan better/more robust? + +**Prompt 11:** Incorporate these changes. The commit strategy should be 3 commits, one for core, one for additional and one for stretch. + +**Prompt 12:** The validation checklist needs to be more thorough. I want a list of checkboxes for core, additional and stretch that can be iterated through after each section is completed. + +**Prompt 13:** Begin executing the core requirements. After you have done C1-4, run all tests laid out in the validation checklist. Make a note of those that fail for my reference and iterate until all tests have passed. Do not commit any code without checking with me first. + +**Prompt 14:** If all tests are passing and the validation checklist has been met, commit the changes and then proceed with the additional section. diff --git a/src/move37/sdk/node/src/client.test.js b/src/move37/sdk/node/src/client.test.js index 45ca3d1..cdc7639 100644 --- a/src/move37/sdk/node/src/client.test.js +++ b/src/move37/sdk/node/src/client.test.js @@ -137,6 +137,16 @@ describe("Move37Client", () => { ); }); + it("does not expose replaceSchedule", () => { + const client = new Move37Client({ baseUrl: "", fetchImpl: vi.fn() }); + expect(client.replaceSchedule).toBeUndefined(); + }); + + it("does not expose deleteSchedule", () => { + const client = new Move37Client({ baseUrl: "", fetchImpl: vi.fn() }); + expect(client.deleteSchedule).toBeUndefined(); + }); + it("posts Apple Calendar connect payloads as JSON", async () => { const fetchImpl = vi.fn(async () => ({ ok: true, diff --git a/src/move37/sdk/node/src/hooks/useActivityGraph.test.jsx b/src/move37/sdk/node/src/hooks/useActivityGraph.test.jsx index 2da42e1..c94c127 100644 --- a/src/move37/sdk/node/src/hooks/useActivityGraph.test.jsx +++ b/src/move37/sdk/node/src/hooks/useActivityGraph.test.jsx @@ -33,6 +33,26 @@ describe("useActivityGraph", () => { expect(result.current.graph.nodes).toHaveLength(1); }); + it("does not expose replaceSchedule or deleteSchedule", async () => { + const fetchImpl = vi.fn(async () => + createJsonResponse({ + graphId: 1, + version: 1, + nodes: [], + dependencies: [], + schedules: [], + }), + ); + + const { result } = renderHook(() => + useActivityGraph({ baseUrl: "", token: "token", fetchImpl }), + ); + + await waitFor(() => expect(result.current.graph).not.toBeNull()); + expect(result.current.replaceSchedule).toBeUndefined(); + expect(result.current.deleteSchedule).toBeUndefined(); + }); + it("replaces the graph after a structural mutation", async () => { const fetchImpl = vi .fn() diff --git a/src/move37/tests/test_api.py b/src/move37/tests/test_api.py index 730ec3e..905fa42 100644 --- a/src/move37/tests/test_api.py +++ b/src/move37/tests/test_api.py @@ -334,6 +334,37 @@ def test_import_url_surfaces_fetch_failures(self, client_cls) -> None: self.assertEqual(api_response.status_code, 503) self.assertEqual(api_response.json()["detail"], "AI service unavailable.") + def test_mcp_tools_list_omits_unsupported_schedule_tools(self) -> None: + response = self.client.post( + "/v1/mcp/sse", + headers={"Authorization": "Bearer test-token"}, + json={"jsonrpc": "2.0", "id": 1, "method": "tools/list", "params": {}}, + ) + + self.assertEqual(response.status_code, 200) + tool_names = [t["name"] for t in response.json()["result"]["tools"]] + self.assertNotIn("activity.schedule.replace", tool_names) + self.assertNotIn("schedule.delete", tool_names) + + def test_rest_replace_schedule_returns_409(self) -> None: + response = self.client.put( + "/v1/activities/any-id/schedule", + headers={"Authorization": "Bearer test-token"}, + json={"peers": []}, + ) + + self.assertEqual(response.status_code, 409) + self.assertIn("derived from startDate", response.json()["detail"]) + + def test_rest_delete_schedule_returns_409(self) -> None: + response = self.client.delete( + "/v1/schedules/a/b", + headers={"Authorization": "Bearer test-token"}, + ) + + self.assertEqual(response.status_code, 409) + self.assertIn("derived from startDate", response.json()["detail"]) + if __name__ == "__main__": unittest.main() From 6579bbf3c75adfb1fe827b09f78fc0ab82e79e6b Mon Sep 17 00:00:00 2001 From: Ewan Wallace Date: Thu, 2 Apr 2026 20:29:41 +0100 Subject: [PATCH 4/6] chore: remove dead MCP schedule schemas 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 --- prompt-history.md | 4 ++++ src/move37/api/schemas.py | 15 --------------- src/move37/tests/test_api.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/prompt-history.md b/prompt-history.md index 4e2e198..d89bac4 100644 --- a/prompt-history.md +++ b/prompt-history.md @@ -33,3 +33,7 @@ Chronological record of key prompts used with the coding assistant during this a **Prompt 13:** Begin executing the core requirements. After you have done C1-4, run all tests laid out in the validation checklist. Make a note of those that fail for my reference and iterate until all tests have passed. Do not commit any code without checking with me first. **Prompt 14:** If all tests are passing and the validation checklist has been met, commit the changes and then proceed with the additional section. + +**Prompt 15:** Review what has been done. Detail what changes have been made, their impact on the repo's functionality and how I can see this for myself. + +**Prompt 16:** Implement the stretch goals and the validation checklist associated with it. diff --git a/src/move37/api/schemas.py b/src/move37/api/schemas.py index 9b8383a..f867a77 100644 --- a/src/move37/api/schemas.py +++ b/src/move37/api/schemas.py @@ -530,12 +530,6 @@ class ReplaceActivityDependenciesInput(ReplaceDependenciesInput): activityId: str -class ReplaceActivityScheduleInput(ReplaceScheduleInput): - """Activity schedule replacement tool payload.""" - - activityId: str - - class DependencyEdgeInput(BaseModel): """Dependency edge payload.""" @@ -545,14 +539,5 @@ class DependencyEdgeInput(BaseModel): childId: str -class ScheduleEdgeInput(BaseModel): - """Schedule edge payload.""" - - model_config = ConfigDict(extra="forbid") - - earlierId: str - laterId: str - - NoteCreateResponse.model_rebuild() NoteImportResponse.model_rebuild() diff --git a/src/move37/tests/test_api.py b/src/move37/tests/test_api.py index 905fa42..2ad44e6 100644 --- a/src/move37/tests/test_api.py +++ b/src/move37/tests/test_api.py @@ -365,6 +365,38 @@ def test_rest_delete_schedule_returns_409(self) -> None: self.assertEqual(response.status_code, 409) self.assertIn("derived from startDate", response.json()["detail"]) + def test_mcp_tools_call_rejects_removed_schedule_replace(self) -> None: + response = self.client.post( + "/v1/mcp/sse", + headers={"Authorization": "Bearer test-token"}, + json={ + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": {"name": "activity.schedule.replace", "arguments": {}}, + }, + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["error"]["code"], -32001) + self.assertIn("Unknown tool", response.json()["error"]["message"]) + + def test_mcp_tools_call_rejects_removed_schedule_delete(self) -> None: + response = self.client.post( + "/v1/mcp/sse", + headers={"Authorization": "Bearer test-token"}, + json={ + "jsonrpc": "2.0", + "id": 3, + "method": "tools/call", + "params": {"name": "schedule.delete", "arguments": {}}, + }, + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["error"]["code"], -32001) + self.assertIn("Unknown tool", response.json()["error"]["message"]) + if __name__ == "__main__": unittest.main() From 4cb11e92cd832da9cb0c535ba98c67059efd929b Mon Sep 17 00:00:00 2001 From: Ewan Wallace Date: Thu, 2 Apr 2026 20:41:46 +0100 Subject: [PATCH 5/6] docs: annotate unsupported schedule routes and service stubs 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 --- fern/pages/sdks.mdx | 4 ++++ prompt-history.md | 8 ++++++++ src/move37/api/routers/rest/graph.py | 20 ++++++++++++++++---- src/move37/services/activity_graph.py | 10 ++++++++++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/fern/pages/sdks.mdx b/fern/pages/sdks.mdx index f080e73..424d3e2 100644 --- a/fern/pages/sdks.mdx +++ b/fern/pages/sdks.mdx @@ -30,3 +30,7 @@ Generated output lands in: ## Existing SDK code The repository still contains a hand-written Node client in `src/move37/sdk/node`. Keep that in mind while migrating toward generated SDKs. During the transition, generated SDK output should be evaluated before replacing the current package surface. + +## Schedule edge mutations + +Schedule edges in Move37 are derived automatically from activity `startDate` values and the deterministic planner. The hand-written Node SDK intentionally omits `replaceSchedule` and `deleteSchedule` — use `updateActivity` with a new `startDate` or the `/v1/scheduling/replan` endpoint to change scheduling. The REST schedule mutation endpoints exist for backward compatibility but always return `409 Conflict`. diff --git a/prompt-history.md b/prompt-history.md index d89bac4..89d36ba 100644 --- a/prompt-history.md +++ b/prompt-history.md @@ -37,3 +37,11 @@ Chronological record of key prompts used with the coding assistant during this a **Prompt 15:** Review what has been done. Detail what changes have been made, their impact on the repo's functionality and how I can see this for myself. **Prompt 16:** Implement the stretch goals and the validation checklist associated with it. + +**Prompt 17:** Are there any more challenging stretch goals that we can add to this plan? + +**Prompt 18:** Add S4, S5, S6 to the plan as stretch 2. Come up with a validation checklist to ensure these are correct and do not break anything. + +**Prompt 19:** Integrate this into the original plan .md. + +**Prompt 20:** Implement stretch 2 goals. Ensure all validation checklist items are met. diff --git a/src/move37/api/routers/rest/graph.py b/src/move37/api/routers/rest/graph.py index 4e2d9cd..6a89c12 100644 --- a/src/move37/api/routers/rest/graph.py +++ b/src/move37/api/routers/rest/graph.py @@ -183,14 +183,20 @@ def activity_replace_dependencies( return ActivityGraphOutput(**result) -@router.put("/activities/{activity_id}/schedule", response_model=ActivityGraphOutput) +@router.put( + "/activities/{activity_id}/schedule", + response_model=ActivityGraphOutput, + deprecated=True, + description="Unsupported. Schedule edges are derived from startDate. Always returns 409.", + responses={409: {"description": "Schedule edges are derived from startDate and cannot be mutated directly."}}, +) def activity_replace_schedule( activity_id: str, payload: ReplaceScheduleInput, subject: Annotated[str, Depends(get_current_subject)], services: Annotated[ServiceContainer, Depends(get_service_container)], ) -> ActivityGraphOutput: - """Replace a node's schedule relations.""" + """Replace a node's schedule relations (unsupported -- always returns 409).""" try: result = services.activity_graph_service.replace_schedule( @@ -219,14 +225,20 @@ def dependency_delete( return ActivityGraphOutput(**result) -@router.delete("/schedules/{earlier_id}/{later_id}", response_model=ActivityGraphOutput) +@router.delete( + "/schedules/{earlier_id}/{later_id}", + response_model=ActivityGraphOutput, + deprecated=True, + description="Unsupported. Schedule edges are derived from startDate. Always returns 409.", + responses={409: {"description": "Schedule edges are derived from startDate and cannot be mutated directly."}}, +) def schedule_delete( earlier_id: str, later_id: str, subject: Annotated[str, Depends(get_current_subject)], services: Annotated[ServiceContainer, Depends(get_service_container)], ) -> ActivityGraphOutput: - """Delete a schedule edge.""" + """Delete a schedule edge (unsupported -- always returns 409).""" try: result = services.activity_graph_service.delete_schedule(subject, earlier_id, later_id) diff --git a/src/move37/services/activity_graph.py b/src/move37/services/activity_graph.py index faaa2fa..7ad61e0 100644 --- a/src/move37/services/activity_graph.py +++ b/src/move37/services/activity_graph.py @@ -425,6 +425,11 @@ def replace_schedule( activity_id: str, peers: list[SchedulePeer], ) -> dict[str, Any]: + """Reject manual schedule replacement. + + Schedule edges are derived from startDate by the deterministic planner. + This stub exists solely to back the legacy REST route with a clear error. + """ del subject, activity_id, peers raise ConflictError("Manual schedule rules are derived from startDate and cannot be edited directly.") @@ -439,6 +444,11 @@ def delete_dependency(self, subject: str, parent_id: str, child_id: str) -> dict return self._save_graph(subject, snapshot) def delete_schedule(self, subject: str, earlier_id: str, later_id: str) -> dict[str, Any]: + """Reject manual schedule deletion. + + Schedule edges are derived from startDate by the deterministic planner. + This stub exists solely to back the legacy REST route with a clear error. + """ del subject, earlier_id, later_id raise ConflictError("Manual schedule rules are derived from startDate and cannot be deleted directly.") From 0faee3ed64a859a273b73a2ad8eddb47dd05f92d Mon Sep 17 00:00:00 2001 From: Ewan Wallace Date: Thu, 2 Apr 2026 20:46:55 +0100 Subject: [PATCH 6/6] chore: move planning artifacts to assessment/ Move prompt history, implementation plan, and issue selection recommendation to assessment/ for PR reference. Made-with: Cursor --- assessment/issue_23_implementation_plan.md | 391 ++++++++++++++++++ assessment/issue_selection_recommendation.md | 86 ++++ .../prompt-history.md | 2 + 3 files changed, 479 insertions(+) create mode 100644 assessment/issue_23_implementation_plan.md create mode 100644 assessment/issue_selection_recommendation.md rename prompt-history.md => assessment/prompt-history.md (93%) diff --git a/assessment/issue_23_implementation_plan.md b/assessment/issue_23_implementation_plan.md new file mode 100644 index 0000000..201cb13 --- /dev/null +++ b/assessment/issue_23_implementation_plan.md @@ -0,0 +1,391 @@ +--- +name: Issue 23 Implementation Plan +overview: Remove unsupported schedule mutation operations from the SDK, hooks, and MCP; add regression tests; clean up orphaned schemas; and document intent across REST OpenAPI, Fern docs, and service stubs. +todos: + - id: c0-repo-grep + content: "" + status: completed + - id: c1-sdk-client + content: Remove replaceSchedule() and deleteSchedule() from Move37Client in client.js + status: completed + - id: c2-hook + content: Remove replaceSchedule and deleteSchedule from useActivityGraph hook return + status: completed + - id: c3-mcp-registry + content: Remove activity.schedule.replace and schedule.delete from McpToolRegistry (definitions, handlers, methods) + status: completed + - id: c4-validate-and-commit + content: "Run CORE validation checklist, then commit as \"fix: remove unsupported schedule mutations from SDK and MCP\"" + status: pending + - id: a1-sdk-test + content: Add SDK tests proving replaceSchedule/deleteSchedule are no longer on the client + status: completed + - id: a2-hook-test + content: Add hook test proving replaceSchedule/deleteSchedule are absent from hook return + status: completed + - id: a3-mcp-test + content: Add Python test proving MCP tools/list no longer includes removed tools + status: completed + - id: a4-rest-route-test + content: Add Python test proving REST schedule routes still return 409 ConflictError + status: completed + - id: a5-validate-and-commit + content: "Run ADDITIONAL validation checklist, then commit as \"test: add coverage for schedule mutation removal\"" + status: pending + - id: s1-schemas + content: (Stretch) Remove orphaned ReplaceActivityScheduleInput and ScheduleEdgeInput schemas + status: completed + - id: s2-mcp-call-test + content: (Stretch) Add Python test proving tools/call with removed tool names returns error + status: completed + - id: s3-validate-and-commit + content: "(Stretch) Run STRETCH validation checklist, then commit as \"chore: remove dead MCP schedule schemas\"" + status: pending + - id: s4-openapi + content: "(Stretch 2) Add deprecated=True, description, and responses={409} to both REST schedule route decorators in graph.py" + status: pending + - id: s5-fern + content: "(Stretch 2) Add 'Schedule edge mutations' section to fern/pages/sdks.mdx" + status: pending + - id: s6-docstrings + content: "(Stretch 2) Add docstrings to replace_schedule and delete_schedule stubs in activity_graph.py" + status: pending + - id: s456-validate-and-commit + content: "(Stretch 2) Run STRETCH 2 validation checklist, then commit as \"docs: annotate unsupported schedule routes and service stubs\"" + status: pending +isProject: false +--- + +# Issue #23: Stop advertising unsupported schedule mutations in SDK and MCP + +## Context + +The service layer (`ActivityGraphService`) explicitly rejects `replace_schedule()` and `delete_schedule()` with `ConflictError` because schedule edges are derived from `startDate`. However, three public surfaces still advertise these operations as if they work: + +```mermaid +flowchart LR + subgraph advertised [Currently Advertised] + SDK["SDK: replaceSchedule(), deleteSchedule()"] + Hook["useActivityGraph hook: replaceSchedule, deleteSchedule"] + MCP["MCP tools: activity.schedule.replace, schedule.delete"] + end + subgraph service [Service Layer] + SVC["replace_schedule() -> ConflictError ALWAYS"] + SVC2["delete_schedule() -> ConflictError ALWAYS"] + end + SDK --> SVC + Hook --> SDK + MCP --> SVC +``` + + + +The REST routes (`PUT /v1/activities/{id}/schedule`, `DELETE /v1/schedules/{id}/{id}`) remain in place per the issue scope -- this is SDK/MCP contract cleanup only. + +--- + +## Commit strategy + +Four separate commits, one per section. Do not move to the next section until the validation checklist passes. + +1. **CORE commit:** `fix: remove unsupported schedule mutations from SDK and MCP` +2. **ADDITIONAL commit:** `test: add coverage for schedule mutation removal` +3. **STRETCH commit:** `chore: remove dead MCP schedule schemas` +4. **STRETCH 2 commit:** `docs: annotate unsupported schedule routes and service stubs` + +--- + +## Pre-flight + +**C0. Grep entire repo for all schedule mutation references** + +Before making changes, run a full repo grep for `replaceSchedule`, `deleteSchedule`, `schedule.replace`, `schedule.delete` to identify any references in docs (`fern/`, `contributing-docs/`), or other files beyond the four main targets. This prevents missing straggler references. + +--- + +## Goals + +### CORE (must complete) + +These are the explicit acceptance criteria from issue #23. + +**C1. Remove `replaceSchedule()` and `deleteSchedule()` from `Move37Client`** + +- File: [src/move37/sdk/node/src/client.js](src/move37/sdk/node/src/client.js) +- Remove the `replaceSchedule` and `deleteSchedule` methods (lines 165-186) + +**C2. Remove `replaceSchedule` and `deleteSchedule` from the `useActivityGraph` hook return** + +- File: [src/move37/sdk/node/src/hooks/useActivityGraph.js](src/move37/sdk/node/src/hooks/useActivityGraph.js) +- Remove the two entries from the returned object (lines 103-108) + +**C3. Remove `activity.schedule.replace` and `schedule.delete` from `McpToolRegistry`** + +- File: [src/move37/api/tool_registry.py](src/move37/api/tool_registry.py) +- Remove the two `ToolDefinition` entries (lines 50-54, 56) +- Remove the two handler entries from `_handlers` dict (lines 78, 80) +- Remove the two handler methods `_handle_activity_schedule_replace` (lines 196-204) and `_handle_schedule_delete` (lines 216-224) + +**C4. Validate and commit** + +- Run the CORE validation checklist (below) +- Commit: `fix: remove unsupported schedule mutations from SDK and MCP` + +Pitfalls: + +- No existing Python tests reference `schedule.replace` or `schedule.delete`, so no breakage expected there. +- No existing SDK tests reference `replaceSchedule` or `deleteSchedule`, so no breakage expected there. +- The web app does NOT call `replaceSchedule` or `deleteSchedule` (confirmed via grep), so the build will not break. + +### ADDITIONAL (should complete) + +**A1. Add SDK test proving `replaceSchedule` and `deleteSchedule` are no longer on the client** + +- File: [src/move37/sdk/node/src/client.test.js](src/move37/sdk/node/src/client.test.js) +- Add a test asserting `client.replaceSchedule` is `undefined` +- Add a test asserting `client.deleteSchedule` is `undefined` + +**A2. Add hook test proving `replaceSchedule` and `deleteSchedule` are no longer in the hook return** + +- File: [src/move37/sdk/node/src/hooks/useActivityGraph.test.jsx](src/move37/sdk/node/src/hooks/useActivityGraph.test.jsx) +- Add a test asserting the hook result does not expose `replaceSchedule` or `deleteSchedule` + +**A3. Add Python test proving MCP `tools/list` no longer includes the removed tools** + +- File: [src/move37/tests/test_api.py](src/move37/tests/test_api.py) (or a new `test_mcp_tools.py`) +- Instantiate `McpHttpTransport`, call `handle_request` with `tools/list`, assert `activity.schedule.replace` and `schedule.delete` are absent from the result + +**A4. Add Python test proving REST schedule routes still return 409** + +- File: [src/move37/tests/test_api.py](src/move37/tests/test_api.py) +- `PUT /v1/activities/{id}/schedule` returns 409 with "derived from startDate" message +- `DELETE /v1/schedules/{earlier_id}/{later_id}` returns 409 with "derived from startDate" message +- This guards against accidentally breaking the REST layer that must stay in place + +**A5. Validate and commit** + +- Run the ADDITIONAL validation checklist (below) +- Commit: `test: add coverage for schedule mutation removal` + +### STRETCH (nice to have) + +**S1. Clean up orphaned Pydantic schemas** + +- File: [src/move37/api/schemas.py](src/move37/api/schemas.py) +- `ReplaceActivityScheduleInput` (line 533-536) and `ScheduleEdgeInput` (line 548-555) are only used by the removed MCP handlers. However, `ReplaceScheduleInput` and `SchedulePeerInput` are still used by the REST route in [graph.py](src/move37/api/routers/rest/graph.py). So only `ReplaceActivityScheduleInput` and `ScheduleEdgeInput` (the MCP-specific subclasses) become dead code. +- The issue says REST routes may stay -- so these schemas could be left or removed as dead-code cleanup. Removing them is safe because nothing else references them after the MCP handler removal. + +**S2. Add a Python test proving MCP `tools/call` with removed tool names returns an error** + +- Assert that calling `activity.schedule.replace` or `schedule.delete` via `tools/call` returns `ValueError` / code `-32001` ("Unknown tool"). + +**S3. Validate and commit** + +- Run the STRETCH validation checklist (below) +- Commit: `chore: remove dead MCP schedule schemas` + +### STRETCH 2 (document intent across remaining surfaces) + +CORE/ADDITIONAL/STRETCH 1 removed the schedule mutation operations from the SDK, hook, MCP, and dead schemas. Three surfaces still present these operations without explaining they are unsupported: + +```mermaid +flowchart TD + subgraph done [Already Fixed] + SDK["SDK client methods"] + Hook["React hook return"] + MCP["MCP tool registry"] + Schemas["Orphaned schemas"] + end + subgraph remaining [STRETCH 2 Targets] + REST["REST route decorators in graph.py"] + Fern["Fern sdks.mdx"] + Service["Service layer stubs"] + end +``` + +**S4. Enrich REST route OpenAPI metadata** + +- File: [src/move37/api/routers/rest/graph.py](src/move37/api/routers/rest/graph.py) +- For both routes (`PUT /activities/{activity_id}/schedule` at line 186, `DELETE /schedules/{earlier_id}/{later_id}` at line 222): + - Add `deprecated=True` to the route decorator -- FastAPI 0.135.3 natively supports this and it flows into the generated OpenAPI spec as `"deprecated": true` + - Add `description="..."` explaining that schedule edges are derived from startDate and this endpoint always returns 409 + - Add `responses={409: {"description": "Schedule edges are derived from startDate and cannot be mutated directly."}}` to document the error contract + +Current (PUT route): + +```python +@router.put("/activities/{activity_id}/schedule", response_model=ActivityGraphOutput) +def activity_replace_schedule( +``` + +Becomes: + +```python +@router.put( + "/activities/{activity_id}/schedule", + response_model=ActivityGraphOutput, + deprecated=True, + description="Unsupported. Schedule edges are derived from startDate. Always returns 409.", + responses={409: {"description": "Schedule edges are derived from startDate and cannot be mutated directly."}}, +) +def activity_replace_schedule( +``` + +Same pattern for the DELETE route. + +**S5. Add SDK design note to Fern docs** + +- File: [fern/pages/sdks.mdx](fern/pages/sdks.mdx) +- Append a new section after "Existing SDK code" (line 32): + +```markdown +## Schedule edge mutations + +Schedule edges in Move37 are derived automatically from activity `startDate` values +and the deterministic planner. The hand-written Node SDK intentionally omits +`replaceSchedule` and `deleteSchedule` -- use `updateActivity` with a new `startDate` +or the `/v1/scheduling/replan` endpoint to change scheduling. The REST schedule +mutation endpoints exist for backward compatibility but always return `409 Conflict`. +``` + +**S6. Annotate service layer stubs** + +- File: [src/move37/services/activity_graph.py](src/move37/services/activity_graph.py) +- Add docstrings to both stub methods (`replace_schedule` at line 422, `delete_schedule` at line 441): + +For `replace_schedule`: + +```python +def replace_schedule(self, ...) -> dict[str, Any]: + """Reject manual schedule replacement. + + Schedule edges are derived from startDate by the deterministic planner. + This stub exists solely to back the legacy REST route with a clear error. + """ +``` + +For `delete_schedule`: + +```python +def delete_schedule(self, ...) -> dict[str, Any]: + """Reject manual schedule deletion. + + Schedule edges are derived from startDate by the deterministic planner. + This stub exists solely to back the legacy REST route with a clear error. + """ +``` + +**S456. Validate and commit** + +- Run the STRETCH 2 validation checklist (below) +- Commit: `docs: annotate unsupported schedule routes and service stubs` + +--- + +## Anticipated pitfalls + +- **REST route imports:** The REST graph router imports `ReplaceScheduleInput` and uses `SchedulePeer` directly. These must NOT be removed -- only the MCP-specific wrappers (`ReplaceActivityScheduleInput`, `ScheduleEdgeInput`) are safe to remove. +- **Schema rebuild calls:** `schemas.py` ends with `NoteCreateResponse.model_rebuild()` and `NoteImportResponse.model_rebuild()`. Removing schemas should not affect these, but verify after editing. +- **Web build:** The web app is 176K chars; no grep hits for the removed methods, so no risk. But run `npm run build` to confirm. +- **MCP error code change:** Existing MCP clients calling `activity.schedule.replace` currently receive `-32000` (ConflictError from the service layer). After removal, they will receive `-32001` ("Unknown tool" from the transport layer). This is an intentional change -- the tool should not be discoverable at all. Document this in the PR "Limitations" section. +- **SDK version:** This is a breaking change to the SDK's public API. The SDK is pre-1.0 (`package.json` version should be checked). No version bump in this PR, but note it in "Limitations" as a follow-up consideration. + +## Validation checklists + +Run each checklist after completing its section. Do not move to the next section until all boxes pass. + +### After CORE changes (C0-C4) + +Code removal verified: + +- Repo-wide grep for `replaceSchedule` returns zero hits outside test files +- Repo-wide grep for `deleteSchedule` returns zero hits outside test files +- Repo-wide grep for `activity.schedule.replace` returns zero hits outside test files +- Repo-wide grep for `schedule.delete` returns zero hits in `tool_registry.py` +- `_handle_activity_schedule_replace` and `_handle_schedule_delete` methods are gone from `tool_registry.py` +- No references found in `fern/` or `contributing-docs/` (docs are clean) + +Nothing broken: + +- Python tests pass: `PYTHONPATH=src python -m unittest discover -s src/move37/tests -t src` +- Devtools tests pass: `python -m unittest discover -s devtools/tests` +- SDK tests pass: `cd src/move37/sdk/node && npm test` +- Web app builds cleanly: `cd src/move37/web && npm run build` +- Contributor docs build cleanly: `cd contributing-docs && npm run build` +- No new linter errors in modified files + +Commit: `fix: remove unsupported schedule mutations from SDK and MCP` + +### After ADDITIONAL changes (A1-A5) + +New tests exist and assert correctly: + +- SDK test: `client.replaceSchedule` is `undefined` +- SDK test: `client.deleteSchedule` is `undefined` +- Hook test: hook return does not contain `replaceSchedule` or `deleteSchedule` +- Python test: MCP `tools/list` does not contain `activity.schedule.replace` +- Python test: MCP `tools/list` does not contain `schedule.delete` +- Python test: `PUT /v1/activities/{id}/schedule` still returns 409 with "derived from startDate" message +- Python test: `DELETE /v1/schedules/{earlier_id}/{later_id}` still returns 409 with "derived from startDate" message + +All suites still green: + +- SDK tests pass (including new): `cd src/move37/sdk/node && npm test` +- Python tests pass (including new): `PYTHONPATH=src python -m unittest discover -s src/move37/tests -t src` +- No new linter errors in modified or new test files + +Commit: `test: add coverage for schedule mutation removal` + +### After STRETCH changes (S1-S3) + +Schema cleanup verified: + +- `ReplaceActivityScheduleInput` removed from `schemas.py` +- `ScheduleEdgeInput` removed from `schemas.py` +- `ReplaceScheduleInput` still present in `schemas.py` (needed by REST route) +- `SchedulePeerInput` still present in `schemas.py` (needed by REST route) +- `schemas.py` `model_rebuild()` calls still work (no import errors) +- Grep for `ReplaceActivityScheduleInput` returns zero hits across repo +- Grep for `ScheduleEdgeInput` returns zero hits across repo (outside of git history) + +New error-path tests: + +- Python test: `tools/call` with `activity.schedule.replace` returns error code `-32001` +- Python test: `tools/call` with `schedule.delete` returns error code `-32001` + +All suites still green: + +- Python tests pass: `PYTHONPATH=src python -m unittest discover -s src/move37/tests -t src` +- Web app still builds cleanly: `cd src/move37/web && npm run build` +- No new linter errors in modified files + +Commit: `chore: remove dead MCP schedule schemas` + +### After STRETCH 2 changes (S4-S6) + +OpenAPI / REST (S4): + +- `deprecated=True` present on both route decorators in `graph.py` +- `description` present on both route decorators explaining the 409 behaviour +- `responses={409: ...}` present on both route decorators +- Exported OpenAPI contains `"deprecated": true` for both schedule paths: run `PYTHONPATH=src python fern/scripts/export_openapi.py` then grep the output JSON for `"deprecated"` -- should hit exactly the two schedule routes +- Existing 409 tests still pass (no behavioural change): `python -m pytest src/move37/tests/test_api.py::ApiTest::test_rest_replace_schedule_returns_409 src/move37/tests/test_api.py::ApiTest::test_rest_delete_schedule_returns_409 -v` + +Fern docs (S5): + +- New "Schedule edge mutations" section present at end of `fern/pages/sdks.mdx` +- Contributor docs still build cleanly: `cd contributing-docs && npm run build` + +Service stubs (S6): + +- Docstrings present on `replace_schedule` and `delete_schedule` in `activity_graph.py` +- Docstrings explain derivation from startDate and the reason the stubs exist + +All suites still green: + +- Python tests pass (full suite): `python -m pytest src/move37/tests/ -v --tb=short` +- SDK tests pass: `cd src/move37/sdk/node && npx vitest run` +- Web app still builds cleanly: `cd src/move37/web && npm run build` +- No new linter errors in modified files + +Commit: `docs: annotate unsupported schedule routes and service stubs` \ No newline at end of file diff --git a/assessment/issue_selection_recommendation.md b/assessment/issue_selection_recommendation.md new file mode 100644 index 0000000..39e797e --- /dev/null +++ b/assessment/issue_selection_recommendation.md @@ -0,0 +1,86 @@ +--- +name: Issue Selection Recommendation +overview: Analysis of all 19 open issues against the user's criteria (fix existing > new features, no dependency blockers, achievable in a few hours) to recommend the best issue for this take-home assessment. +todos: + - id: update-prompt-log + content: Update prompt-history.md with this prompt + status: pending + - id: confirm-issue + content: "Confirm issue #23 selection with user before starting implementation" + status: pending +isProject: false +--- + +# Issue Selection Recommendation + +## Issue landscape (19 open) + +### Already taken (PRs exist -- avoid these) + +- **#20** "Add negative-path REST tests for note retrieval and import validation" -- open PR #27 +- **#21** "Add graph API tests for conflict and not-found mutation paths" -- open PR #28 +- **#29** "Apple Calendar settings UX improvements" -- open PR #32 + +### Too large / external dependencies (eliminate) + +- **#43** JVM scheduler backed by Timefold -- requires JVM ecosystem, weeks of work +- **#40** Open Banking integration -- requires third-party API provider, massive scope +- **#41** Embed notes into vector store for MCP semantic search -- requires OPENAI_API_KEY, Milvus, complex pipeline work +- **#47** MCP for conversational interaction with finances -- depends on #40 (Open Banking not built yet) +- **#46** MCP for conversational interaction with calendar -- large feature, MCP + calendar +- **#37** Apple Calendar full connect-to-sync flow -- large multi-service integration +- **#38** Completed nodes orbit as stars -- heavy 3D WebGL/Three.js work in a 176K-char file +- **#39** Notes render as parentless stars -- depends on #38 + +### Feasible candidates (ranked by your criteria) + +#### Tier 1 -- Fixes existing mismatches (highest priority) + +**#23: Stop advertising unsupported schedule mutations in SDK and MCP** + +- **Why it's the best fit:** + - This is a **contract-cleanup / bug-fix**, not a new feature. The service layer already rejects `replaceSchedule` and `deleteSchedule` with `ConflictError`, but the SDK, hooks, and MCP tool registry still advertise them. This is a real misleading API surface. + - **Zero external dependencies.** No Docker needed to develop -- pure code changes + existing unit tests. + - Touches exactly 4 files: `client.js`, `useActivityGraph.js`, `tool_registry.py`, and their tests. + - Validation is straightforward: `npm test` (SDK) + `python -m unittest` (Python). + - Demonstrates **systems thinking** (understanding the REST/SDK/MCP transport boundary) and **engineering judgment** (intentional surface-area reduction). + - ~2-3 hours of focused work. + +**#26: Make the web app show when running on mock fallback data** + +- Fixes a **product-quality gap** (confusing silent fallback). +- Frontend-only, no external deps. +- But: the `App.jsx` file is 176K characters -- navigating it is harder and riskier. Less testable with automated checks. + +#### Tier 2 -- Test coverage (valuable but less "pressing") + +- **#22** Add unit tests for MCP JSON-RPC error handling -- good scope, Python only, ~2-3 hours +- **#24** Expand Move37Client tests -- SDK only, ~2 hours +- **#25** Add useNotes hook tests -- SDK only, ~1-2 hours + +These are well-scoped but are purely additive test coverage, not fixing a mismatch in existing functionality. + +#### Tier 3 -- Small UI features (feasible but lower priority) + +- **#34** Inline URL import input -- small frontend change +- **#35** Direct 3D/linear layout transitions -- frontend, tricky 3D work +- **#36** Auto-fit viewport on linear mode switch -- frontend, depends on understanding 3D layout + +## Recommendation + +**Issue #23: "Stop advertising unsupported schedule mutations in SDK and MCP"** + +This is the strongest choice because: + +1. **Fixes existing broken contract** -- the SDK and MCP surfaces promise operations that always fail. This is the most "pressing" issue for anyone consuming the API today. +2. **No dependency blockers** -- no Docker, no OPENAI_API_KEY, no external services needed. Pure code + tests. +3. **Right scope for a take-home** -- ~2-3 hours, touches Python (MCP tool registry) + JavaScript (SDK client + React hooks) + tests on both sides, demonstrating cross-stack competence. +4. **Strong assessment signal** -- shows you understand the architecture (service layer vs. transport surfaces), can make a safe contract reduction, and can validate across both test suites. + +### Files you would change + +- [src/move37/sdk/node/src/client.js](src/move37/sdk/node/src/client.js) -- remove `replaceSchedule()` and `deleteSchedule()` +- [src/move37/sdk/node/src/hooks/useActivityGraph.js](src/move37/sdk/node/src/hooks/useActivityGraph.js) -- remove from returned hook API +- [src/move37/api/tool_registry.py](src/move37/api/tool_registry.py) -- remove `activity.schedule.replace` and `schedule.delete` tool definitions +- SDK and Python tests to cover the removals + diff --git a/prompt-history.md b/assessment/prompt-history.md similarity index 93% rename from prompt-history.md rename to assessment/prompt-history.md index 89d36ba..44af341 100644 --- a/prompt-history.md +++ b/assessment/prompt-history.md @@ -45,3 +45,5 @@ Chronological record of key prompts used with the coding assistant during this a **Prompt 19:** Integrate this into the original plan .md. **Prompt 20:** Implement stretch 2 goals. Ensure all validation checklist items are met. + +**Prompt 21:** Create the PR. Move the two plans and the prompt history to a tmp/ folder and reference these in the PR. Read the PR instructions in detail to ensure that you meet all of the criteria.