Skip to content

fix(mcp): handle non-object checkpoint acknowledgements - #2273

Draft
fallenmi wants to merge 1 commit into
MemPalace:developfrom
fallenmi:codex/fix-checkpoint-ack-json-shape
Draft

fix(mcp): handle non-object checkpoint acknowledgements#2273
fallenmi wants to merge 1 commit into
MemPalace:developfrom
fallenmi:codex/fix-checkpoint-ack-json-shape

Conversation

@fallenmi

Copy link
Copy Markdown

What does this PR do?

Fixes #2272.

json.loads() accepts scalar and array roots, but tool_memories_filed_away() assumed the decoded checkpoint was an object. It unlinked the one-shot last_checkpoint marker and then called .get(), so MCP clients received an internal error with no acknowledgement marker left to retry.

This change:

  • requires a decoded checkpoint to be a dict before reading msgs or ts;
  • deliberately keeps unlink-before-validation, preserving the tool's existing consume-on-read contract;
  • maps null, array, string, number, and boolean roots to the same content-free status: error response already used for malformed JSON syntax;
  • preserves valid-object and missing-file behavior.

The scope is intentionally limited to valid non-object JSON roots. Invalid UTF-8, unlink races/failures, and malformed fields inside an object remain separate concerns.

How to test

  • python -m pytest -q tests/test_mcp_server.py tests/test_mcp_http_transport.py tests/test_hooks_cli.py — 548 passed, 1 skipped
  • Full split suite — 4,306 passed, 31 skipped
    • main suite: 4,297 passed, 31 skipped, 107 deselected
    • isolated cwd/install-sensitive subprocess cases: 9 passed
  • pre-commit run --all-files

The regression matrix covers all five non-object JSON root types, a valid checkpoint object, invalid JSON syntax, a missing marker, marker consumption, and rejected-payload non-reflection. The existing HTTP read-only acknowledgement contract also remains green.

Checklist

  • Tests pass
  • No hardcoded paths
  • Linter passes

@igorls igorls added bug Something isn't working area/mcp MCP server and tools needs-rebase PR has merge conflicts with develop and needs rebase labels Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/mcp MCP server and tools bug Something isn't working needs-rebase PR has merge conflicts with develop and needs rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: checkpoint acknowledgement crashes after consuming non-object JSON

2 participants