fix(mcp): validate updates against the expected owner

[dnlrsls]

🔗 Linked Issue

Closes #1720

---

🏷️ PR Type

• type:bug — Bug fix
• type:feature — New feature
• type:question — Question requiring tracked work
• type:docs — Documentation only
• type:refactor — Code refactoring (no behavior change)
• type:chore — Maintenance, dependencies, tooling
• type:breaking-change — Breaking change

---

📝 Summary

• Validate MCP updates using the atomic expected-owner assertion, independently of the server cwd or process project.
• Preserve mandatory owner validation and unchanged save/session/delete behavior.
• Align response metadata, regression tests, and documentation.

📂 Changes

File	Change
internal/mcp/mcp.go	Remove the extra process-project gate; report the canonical matched owner and no filesystem path.
internal/mcp/mcp_test.go, internal/mcp/ownership_update_test.go	Replace obsolete cwd-guard expectations with matching/mismatched owner regressions and unchanged-record/revision/queue assertions.
DOCS.md, docs/AGENT-SETUP.md, docs/codebase/memory-core.md	Document the MCP/REST ownership contract and its distinction from authentication.
internal/project/detect.go	Correct the read-only stored-project fallback comment.

🧪 Test Plan

• Focused regression: go test ./internal/mcp -run 'TestHandleUpdate|TestMutationExpectedProject' -count=1 — PASS, including independent verifier execution (exit 0).
• Affected package tests: go test ./internal/mcp ./internal/store ./internal/server ./internal/project -count=1 — PASS.
• Other local checks: git diff --check — PASS; independent static verification — PASS; native risk/readability/reliability review — approved and acknowledged.
• GitHub CI full unit/E2E/plugin/lint and applicable platform checks — pending; local checks do not substitute for CI.

---

🤖 Automated Checks

After pushing/opening the PR, GitHub CI runs the broad unit/E2E/lint and applicable platform checks. These automated statuses remain pending until the actual checks run; do not claim a pass from local evidence. Windows checks run for PRs but not merge groups, and are not among the six required contexts. Lint and Check PR Has No Transient Artifacts also run for PRs but are not required contexts; Lint also runs on pushes to main. All required checks must pass before merge:

Check	What it verifies	Status
Check Issue Reference	PR body contains Closes #N / Fixes #N / Resolves #N	⏳
Check Issue Has status:approved	Linked issue has status:approved label	⏳
Check PR Has type: Label*	Canonical labels, applicability, and cardinality	⏳
Check PR Has No Transient Artifacts	PR files comply with the Transient Artifact Policy	⏳
Unit Tests	go test ./... passes	⏳
E2E Tests	go test -tags e2e ./internal/server/... passes	⏳
Plugin Tests	npm test passes in plugin/pi	⏳
Lint	golangci-lint reports no new findings	⏳
Windows Setup Test	Windows setup preserves absolute paths and MCP job-object parent-lifetime tests pass	⏳
Cloud Sync Wrapper Tests (Windows)	Cloud sync wrapper and missing-PowerShell-Engram tests pass on Windows	⏳

---

✅ Contributor Checklist

• I linked an approved issue above (Closes #N)
• I added exactly one type:* label to this PR
• I recorded actual focused regression and affected package test commands/outcomes for behavior changes, or N/A for docs-only changes
• I recorded additional applicable local checks for an unpushed/no-PR or high-risk change, and identified any missing CI evidence
• Docs updated (if behavior changed)
• Commits follow conventional commits format
• No Co-Authored-By trailers in commits
• I checked every changed path against the Transient Artifact Policy

---

💬 Notes for Reviewers

Single review unit: 559 changed lines (123 additions, 436 deletions), predominantly removal of obsolete process/cwd-guard tests. A maintainer explicitly approved size:exception; separating the behavior, required test expectation changes, and docs would leave an inconsistent intermediate contract.

The owner assertion is not authentication or a permission grant. Remote client authorization remains separate. No store schema changes, save/session resolution changes, or mem_delete implementation changes.

Native review: review-b95667b64e03d644 approved and consumed. Risk: item 2 (ownership guard change), independently verified. Nonblocking coverage note: direct whitespace-only stored-owner MCP coverage was removed; NULL-owner regressions remain and both use the shared normalization guard.

Summary by CodeRabbit

• Behavior Changes
  • Observation updates now rely on the supplied project assertion matching the observation’s stored owner, independent of the server’s current project or working directory.
  • Updates with a mismatched owner are rejected without changing the observation. Successful responses identify the asserted owner without claiming a verified repository path.
• Documentation
  • Clarified how project ownership assertions work for updates and deletes, and distinguished them from authentication or authorization.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: e906d110-9203-4f11-93fc-53a9b0e87495

📥 Commits

Reviewing files that changed from the base of the PR and between efb1277 and e4f4700.

📒 Files selected for processing (7)

• DOCS.md
• docs/AGENT-SETUP.md
• docs/codebase/memory-core.md
• internal/mcp/mcp.go
• internal/mcp/mcp_test.go
• internal/mcp/ownership_update_test.go
• internal/project/detect.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.

---

📝 Walkthrough

Walkthrough

mem_update now checks expected_project against the observation’s normalized stored owner, independent of current-project detection. Its success response reports that owner as an explicit override. Updated tests cover mismatches, NULL-owned observations, and normalized matches.

Changes

Observation ownership assertion

Layer / File(s)	Summary
Update ownership assertion and response internal/mcp/mcp.go, internal/project/detect.go, internal/mcp/mcp_test.go, internal/mcp/ownership_update_test.go, DOCS.md, docs/AGENT-SETUP.md, docs/codebase/memory-core.md	mem_update passes expected_project to the stored-owner check and reports the normalized owner in successful responses. Tests cover mismatches, NULL-owned observations, and normalized matches. Documentation describes the assertion and distinguishes it from authorization.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: gentleman-programming, alan-thegentleman

Merge Risk: ⚪ Minimal · up to e4f47

mem_update now validates expected_project against the observation's stored owner, so shared multi-project servers can update observations. Mismatches are still rejected without mutation. Docs and tests cover the change, and no merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files. (3 skipped: 3…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: validating MCP updates against the caller-provided expected owner.

Full details: Docstring Coverage

Explanation

Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}