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/assessment/prompt-history.md b/assessment/prompt-history.md new file mode 100644 index 0000000..44af341 --- /dev/null +++ b/assessment/prompt-history.md @@ -0,0 +1,49 @@ +# 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. + +**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. + +**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. 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/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/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/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/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.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)), }; } 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/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.") diff --git a/src/move37/tests/test_api.py b/src/move37/tests/test_api.py index 730ec3e..2ad44e6 100644 --- a/src/move37/tests/test_api.py +++ b/src/move37/tests/test_api.py @@ -334,6 +334,69 @@ 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"]) + + 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()