feat: DELETE /memory/{id} + PATCH /memory/{id} by jphein · Pull Request #17 · rboarescu/palace-daemon

jphein · 2026-04-30T17:02:48Z

Adds two small REST endpoints that wrap mempalace_delete_drawer and mempalace_update_drawer. Both tools have been in mempalace since 3.x — this just exposes them over HTTP so consumers can curate write-side outputs without going through MCP.

Shape

DELETE /memory/{drawer_id}                         -> mempalace_delete_drawer
PATCH  /memory/{drawer_id}  {content?, wing?, room?}  -> mempalace_update_drawer

PATCH body keys are all optional; only supplied fields are forwarded. So PATCH /memory/abc {"content": "..."} updates content and leaves wing/room alone.
Both go through the write semaphore on the same code path as /silent-save (so they coordinate with /repair mode=rebuild and don't race the rebuild swap).
Auth via X-Api-Key header, same as every other endpoint.

Why

A small reflect-memories curation UI on my side needed to drop bad drawers and fix typos in good ones. Doable through MCP, but adding HTTP made the frontend much simpler. Posting upstream because it generalises — any UI that wants to show a list and let the user fix entries probably wants the same shape.

What's not in this PR

verify-routes.sh probes for DELETE / PATCH are held back because that file is being added by chore(scripts): add verify-routes.sh smoke test #9. Happy to fold them into chore(scripts): add verify-routes.sh smoke test #9 or send a follow-up after chore(scripts): add verify-routes.sh smoke test #9 lands — whatever's simpler for you.
No version bump, no README changes — kept tight.

29 lines of main.py, no other files touched. Tested in production on the canonical 151K-drawer palace since 2026-04-26.

Thanks for considering it!

Exposes mempalace_delete_drawer and mempalace_update_drawer over HTTP so write-side consumers don't need MCP just to fix a typo or drop a bad drawer. Both tools have existed in mempalace since 3.x; this only adds the REST surface. DELETE /memory/{drawer_id} -> mempalace_delete_drawer PATCH /memory/{drawer_id} {content?, wing?, room?} -> mempalace_update_drawer PATCH treats body keys as optional patches — only the supplied fields are forwarded to mempalace_update_drawer. verify-routes.sh probes held back because that file is added by rboarescu#9. Happy to send a follow-up once rboarescu#9 lands.

Copilot

Pull request overview

Adds two new write-side REST endpoints to palace-daemon that expose existing MemPalace drawer curation tools over HTTP, enabling clients to delete or patch existing drawers without going through MCP.

Changes:

Add DELETE /memory/{drawer_id} to wrap mempalace_delete_drawer.
Add PATCH /memory/{drawer_id} to wrap mempalace_update_drawer, forwarding only provided fields (content, wing, room).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-30T17:05:43Z

+async def update_memory(drawer_id: str, request: Request, x_api_key: str | None = Header(default=None)):
+    """Update a drawer's content/wing/room. Wraps mempalace_update_drawer."""
+    _check_auth(x_api_key)
+    body = await request.json()


await request.json() can raise on invalid/empty bodies, and this handler also assumes the parsed JSON is an object (dict). With malformed JSON, empty bodies, or non-object JSON (e.g., a string/array), this will currently bubble up as a 500 via JSONDecodeError/TypeError. Align with /mcp//silent-save by catching parse/type errors and returning a 400, and validate body is a dict before reading keys.

Suggested change

body = await request.json()

try:

body = await request.json()

except json.JSONDecodeError as exc:

raise HTTPException(status_code=400, detail="Request body must be valid JSON.") from exc

if not isinstance(body, dict):

raise HTTPException(status_code=400, detail="Request body must be a JSON object.")

Already addressed at main.py:482-487: try: body = await request.json() except json.JSONDecodeError as exc: raise HTTPException(status_code=400, detail="Request body must be valid JSON.") from exc; if not isinstance(body, dict): raise HTTPException(status_code=400, detail="Request body must be a JSON object."). Both "malformed JSON" and "non-object JSON" return 400 with explanatory messages now, instead of bubbling up as a 500 from .get() raised on something that isn't a dict. Just pushed 98dd7bc to apply the same guards to POST /memory for consistency.

Copilot · 2026-04-30T17:05:44Z

+    args: dict = {"drawer_id": drawer_id}
+    if "content" in body: args["content"] = body["content"]
+    if "wing" in body: args["wing"] = body["wing"]
+    if "room" in body: args["room"] = body["room"]
+    result = await _call({
+        "jsonrpc": "2.0", "id": 1,
+        "method": "tools/call",
+        "params": {"name": "mempalace_update_drawer", "arguments": args},
+    })


This PATCH endpoint will call mempalace_update_drawer even when none of content/wing/room are provided (e.g., {}), resulting in an ambiguous no-op request and potentially a tool-level error. Consider validating that at least one updatable field is present and returning a 400 with a clear message otherwise.

Done at main.py:493-497: if len(args) == 1: raise HTTPException(status_code=400, detail="PATCH /memory/{id} requires at least one of: content, wing, room."). The args dict starts with just {"drawer_id": ...}, so when no field-bearing keys are added it stays at length 1 and trips the guard. An empty body or a body with no recognized fields gets a 400 with the explicit list of acceptable keys, not an ambiguous no-op forwarded to mempalace.

…nding queue; trim patch Filed four upstream PRs on 2026-04-30: - rboarescu#15 feat: GET /viz status dashboard (stacks on rboarescu#13) - rboarescu#16 feat: GET /list — query-free metadata browse - rboarescu#17 feat: DELETE /memory + PATCH /memory - rboarescu#18 feat(lifespan): auto-migrate Stop-hook checkpoints on startup Also rebased PR rboarescu#13 onto upstream/main to clear a CHANGELOG conflict left by upstream's b4aee82 (patch sync) — state went CONFLICTING -> MERGEABLE / CLEAN. README: - Open upstream PRs table: four new rows (rboarescu#15-rboarescu#18) plus a 2026-04-30 note covering today's rebase + new PRs in one breath. - Pending PRs queue: now empty. Replaced the four stale rows (event-log-frame and graph-endpoint were already in flight via rboarescu#11 and rboarescu#13; mempal-fast.py was already upstream via the merged PR #4 omnibus; /viz is now PR rboarescu#15) with a brief empty-state note. CLAUDE.md: - Patch description trimmed to reflect that the hnsw:num_threads enforcement landed upstream via _pin_hnsw_threads(); only the log + retry-once slice remains. patches/mcp_server_get_collection.patch: - Regenerated against current mempalace develop. The patch is now just the "log exception + retry once on cache failure" change. Filed upstream as MemPalace/mempalace#1286; once that merges this patch retires. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

README: - Intro paragraph: appended GET /list (PR rboarescu#16), DELETE/PATCH /memory (PR rboarescu#17), and lifespan auto-migrate (PR rboarescu#18) to "what this fork adds" since they're already on fork main even though the PRs are awaiting review upstream. - Links bar: added docs/typescript-port-plan.md. - Added a "Cross-repo coordination" subsection under "Recently landed in upstream" — calls out the local patches/ directory and the in-flight MemPalace/mempalace#1286 that retires it. Also references #1142 (RELEASING.md) for completeness. - Requirements: dropped the stale "kind= searcher filter" justification for recommending the fork (kind= was retired in 1.7.1); replaced with daemon-strict hook mode + warnings/sqlite-fallback search path as the actual current reasons. Added the patches/ re-apply step. - API table: added /list, DELETE /memory/{id}, PATCH /memory/{id} rows so the table reflects what the fork main actually exposes. - Sources: cross-repo PR note for #1286. CHANGELOG: - Added [Unreleased] section: patch trim (Maintenance), TS port plan doc, hook-routing-fix.md status header, and a Docs entry covering today's README updates + PR rboarescu#13 rebase context. docs/hook-routing-fix.md: - Added a Status: SHIPPED header pointing at 62425e3 (2026-04-24 clients/hook.py introduction) and clarifying mempal-fast.py is the simpler successor — same shape as docs/graph-endpoint.md's status header. No daemon behaviour changes.

Two issues: 1. await request.json() bubbled up as 500 on invalid/empty bodies and non-object JSON. Catch JSONDecodeError → 400 with a clear message, and validate the parsed value is a dict before reading keys. (Same pattern /silent-save and /mcp use.) 2. PATCH with no patch fields (e.g., {}) was forwarded to mempalace_update_drawer with only drawer_id, producing an ambiguous no-op or tool-level error. Reject with 400 and an explicit message listing the valid patch keys. DELETE handler unchanged — no body parsing, nothing to harden. Docstring expanded to make the body contract explicit.

…boarescu#18 + #1286 to main Same shape as 152e428 — production at disks runs fork main, so review amendments to the in-flight upstream PRs need to ship here too. ## main.py — /list docstring (from PR rboarescu#16) The previous docstring claimed both "sorted by recency" and "ordered by mempalace's natural metadata-table order", which contradicted itself. Replaced with an accurate description: ordering is whatever mempalace_list_drawers returns (sqlite metadata-table order, which approximates insertion order but isn't strictly chronological). Pagination contract called out explicitly. ## main.py — PATCH /memory body validation (from PR rboarescu#17) - await request.json() now wraps in try/except for JSONDecodeError → 400 with a clear message (was bubbling as 500). - Validate parsed body is a dict before reading keys. - Reject empty patches (no content/wing/room) with 400 listing the valid keys, instead of forwarding {drawer_id: ...} alone to mempalace_update_drawer where it's an ambiguous no-op. DELETE handler unchanged — no body parsing, nothing to harden. ## main.py — Lifespan auto-migrate ImportError narrowing (from PR rboarescu#18) Previous `except ImportError:` swallowed transitive dep failures inside mempalace.migrate (a real bug worth surfacing) as if they were "feature unavailable" (expected). Distinguish via `getattr(e, "name", None) == "mempalace.migrate"` — module genuinely absent → debug log, anything else → warning log so it's visible. Comment that pointed at `mempalace docs/superpowers/specs/2026-04-25- checkpoint-collection-split.md` (a path that doesn't exist in this repo, lives only in the jphein/mempalace fork's superpowers tree) replaced with a public-facing reference to `mempalace repair --mode reorganize` — the operator-driven equivalent that's actually discoverable. ## patches/mcp_server_get_collection.patch (from MemPalace/mempalace #1286) Regenerated against current upstream develop with the Copilot review fix folded in: logger.error(..., str(e)) → logger.exception(...). Now the traceback lands in the daemon log alongside the message instead of being dropped. Patch behaviour is otherwise identical. The upstream PR also gained TestGetCollectionRetry — three monkeypatch tests that pin the retry semantics. Those tests live in mempalace, not here; they'll arrive on this side automatically once #1286 merges and the patch retires. All four fixes are amendments on the corresponding upstream PR branches (force-pushed earlier today). No behaviour change for the healthy path; the changes harden failure modes that were caught in review. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

jphein · 2026-04-30T21:00:51Z

Pushed 06bf59e addressing both Copilot notes:

await request.json() now wraps in try/except for JSONDecodeError → 400 with a clear message (was bubbling as 500), and validates the parsed body is a dict before reading keys. Aligned with the /silent-save and /mcp shape.
PATCH with no patch fields ({}) now returns 400 listing the valid keys instead of forwarding an ambiguous no-op to mempalace_update_drawer.

DELETE handler unchanged (no body parsing). Docstring expanded to make the body contract explicit.

… PATCH The PATCH /memory/{id} handler already has malformed-JSON + non-object guards (per Copilot review on this PR). Mirrored the same shape on POST /memory so a client sending an empty body or a JSON array gets a 400 with a clear message instead of an opaque 500 from .get() raised on something that isn't a dict. No behavior change for well-formed requests.

Copilot AI review requested due to automatic review settings April 30, 2026 17:02

Copilot started reviewing on behalf of jphein April 30, 2026 17:03 View session

jphein mentioned this pull request Apr 30, 2026

feat: GET /graph — single-shot structural snapshot for SME-style consumers #13

Open

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: DELETE /memory/{id} + PATCH /memory/{id}#17

feat: DELETE /memory/{id} + PATCH /memory/{id}#17
jphein wants to merge 3 commits intorboarescu:mainfrom
jphein:feat/memory-rest-crud

jphein commented Apr 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

jphein May 6, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

jphein May 6, 2026

Uh oh!

jphein commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    body = await request.json()
+    try:
+        body = await request.json()
+    except json.JSONDecodeError as exc:
+        raise HTTPException(status_code=400, detail="Request body must be valid JSON.") from exc
+    if not isinstance(body, dict):
+        raise HTTPException(status_code=400, detail="Request body must be a JSON object.")

Conversation

jphein commented Apr 30, 2026

Shape

Why

What's not in this PR

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

jphein May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

jphein May 6, 2026

Choose a reason for hiding this comment

Uh oh!

jphein commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants