Skip to content

Add a place-axis project parameter to save_memory - #761

Merged
alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/ai-2473-save-memory-project-place
Sep 3, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/ai-2473-save-memory-project-place

Conversation

@alexeyzimarev

@alexeyzimarev alexeyzimarev commented Sep 3, 2026 •

Copy link
Copy Markdown
Member

Closes #755 — AI-2473

What & why

save_memory could only home a memory at the cwd repo or, with global: true, at the org, so a learning that spans a project's repos landed at org and was injected into every repo's SessionStart index. The tool now takes an optional project slug on the place axis, the same field and meaning as rescope_memory's project, and distinct from audience_project (the people axis). A project save skips the "cannot resolve the current repository" check, since the project is the home. Needs the server side of AI-2469 (project on POST /api/memories).

Where to look

The cwd repo hash still rides along with project. A server with AI-2469 drops it once the project resolves; a server without it ignores project and would read a null hash as org-wide, so with the hash it homes the memory at the repo, narrower than asked rather than wider. A present-but-blank project is rejected locally instead of being read as absent, since absent falls back to the repo or org home. project beside global: true is not an error; project wins, matching the server and rescope_memory.

Verification

dotnet run --project test/Capacitor.Cli.Tests.Unit/... -- --treenode-filter "/*/*/McpMemoryServerTests/*"
  total: 21  failed: 0   (5 new; each watched failing before its change)
Full Capacitor.Cli.Tests.Unit: total 3948, failed 0, skipped 19
dotnet publish -c Release | grep -E 'IL[23][01][0-9]{2}'  → no output

A project home sends repo_hash as null: the server stores one (scope_kind, scope_id) home and derives the response's repo hash from it, so a hash beside a project is dropped, not kept. A blank project is rejected rather than read as absent, because absent falls back to the repo or org home and would silently widen where the memory surfaces.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 3, 2026

Copy link
Copy Markdown

AI-2473

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-03T17:12:03.850650Z cec8002 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add project-scoped placement to save_memory

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Add project homes to save_memory for learnings shared across a project's repositories.
• Keep project placement distinct from project-member audience selection.
• Reject blank projects and verify precedence, schema exposure, and repo-independent saves.
Diagram

graph TD
    A["MCP client"] --> B["save_memory"] --> C{"Project supplied?"}
    C -->|Yes| D["Project payload"] --> H["Memory API"]
    C -->|No| E{"Global save?"}
    E -->|Yes| F["Org payload"] --> H
    E -->|No| G["Repo payload"] --> H
Loading
High-Level Assessment

The direct schema and request-body extension is the best approach because it matches the server contract and existing rescope_memory place-axis semantics. A separate save tool or reuse of audience_project would unnecessarily duplicate behavior or conflate placement with access control.

Files changed (5) +74 / -16

Enhancement (1) +20 / -12
McpMemoryServer.csSupport project-scoped save_memory requests +20/-12

Support project-scoped save_memory requests

• Adds the optional place-axis 'project' argument, rejects blank slugs, and permits project saves without repository context. Project placement is sent separately from 'audience_project', clears 'repo_hash', takes precedence over global/repository placement, and is exposed through the MCP tool schema.

src/Capacitor.Cli/Commands/McpMemoryServer.cs

Tests (1) +49 / -0
McpMemoryServerTests.csCover project placement payloads and schema +49/-0

Cover project placement payloads and schema

• Adds tests for project payload serialization, repository-hash removal, operation without repository context, blank-slug rejection, and separation from 'audience_project'. Also verifies that project placement is optional and exposed by the tool schema.

test/Capacitor.Cli.Tests.Unit/Commands/McpMemoryServerTests.cs

Documentation (3) +5 / -4
README.mdDocument project homes for saved memories +1/-1

Document project homes for saved memories

• Expands the 'save_memory' documentation to distinguish audience from placement. Explains repository, project, and organization homes and clarifies that project placement wins over repository scope.

README.md

README.mdAdd project placement to the memory tool summary +1/-1

Add project placement to the memory tool summary

• Updates the concise tool table to list project-scoped placement alongside repository-default and organization-wide saves.

kcap/README.md

help-mcp.txtDescribe independent memory audience and home scopes +3/-2

Describe independent memory audience and home scopes

• Reframes memory help around separate audience and home concepts, including project homes spanning multiple repositories.

src/Capacitor.Cli.Core/Resources/help-mcp.txt

@qodo-code-review

qodo-code-review Bot commented Sep 3, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Malformed project returns internal error ✗ Dismissed 🐞 Bug ☼ Reliability
Description
BuildSaveBody reads the new project field with an unchecked GetValue<string>(), so non-string
JSON values bypass the validation-error path and produce a generic internal error. This prevents
callers from receiving an actionable argument error for malformed project input.
Code

src/Capacitor.Cli/Commands/McpMemoryServer.cs[285]

+        var project         = args?["project"]?.GetValue<string>();
Relevance

●●● Strong

Recent MCP precedents accept hardening unchecked GetValue parsing to return actionable validation
errors.

PR-#344
PR-#284

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed line directly performs the unchecked conversion. HandleToolCallAsync only converts
ArgumentException and HttpRequestException into specific tool errors, while its outer dispatcher
catches other exceptions and returns Error: internal error handling the request.; this
malformed-input pattern has also previously required hardening in MCP argument parsing.

src/Capacitor.Cli/Commands/McpMemoryServer.cs[163-205]
src/Capacitor.Cli/Commands/McpMemoryServer.cs[279-290]
PR-#344

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `save_memory.project` argument is read using `GetValue<string>()`. Non-string JSON values throw outside the local `ArgumentException` handling and are returned as a generic internal error.

## Issue Context
The MCP schema advertises a string, but incoming tool arguments are not guaranteed to have been schema-validated. Convert malformed values into a precise `ArgumentException` and add coverage for numeric, boolean, object, and array values.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpMemoryServer.cs[283-290]
- test/Capacitor.Cli.Tests.Unit/Commands/McpMemoryServerTests.cs[114-122]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 55 rules
Review mode: ⚖️ Balanced: This is a localized runtime/API behavior change to memory scoping with server-contract and fail-closed semantics, warranting a careful single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Commands/McpMemoryServer.cs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cec80020c5

ℹ️ 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".

["repo_hash"] = global ? null : cwdRepoHash,
// The place axis: a project home wins over the repo, so the record carries no repo hash.
["project"] = project,
["repo_hash"] = global || project is not null ? null : cwdRepoHash,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fail closed when the server cannot honor project scope

When project is used against a server predating support for that request field, the server ignores project but still receives repo_hash: null, which it interprets as an org-wide home. A memory requested for one project can therefore surface in every repository—and, for a broad audience, expose its contents across the organization—during staggered client/server upgrades. Gate this path on server capability/version or otherwise require an acknowledgement before removing the repository scope.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 21f9fcd: the cwd repo hash now rides along with project. A server that resolves project drops the hash (verified on the server branch: after ResolveSaveScopeAsync only the resolved scope is read), and a server that does not know the field homes the memory at the repo, narrower than asked rather than org-wide. No version gate: the fallback is in the request itself.

One correction to the framing: place does not govern visibility, audience does. A user-audience memory homed at org is still visible only to that user, so the failure mode is surfacing in every repo's index, not exposure across the organization. The remaining gap is a project save from outside a checkout against an old server, which has no narrower fallback than org.

A server without project homes ignores the field and reads repo_hash: null as org-wide, so the hash rides along: a server that resolves project drops it, an older one homes the memory at the repo, narrower than asked rather than wider.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev merged commit dc960cd into main Sep 3, 2026
10 of 11 checks passed
@alexeyzimarev
alexeyzimarev deleted the alexeyzimarev/ai-2473-save-memory-project-place branch September 3, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

save_memory: add a place-axis project parameter

1 participant