feat(mcp): add metadata param to edit_note for frontmatter updates by phernandez · Pull Request #1090 · basicmachines-co/basic-memory

phernandez · 2026-07-16T20:30:35Z

Implements #1011. Generated by the @claude workflow; opened for review — not yet verified by a human (the run intentionally did not execute the test suite; CI does — see note below).

Summary

Adds an optional metadata: dict param to edit_note that merges into a note's YAML frontmatter independent of operation, so an agent can update structured fields (e.g. status, closed_at) without a fragile find_replace on an exact frontmatter line. Mirrors write_note's existing metadata param.

edit_note(identifier="tickets/...", operation="append", content="",
          metadata={"status": "resolved", "closed_at": "2026-06-18T10:42:00Z"})

Merge semantics: set/overwrite provided keys; other frontmatter + the body untouched. Key deletion out of scope for v1.
Identity fields (title/type/permalink) are dropped from the merge (they have dedicated resolution paths).
Threaded through EditEntityRequest → the accepted-note-mutation pipeline → EntityService.

Review checklist

frontmatter merge preserves unrelated keys + body
identity-field guard behaves as intended
auto-create path (append/prepend on missing note) accepts metadata

Note

This branch was produced without running just/pytest locally — just/uv aren't installed on the Actions runner, and the first run timed out trying to install them. Rely on this PR's CI run for verification. (Follow-up: add a uv/just setup step to claude.yml so future runs can self-verify.)

Closes #1011

🤖 Generated with Claude Code

Adds a `metadata: Optional[dict]` param to edit_note/EditEntityRequest that merges YAML frontmatter fields independent of `operation`, mirroring the `metadata` precedent already on write_note. Previously the only way to touch frontmatter through edit_note was a fragile find_replace against an exact line. Threaded through the accepted-note-mutation pipeline (run_accepted_note_edit -> prepare_accepted_note_edit -> EntityService.prepare_edit_entity_content) via a new _merge_metadata_into_markdown helper that reuses the existing frontmatter parsing so title/permalink resolution stays consistent. The auto-create path (append/prepend on a missing note) also accepts metadata. title/type/permalink are ignored since they already have dedicated resolution paths elsewhere. Closes #1011 Co-authored-by: Paul Hernandez <60959+phernandez@users.noreply.github.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e45ba820a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-16T20:34:40Z

+
+    if has_frontmatter(markdown_content):
+        current_metadata = parse_frontmatter(markdown_content)
+        body = remove_frontmatter(markdown_content)


Preserve note body when merging metadata

When metadata is applied to a note that already has frontmatter, remove_frontmatter() returns body.strip(), so a frontmatter-only update rewrites the body by deleting leading indentation/blank lines and trailing spaces/newlines (for example Markdown hard-break spaces at the end of the body). That violates the advertised merge semantics that the body is left untouched and can corrupt formatting even though the caller only asked to set frontmatter; extract the body after the closing fence without stripping before dumping it back.

Useful? React with 👍 / 👎.

phernandez · 2026-07-16T20:54:11Z

Local validation

CI didn't auto-run on this branch (bot-token push + test.yml is push-only, so GitHub doesn't fire it). Ran the added/affected tests locally:

uv run pytest test-int/mcp/test_edit_note_integration.py \
  tests/api/v2/test_knowledge_router.py \
  tests/services/test_entity_service_prepare.py \
  tests/indexing/test_accepted_note_mutation_runner.py \
  tests/indexing/test_accepted_note_write_runner.py
→ 143 passed, 1 failed

