diff --git a/internal/mcp/mcp.go b/internal/mcp/mcp.go index 547f76f38..8b13da0af 100644 --- a/internal/mcp/mcp.go +++ b/internal/mcp/mcp.go @@ -65,10 +65,6 @@ var addPromptIfMissing = func(s *store.Store, params store.AddPromptParams) (int return s.AddPromptIfMissing(params) } -var findSessionSummaryCandidates = func(s *store.Store, savedID int64, opts store.CandidateOptions) ([]store.Candidate, error) { - return s.FindCandidates(savedID, opts) -} - var loadMCPStats = func(s *store.Store) (*store.Stats, error) { return s.Stats() } @@ -1424,81 +1420,6 @@ func handleSave(s *store.Store, cfg MCPConfig, activity *SessionActivity) server } } -// sessionSummaryCandidateResponse runs candidate detection only for a persisted -// session summary and builds its conflict-review response metadata. -func sessionSummaryCandidateResponse(s *store.Store, cfg MCPConfig, savedID int64, project string, msg *string) map[string]any { - extra := map[string]any{} - obs, obsErr := s.GetObservation(savedID) - if obsErr == nil { - extra["id"] = savedID - extra["sync_id"] = obs.SyncID - extra["state"] = obs.State() - if obs.ReviewAfter != nil { - extra["review_after"] = *obs.ReviewAfter - } - } - if obsErr != nil { - extra["judgment_required"] = false - return extra - } - query := sessionSummaryCandidateQuery(obs.Content) - if strings.TrimSpace(query) == "" { - extra["judgment_required"] = false - return extra - } - candOpts := store.CandidateOptions{Project: project, BM25Floor: cfg.BM25Floor, Query: query} - if cfg.Limit != nil { - candOpts.Limit = *cfg.Limit - } - candidates, candErr := findSessionSummaryCandidates(s, savedID, candOpts) - if candErr != nil { - // Candidate discovery is non-fatal after the source observation is saved. - fmt.Fprintf(os.Stderr, "engram: FindCandidates error (non-fatal): %v\n", candErr) - } - - if len(candidates) == 0 { - extra["judgment_required"] = false - return extra - } - - extra["judgment_required"] = true - extra["judgment_status"] = "pending" - extra["judgment_id"] = candidates[0].JudgmentID - candList := make([]map[string]any, 0, len(candidates)) - for _, c := range candidates { - entry := map[string]any{"id": c.ID, "sync_id": c.SyncID, "title": c.Title, "type": c.Type, "score": c.Score, "judgment_id": c.JudgmentID} - if c.TopicKey != nil { - entry["topic_key"] = *c.TopicKey - } - candList = append(candList, entry) - } - extra["candidates"] = candList - *msg += fmt.Sprintf("\nCONFLICT REVIEW PENDING — %d candidate(s); use mem_judge to record verdicts.", len(candidates)) - return extra -} - -// sessionSummaryCandidateQuery removes the required protocol headings before -// OR-based FTS matching so they cannot create candidates by themselves. -func sessionSummaryCandidateQuery(content string) string { - mandatoryHeadings := map[string]struct{}{ - "goal": {}, "instructions": {}, "discoveries": {}, "accomplished": {}, - "next steps": {}, "relevant files": {}, - } - lines := strings.Split(content, "\n") - retained := make([]string, 0, len(lines)) - for _, line := range lines { - heading := strings.TrimSpace(line) - if strings.HasPrefix(heading, "#") { - heading = strings.TrimSpace(strings.TrimLeft(heading, "#")) - if _, mandatory := mandatoryHeadings[strings.ToLower(heading)]; mandatory { - continue - } - } - retained = append(retained, line) - } - return strings.Join(retained, "\n") -} - func handleSuggestTopicKey() server.ToolHandlerFunc { return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { typ, _ := req.GetArguments()["type"].(string) @@ -2094,7 +2015,15 @@ func handleSessionSummary(s *store.Store, cfg MCPConfig, activity *SessionActivi msg += "\n" + score } detRes.Project = project - extra := sessionSummaryCandidateResponse(s, cfg, savedID, project, &msg) + extra := map[string]any{"judgment_required": false} + if obs, obsErr := s.GetObservation(savedID); obsErr == nil { + extra["id"] = savedID + extra["sync_id"] = obs.SyncID + extra["state"] = obs.State() + if obs.ReviewAfter != nil { + extra["review_after"] = *obs.ReviewAfter + } + } return respondWithProject(detRes, msg, extra), nil } } diff --git a/internal/mcp/mcp_test.go b/internal/mcp/mcp_test.go index f4006b655..66f135dc9 100644 --- a/internal/mcp/mcp_test.go +++ b/internal/mcp/mcp_test.go @@ -3615,14 +3615,24 @@ func TestHandleSessionSummaryCreatesProjectScopedSession(t *testing.T) { assertSessionSyncMutationDirectory(t, s, "manual-save-summary-session-project", dir) } -func TestHandleSessionSummarySurfacesConflictCandidates(t *testing.T) { +func TestHandleSessionSummarySkipsConflictCandidates(t *testing.T) { s := newMCPTestStore(t) - for _, sessionID := range []string{"summary-candidates-1", "summary-candidates-2", "summary-candidates-3"} { + for _, sessionID := range []string{"summary-candidates-1", "summary-candidates-2"} { if err := s.CreateSession(sessionID, "summary-candidates", t.TempDir()); err != nil { t.Fatalf("CreateSession(%q): %v", sessionID, err) } } h := handleSessionSummary(s, MCPConfig{}, NewSessionActivity(10*time.Minute)) + assertNoCandidateMetadata := func(name string, envelope map[string]any) { + if required, ok := envelope["judgment_required"].(bool); !ok || required { + t.Fatalf("%s judgment_required = %v, want false", name, envelope["judgment_required"]) + } + for _, field := range []string{"candidates", "judgment_id", "judgment_status"} { + if _, ok := envelope[field]; ok { + t.Fatalf("%s unexpectedly includes %q: %v", name, field, envelope[field]) + } + } + } first, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{ "project": "summary-candidates", @@ -3649,6 +3659,7 @@ internal/billing/ledger.go`, t.Fatalf("first session summary: err=%v isError=%v text=%s", err, first.IsError, callResultText(t, first)) } firstEnvelope := parseEnvelope(t, "first session summary", first) + assertNoCandidateMetadata("first session summary", firstEnvelope) firstSyncID, _ := firstEnvelope["sync_id"].(string) firstSummary, err := s.GetObservationBySyncID(firstSyncID) if err != nil { @@ -3659,90 +3670,51 @@ internal/billing/ledger.go`, "project": "summary-candidates", "session_id": "summary-candidates-2", "content": `## Goal -PostgreSQL partition strategy revision + PostgreSQL partition pruning strategy revision ## Instructions -Benchmark archive ledger ingestion + Benchmark invoice ledger ingestion ## Discoveries -Btree statistics refresh behavior + Btree vacuum checkpoint behavior ## Accomplished -Created partition constraints + Created partition indexes and constraints ## Next Steps -Rehearse rollback + Rehearse failover and rollback ## Relevant Files -internal/billing/archive.go`, + internal/billing/ledger.go`, }}}) if err != nil || second.IsError { t.Fatalf("second session summary: err=%v isError=%v text=%s", err, second.IsError, callResultText(t, second)) } - envelope := parseEnvelope(t, "session summary candidates", second) - if required, _ := envelope["judgment_required"].(bool); !required { - t.Fatalf("expected judgment_required=true, got %v", envelope["judgment_required"]) - } - candidates, _ := envelope["candidates"].([]any) - if len(candidates) == 0 { - t.Fatal("expected a summary conflict candidate") - } - candidate, ok := candidates[0].(map[string]any) - if !ok { - t.Fatalf("candidate has type %T, want object", candidates[0]) - } - for _, field := range []string{"id", "sync_id", "title", "type", "score", "judgment_id"} { - if _, ok := candidate[field]; !ok { - t.Errorf("candidate is missing %q", field) - } - } - if candidate["title"] != "Session summary: summary-candidates" { - t.Fatalf("summary title changed to %q", candidate["title"]) - } - judgmentID, _ := candidate["judgment_id"].(string) - relation, err := s.GetRelation(judgmentID) + secondEnvelope := parseEnvelope(t, "second session summary", second) + assertNoCandidateMetadata("second session summary", secondEnvelope) + secondSyncID, _ := secondEnvelope["sync_id"].(string) + secondSummary, err := s.GetObservationBySyncID(secondSyncID) if err != nil { - t.Fatalf("GetRelation(%q): %v", judgmentID, err) + t.Fatalf("GetObservationBySyncID(%q): %v", secondSyncID, err) } - if relation.JudgmentStatus != store.JudgmentStatusPending { - t.Fatalf("judgment status = %q, want pending", relation.JudgmentStatus) + if firstSummary.Content == secondSummary.Content { + t.Fatal("expected distinct session summaries to persist") } - third, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{ - "project": "summary-candidates", - "session_id": "summary-candidates-3", - "content": `## Goal -Mobile screenreader navigation release - -## Instructions -Audit focus order contrast - -## Discoveries -VoiceOver rotor semantics - -## Accomplished -Updated accessible labels - -## Next Steps -Validate TalkBack gestures - -## Relevant Files -web/mobile/navigation.ts`, - }}}) - if err != nil || third.IsError { - t.Fatalf("third session summary: err=%v isError=%v text=%s", err, third.IsError, callResultText(t, third)) + var pendingRelations int + if err := s.DB().QueryRow(`SELECT COUNT(*) FROM memory_relations WHERE judgment_status = ?`, store.JudgmentStatusPending).Scan(&pendingRelations); err != nil { + t.Fatalf("count pending relations: %v", err) } - thirdEnvelope := parseEnvelope(t, "unrelated session summary", third) - if required, _ := thirdEnvelope["judgment_required"].(bool); required { - t.Fatalf("unrelated summary should not match only because session summary titles are constant: %v", thirdEnvelope["candidates"]) + if pendingRelations != 0 { + t.Fatalf("pending relations = %d, want 0", pendingRelations) } summaries, err := s.RecentObservations("summary-candidates", firstSummary.Scope, 10) if err != nil { t.Fatalf("RecentObservations: %v", err) } - if len(summaries) != 3 { - t.Fatalf("expected three separate summaries, got %d", len(summaries)) + if len(summaries) != 2 { + t.Fatalf("expected two separate summaries, got %d", len(summaries)) } for _, summary := range summaries { if summary.TopicKey != nil { @@ -3751,162 +3723,6 @@ web/mobile/navigation.ts`, } } -func TestHandleSessionSummarySkipsCandidatesForHeadingsOnlyContent(t *testing.T) { - s := newMCPTestStore(t) - const project = "headings-only-summary" - const sessionID = "headings-only-summary-session" - if err := s.CreateSession(sessionID, project, t.TempDir()); err != nil { - t.Fatalf("CreateSession: %v", err) - } - - originalFindCandidates := findSessionSummaryCandidates - candidateSearchCalled := false - findSessionSummaryCandidates = func(*store.Store, int64, store.CandidateOptions) ([]store.Candidate, error) { - candidateSearchCalled = true - return nil, nil - } - t.Cleanup(func() { findSessionSummaryCandidates = originalFindCandidates }) - - h := handleSessionSummary(s, MCPConfig{}, NewSessionActivity(10*time.Minute)) - result, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{ - "project": project, - "session_id": sessionID, - "content": `## Goal -## Instructions -## Discoveries -## Accomplished -## Next Steps -## Relevant Files`, - }}}) - if err != nil || result.IsError { - t.Fatalf("session summary: err=%v isError=%v text=%s", err, result.IsError, callResultText(t, result)) - } - if candidateSearchCalled { - t.Fatal("headings-only session summary should skip candidate search") - } - - envelope := parseEnvelope(t, "headings-only session summary", result) - if required, _ := envelope["judgment_required"].(bool); required { - t.Fatalf("judgment_required = %v, want false", envelope["judgment_required"]) - } - if _, ok := envelope["candidates"]; ok { - t.Fatalf("headings-only session summary should not include candidates: %v", envelope["candidates"]) - } -} - -func TestHandleSessionSummaryCandidateQueryUsesPersistedContent(t *testing.T) { - for _, tc := range []struct { - name string - maxContent int - content string - absentTerm string - }{ - { - name: "private tag redaction", - content: "codenameaerolith public summary text", - absentTerm: "codenameaerolith", - }, - { - name: "storage truncation", - maxContent: 32, - content: "visible summary text candidategammafourteen", - absentTerm: "candidategammafourteen", - }, - } { - t.Run(tc.name, func(t *testing.T) { - s := newMCPTestStoreWithMaxContentLength(t, tc.maxContent) - const project = "persisted-summary-candidates" - for _, sessionID := range []string{"candidate-source", "summary-source"} { - if err := s.CreateSession(sessionID, project, t.TempDir()); err != nil { - t.Fatalf("CreateSession(%q): %v", sessionID, err) - } - } - if _, err := s.AddObservation(store.AddObservationParams{ - SessionID: "candidate-source", - Type: "decision", - Title: tc.absentTerm, - Content: tc.absentTerm, - Project: project, - Scope: "project", - }); err != nil { - t.Fatalf("AddObservation(candidate): %v", err) - } - - originalFindCandidates := findSessionSummaryCandidates - var candidateQuery string - findSessionSummaryCandidates = func(s *store.Store, savedID int64, opts store.CandidateOptions) ([]store.Candidate, error) { - candidateQuery = opts.Query - if strings.Contains(opts.Query, tc.absentTerm) { - return originalFindCandidates(s, savedID, opts) - } - return nil, nil - } - t.Cleanup(func() { findSessionSummaryCandidates = originalFindCandidates }) - - h := handleSessionSummary(s, MCPConfig{}, NewSessionActivity(10*time.Minute)) - result, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{ - "project": project, - "session_id": "summary-source", - "content": tc.content, - }}}) - if err != nil || result.IsError { - t.Fatalf("session summary: err=%v isError=%v text=%s", err, result.IsError, callResultText(t, result)) - } - - envelope := parseEnvelope(t, "persisted summary candidate query", result) - syncID, _ := envelope["sync_id"].(string) - summary, err := s.GetObservationBySyncID(syncID) - if err != nil { - t.Fatalf("GetObservationBySyncID(%q): %v", syncID, err) - } - if strings.Contains(summary.Content, tc.absentTerm) { - t.Fatalf("persisted content %q unexpectedly contains candidate term %q", summary.Content, tc.absentTerm) - } - if candidateQuery != summary.Content { - t.Fatalf("candidate query = %q, want persisted content %q", candidateQuery, summary.Content) - } - if required, _ := envelope["judgment_required"].(bool); required { - t.Fatalf("unexpected candidate from content absent after persistence %q: %v", summary.Content, envelope["candidates"]) - } - }) - } -} - -func TestHandleSessionSummaryPersistsWhenCandidateDetectionFails(t *testing.T) { - s := newMCPTestStore(t) - const sessionID = "summary-candidate-failure" - if err := s.CreateSession(sessionID, "summary-candidate-failure", t.TempDir()); err != nil { - t.Fatalf("CreateSession: %v", err) - } - - originalFindCandidates := findSessionSummaryCandidates - findSessionSummaryCandidates = func(*store.Store, int64, store.CandidateOptions) ([]store.Candidate, error) { - return nil, errors.New("forced candidate detection failure") - } - t.Cleanup(func() { findSessionSummaryCandidates = originalFindCandidates }) - - content := "The session summary must survive a candidate detection failure." - h := handleSessionSummary(s, MCPConfig{}, NewSessionActivity(10*time.Minute)) - result, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{ - "project": "summary-candidate-failure", - "session_id": sessionID, - "content": content, - }}}) - if err != nil || result.IsError { - t.Fatalf("session summary should succeed when candidate detection fails: err=%v isError=%v text=%s", err, result.IsError, callResultText(t, result)) - } - - envelope := parseEnvelope(t, "candidate detection failure", result) - syncID, _ := envelope["sync_id"].(string) - summary, err := s.GetObservationBySyncID(syncID) - if err != nil { - t.Fatalf("GetObservationBySyncID(%q): %v", syncID, err) - } - if summary.Type != "session_summary" || summary.Content != content { - t.Fatalf("persisted summary = type %q content %q, want session_summary and %q", summary.Type, summary.Content, content) - } -} - func TestHandleCapturePassiveCreatesProjectScopedSession(t *testing.T) { s := newMCPTestStore(t) h := handleCapturePassive(s, MCPConfig{}, NewSessionActivity(10*time.Minute))