fix(mcp): harden handleSessionSummary with process override and empty-content guard#424
Merged
Merged
Conversation
…ent guard Closes #403 Closes #413 Closes #393 handleSessionSummary was the one write tool missed in commit b094052: it called bare resolveWriteProject() instead of resolveWriteProjectWithProcessOverride(), so ENGRAM_PROJECT / `engram mcp --project` was silently ignored, causing writes to the wrong project and an OpenCode timeout on ambiguous cwd. Additionally, no empty-content guard existed before AddObservation; an empty mem_session_summary would persist a blank observation that created a stuck sync_mutation blocking cloud upgrade. Fixes: - Switch to resolveWriteProjectWithProcessOverride(cfg.DefaultProject) so the process-level override takes precedence over cwd detection, matching all other write tools. - Add strings.TrimSpace(content) == "" guard (same pattern as handleSave / #374) before any project resolution or store write. Tests: 4 new table-driven tests covering process-override with no-git dir, process-override bypassing ambiguous cwd, empty content rejection, and whitespace-only content rejection.
There was a problem hiding this comment.
Pull request overview
This PR hardens the MCP mem_session_summary write path to better match the expected “trusted process override first” project-resolution behavior and to prevent persisting empty session summaries that can later break cloud upgrade/sync validation.
Changes:
- Reject empty/whitespace-only
contentinhandleSessionSummarybefore any project resolution or store writes. - Resolve write project via
resolveWriteProjectWithProcessOverride(cfg.DefaultProject)soENGRAM_PROJECT/engram mcp --projectis honored. - Add unit tests covering process override behavior and empty/whitespace content rejection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/mcp/mcp.go |
Adds empty-content guard and switches session summary project resolution to honor the process-level override. |
internal/mcp/mcp_test.go |
Adds tests for session summary process override and empty/whitespace content rejection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1608
to
1613
| // Honour process-level project override (cfg.DefaultProject) set via | ||
| // ENGRAM_PROJECT or `engram mcp --project` (#403/#413). Falls back to cwd | ||
| // detection when no override is configured. | ||
| detRes, err := resolveWriteProjectWithProcessOverride(cfg.DefaultProject) | ||
| if err != nil { | ||
| return writeProjectErrorResult(nil, "", detRes, err), nil |
…ardening # Conflicts: # internal/mcp/mcp_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
handleSessionSummarywas the sole write tool still calling bareresolveWriteProject()after commitb094052hardened every other write tool. The process-level project override (ENGRAM_PROJECT/engram mcp --project) was silently ignored, causing wrong-project writes and an OpenCode timeout in ambiguous-cwd environments.mem_session_summaryhad no content guard, so it persisted a blank observation that created a stucksync_mutationblockingcloud upgrade.Changes
resolveWriteProjectWithProcessOverride(cfg.DefaultProject), consistent with all other write tools.strings.TrimSpace(content) == ""guard beforeAddObservation, mirroringhandleSave(fix(mcp): preserve mem_save observation content #374).Test plan
4 new tests in
internal/mcp/mcp_test.go: process override writes to default project, override bypasses ambiguous cwd, empty content rejected, whitespace-only rejected.go test ./... && go vet ./... && go build ./...clean.Closes #403
Closes #413
Closes #393