The one failure is a test-assertion bug, not a feature bug. The metadata merge and the identity-field guard both work — status: resolved was merged, and title/permalink were correctly left untouched. The test just hardcoded the wrong expected permalink (missing the fixture's test-project/ prefix):

test_edit_note_metadata_ignores_identity_fields
assert frontmatter["permalink"] == "tickets/identity-guard-note" → actual test-project/tickets/identity-guard-note

Fix: expect the project-prefixed permalink.

@claude

Anthropic's OIDC exchange is broken for the pull_request_target event (401 'Invalid OIDC token'; anthropics/claude-code-action#713). The workflow invoked that path whenever a PR body contained '@claude' — and bot-authored PR descriptions carry attribution text ('Generated by the @claude workflow'), so opening #1088/#1089/#1090/#1091 self-triggered the broken path and each failed its 'claude' check with a 401. A normal issue_comment run succeeded with the same secret in between, so the OAuth token is valid and needs no rotation. Remove the pull_request_target trigger and its PR-body '@claude' clause, so attribution text can never be read as a command and no event routes into the broken OIDC path. Explicit '@claude' mentions in issues, issue comments, PR reviews, and PR review comments remain the trigger. Also drop the now-dead pull_request.author_association gate clauses (reviewer identity is covered by sender.author_association) and the pull_request_target-only checkout ref. If automatic PR-open review is wanted later, it should be a dedicated workflow with a fixed prompt, not a PR-body mention. Signed-off-by: phernandez <paul@basicmachines.co>

…xed permalink test_edit_note_metadata_ignores_identity_fields hardcoded the permalink as 'tickets/identity-guard-note', but this integration harness prefixes the project name ('test-project/tickets/identity-guard-note'), so the assertion failed even though the identity guard worked (permalink was not hijacked to the metadata value). Assert the note's own slug via endswith and that it is not the hijack value, which tests the actual intent and is robust to the fixture prefix. Signed-off-by: phernandez <paul@basicmachines.co>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 86d8093d73

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-17T00:05:13Z

+    overwrites the existing frontmatter value or is added new. The note body, and any
+    frontmatter keys not present in ``metadata``, are left untouched.
+    """
+    sanitized = {k: v for k, v in metadata.items() if k not in _METADATA_IDENTITY_FIELDS}


Reject null metadata values instead of dropping fields

When a JSON/API caller sends metadata: {"status": null} (for example from an optional field left unset), this merge keeps status and overwrites the existing value with YAML null; immediately afterward prepare_edit_entity_content() normalizes and filters None out of entity_fields, so the indexed metadata loses status even though the tool docs say key deletion is not supported. Please reject or ignore None values before merging, or add explicit delete semantics.

Useful? React with 👍 / 👎.

… param The new optional edit_note(metadata=...) param changes two pinned expectations: - test_mcp_tool_signatures_are_stable: add 'metadata' to edit_note's expected signature (between replace_subsections and output_format). - test_edit_note_emits_root_operation_and_project_context: the span now carries has_metadata; add it to the exact-match expected attrs. Both are intended consequences of the additive, backward-compatible param. Signed-off-by: phernandez <paul@basicmachines.co>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1adec0b040

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-17T00:36:58Z

+        if metadata:
+            markdown_content = _merge_metadata_into_markdown(markdown_content, metadata)


Preserve body for metadata-only edits

When callers follow the documented metadata-only pattern (operation="append", content="", with metadata), the edit operation has already run before the metadata merge. For notes whose current markdown has no trailing newline (especially plain notes without frontmatter), append with empty content still appends "\n", so a request advertised as leaving the body untouched changes the note body just to update frontmatter. Skip the content operation for metadata-only no-op edits, or make empty append/prepend a true no-op in this path.

Useful? React with 👍 / 👎.

chatgpt-codex-connector Bot reviewed Jul 16, 2026

View reviewed changes

phernandez mentioned this pull request Jul 16, 2026

fix(ci): drop broken pull_request_target trigger + provision toolchain for Claude workflow #1091

Merged

chatgpt-codex-connector Bot reviewed Jul 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(mcp): add metadata param to edit_note for frontmatter updates#1090

feat(mcp): add metadata param to edit_note for frontmatter updates#1090
phernandez wants to merge 3 commits into
mainfrom
claude/issue-1011-20260716-2019

phernandez commented Jul 16, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 16, 2026

Uh oh!

phernandez commented Jul 16, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		if metadata:
		markdown_content = _merge_metadata_into_markdown(markdown_content, metadata)

Uh oh!

Conversation

phernandez commented Jul 16, 2026

Summary

Review checklist

Note

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

phernandez commented Jul 16, 2026

Local validation

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant