diff --git a/internal/mcp/mcp.go b/internal/mcp/mcp.go
index 336f334e..f34c185c 100644
--- a/internal/mcp/mcp.go
+++ b/internal/mcp/mcp.go
@@ -1176,6 +1176,14 @@ func handleSave(s *store.Store, cfg MCPConfig, activity *SessionActivity) server
if strings.TrimSpace(content) == "" {
return mcp.NewToolResultError("content is required for mem_save (use content, or observation for backward-compatible clients)"), nil
}
+ // Reject empty titles early with an agent-actionable message. The store
+ // enforces the same rule (single source of truth), but returning here
+ // lets us tell the model exactly what to provide on retry instead of
+ // surfacing a generic store error. An empty title would otherwise be
+ // accepted and silently block cloud sync downstream. See issue #459.
+ if strings.TrimSpace(title) == "" {
+ return mcp.NewToolResultError("title is required for mem_save — pass a short, searchable title (e.g. 'Fixed N+1 query in UserList', 'Decision: use OpenTofu over Terraform'). Empty titles silently block cloud sync (issue #459)."), nil
+ }
typ, _ := req.GetArguments()["type"].(string)
sessionID, _ := req.GetArguments()["session_id"].(string)
scope, _ := req.GetArguments()["scope"].(string)
diff --git a/internal/mcp/mcp_test.go b/internal/mcp/mcp_test.go
index d09dbc92..a0c045dc 100644
--- a/internal/mcp/mcp_test.go
+++ b/internal/mcp/mcp_test.go
@@ -1478,6 +1478,62 @@ func TestHandleUpdateAcceptsAllOptionalFields(t *testing.T) {
}
}
+// TestHandleUpdateRejectsEmptyTitle pins that mem_update surfaces an
+// agent-actionable error when the new title is empty or whitespace-only, and
+// that the original title is left untouched. This closes the same stuck-sync
+// class as the save path (issue #459) on the update surface.
+func TestHandleUpdateRejectsEmptyTitle(t *testing.T) {
+ cases := []struct {
+ name string
+ title string
+ }{
+ {"empty string", ""},
+ {"only whitespace", " \t\n"},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ s := newMCPTestStore(t)
+ if err := s.CreateSession("s-empty-update", "engram", "/tmp/engram"); err != nil {
+ t.Fatalf("create session: %v", err)
+ }
+ id, err := s.AddObservation(store.AddObservationParams{
+ SessionID: "s-empty-update",
+ Type: "decision",
+ Title: "Original",
+ Content: "Original content",
+ Project: "engram",
+ Scope: "project",
+ })
+ if err != nil {
+ t.Fatalf("add observation: %v", err)
+ }
+
+ res, err := handleUpdate(s)(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
+ "id": float64(id),
+ "title": tc.title,
+ }}})
+ if err != nil {
+ t.Fatalf("update handler error: %v", err)
+ }
+ if !res.IsError {
+ t.Fatalf("expected an error result for empty title, got success")
+ }
+ if !strings.Contains(callResultText(t, res), "title is required") {
+ t.Fatalf("expected 'title is required' in error, got %q", callResultText(t, res))
+ }
+
+ // The original title must survive the rejected update.
+ obs, err := s.GetObservation(id)
+ if err != nil {
+ t.Fatalf("get observation: %v", err)
+ }
+ if obs.Title != "Original" {
+ t.Fatalf("expected title unchanged %q, got %q", "Original", obs.Title)
+ }
+ })
+ }
+}
+
func TestHandleContextWithSessionOnlyUsesNoneProjects(t *testing.T) {
s := newMCPTestStore(t)
if err := s.CreateSession("s-context-none", "engram", "/tmp/engram"); err != nil {
@@ -7293,3 +7349,50 @@ func TestHandleSearchLegacyMixedCaseProject(t *testing.T) {
t.Fatalf("expected search results for legacy project, got: %s", text)
}
}
+
+func TestHandleSaveRejectsEmptyTitle(t *testing.T) {
+ cases := []struct {
+ name string
+ title any
+ }{
+ {"missing title", nil},
+ {"empty string", ""},
+ {"only whitespace", " \t\n"},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ s := newMCPTestStore(t)
+ h := handleSave(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
+
+ args := map[string]any{
+ "content": "Body with no usable title",
+ "type": "bugfix",
+ "project": "engram",
+ }
+ if tc.title != nil {
+ args["title"] = tc.title
+ }
+
+ res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: args}})
+ if err != nil {
+ t.Fatalf("handler error: %v", err)
+ }
+ if !res.IsError {
+ t.Fatalf("expected an error result for empty title, got success")
+ }
+ text := callResultText(t, res)
+ if !strings.Contains(text, "title is required") {
+ t.Fatalf("expected 'title is required' in error, got %q", text)
+ }
+
+ // Nothing should have been persisted.
+ obs, err := s.RecentObservations("engram", "project", 5)
+ if err != nil {
+ t.Fatalf("recent observations: %v", err)
+ }
+ if len(obs) != 0 {
+ t.Fatalf("expected no persisted observations, got %d", len(obs))
+ }
+ })
+ }
+}
diff --git a/internal/server/server.go b/internal/server/server.go
index 244d3b38..1d2ce2ea 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -337,6 +337,10 @@ func (s *Server) handleAddObservation(w http.ResponseWriter, r *http.Request) {
id, err := s.store.AddObservation(body)
if err != nil {
+ if errors.Is(err, store.ErrEmptyObservationTitle) {
+ jsonError(w, http.StatusBadRequest, err.Error())
+ return
+ }
jsonError(w, http.StatusInternalServerError, err.Error())
return
}
@@ -452,7 +456,14 @@ func (s *Server) handleUpdateObservation(w http.ResponseWriter, r *http.Request)
obs, err := s.store.UpdateObservation(id, body)
if err != nil {
- jsonError(w, http.StatusNotFound, err.Error())
+ switch {
+ case errors.Is(err, store.ErrEmptyObservationTitle):
+ jsonError(w, http.StatusBadRequest, err.Error())
+ case errors.Is(err, store.ErrObservationNotFound), errors.Is(err, sql.ErrNoRows):
+ jsonError(w, http.StatusNotFound, err.Error())
+ default:
+ jsonError(w, http.StatusInternalServerError, err.Error())
+ }
return
}
diff --git a/internal/server/server_test.go b/internal/server/server_test.go
index 81f0e157..4358f24d 100644
--- a/internal/server/server_test.go
+++ b/internal/server/server_test.go
@@ -169,6 +169,121 @@ func TestClaudeSaveNudgeCompatibilityRoutes(t *testing.T) {
}
}
+// TestUpdateObservationRejectsEmptyTitle covers the HTTP PATCH path used by the
+// gentle-engram (opencode) plugin, which forwards mem_update to
+// PATCH /observations/{id}. An empty or whitespace-only title must be rejected
+// with 400 (not persisted, not enqueued) and the original title preserved —
+// closing the stuck-sync class from issue #459 on the supported HTTP surface.
+func TestUpdateObservationRejectsEmptyTitle(t *testing.T) {
+ st := newServerTestStore(t)
+ srv := New(st, 0)
+ h := srv.Handler()
+
+ createReq := httptest.NewRequest(http.MethodPost, "/sessions", strings.NewReader(`{"id":"s-empty","project":"engram"}`))
+ createReq.Header.Set("Content-Type", "application/json")
+ createRec := httptest.NewRecorder()
+ h.ServeHTTP(createRec, createReq)
+ if createRec.Code != http.StatusCreated {
+ t.Fatalf("expected session create 201, got %d", createRec.Code)
+ }
+
+ obsReq := httptest.NewRequest(http.MethodPost, "/observations", strings.NewReader(`{"session_id":"s-empty","type":"note","title":"Original","content":"body","project":"engram"}`))
+ obsReq.Header.Set("Content-Type", "application/json")
+ obsRec := httptest.NewRecorder()
+ h.ServeHTTP(obsRec, obsReq)
+ if obsRec.Code != http.StatusCreated {
+ t.Fatalf("expected observation create 201, got %d body=%s", obsRec.Code, obsRec.Body.String())
+ }
+ var created map[string]any
+ if err := json.Unmarshal(obsRec.Body.Bytes(), &created); err != nil {
+ t.Fatalf("decode created observation: %v", err)
+ }
+ id := int64(created["id"].(float64))
+
+ for _, tc := range []struct {
+ name string
+ body string
+ }{
+ {"empty string", `{"title":""}`},
+ {"only whitespace", `{"title":" \t\n"}`},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ req := httptest.NewRequest(http.MethodPatch, fmt.Sprintf("/observations/%d", id), strings.NewReader(tc.body))
+ req.Header.Set("Content-Type", "application/json")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusBadRequest {
+ t.Fatalf("expected 400 for empty title, got %d body=%s", rec.Code, rec.Body.String())
+ }
+
+ getReq := httptest.NewRequest(http.MethodGet, fmt.Sprintf("/observations/%d", id), nil)
+ getRec := httptest.NewRecorder()
+ h.ServeHTTP(getRec, getReq)
+ if getRec.Code != http.StatusOK {
+ t.Fatalf("expected get 200, got %d", getRec.Code)
+ }
+ var got map[string]any
+ if err := json.Unmarshal(getRec.Body.Bytes(), &got); err != nil {
+ t.Fatalf("decode observation: %v", err)
+ }
+ if got["title"] != "Original" {
+ t.Fatalf("expected title unchanged %q, got %v", "Original", got["title"])
+ }
+ })
+ }
+}
+
+// TestAddObservationRejectsEmptyTitle covers the POST /observations route after
+// the store-level empty-title validation change: a whitespace-only title (which
+// passes the raw `title == ""` pre-check) must be rejected with 400 by the
+// store sentinel, and an empty string by the pre-check — neither may persist.
+func TestAddObservationRejectsEmptyTitle(t *testing.T) {
+ st := newServerTestStore(t)
+ srv := New(st, 0)
+ h := srv.Handler()
+
+ createReq := httptest.NewRequest(http.MethodPost, "/sessions", strings.NewReader(`{"id":"s-add-empty","project":"engram"}`))
+ createReq.Header.Set("Content-Type", "application/json")
+ createRec := httptest.NewRecorder()
+ h.ServeHTTP(createRec, createReq)
+ if createRec.Code != http.StatusCreated {
+ t.Fatalf("expected session create 201, got %d", createRec.Code)
+ }
+
+ for _, tc := range []struct {
+ name string
+ body string
+ }{
+ {"empty string", `{"session_id":"s-add-empty","type":"note","title":"","content":"body","project":"engram"}`},
+ {"only whitespace", `{"session_id":"s-add-empty","type":"note","title":" \t\n","content":"body","project":"engram"}`},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ req := httptest.NewRequest(http.MethodPost, "/observations", strings.NewReader(tc.body))
+ req.Header.Set("Content-Type", "application/json")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusBadRequest {
+ t.Fatalf("expected 400 for empty title, got %d body=%s", rec.Code, rec.Body.String())
+ }
+ })
+ }
+
+ // Nothing should have been persisted by the rejected requests.
+ listReq := httptest.NewRequest(http.MethodGet, "/observations?project=engram&limit=10", nil)
+ listRec := httptest.NewRecorder()
+ h.ServeHTTP(listRec, listReq)
+ if listRec.Code != http.StatusOK {
+ t.Fatalf("expected list 200, got %d", listRec.Code)
+ }
+ var obs []map[string]any
+ if err := json.Unmarshal(listRec.Body.Bytes(), &obs); err != nil {
+ t.Fatalf("decode observations: %v", err)
+ }
+ if len(obs) != 0 {
+ t.Fatalf("expected no persisted observations, got %d", len(obs))
+ }
+}
+
func TestAdditionalServerErrorBranches(t *testing.T) {
st := newServerTestStore(t)
srv := New(st, 0)
diff --git a/internal/store/store.go b/internal/store/store.go
index f4e8a32a..ee7e2648 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -51,6 +51,14 @@ var (
ErrObservationNotFound = errors.New("observation not found")
ErrPromptNotFound = errors.New("prompt not found")
ErrProjectNotFound = errors.New("project not found")
+
+ // ErrEmptyObservationTitle is returned by AddObservation and
+ // UpdateObservation when the post-stripPrivateTags title is empty after
+ // trimming whitespace. The check runs on the persisted value because that is
+ // exactly what gets pushed to the cloud, where ValidateSyncMutationPayload
+ // rejects empty titles and silently stalls the sync queue (issue #459).
+ // Callers use errors.Is to map this to a 400-class response.
+ ErrEmptyObservationTitle = errors.New("observation title is required (non-empty after trimming whitespace)")
)
// Sentinel errors for relation sync apply path (Phase 2).
@@ -2255,6 +2263,17 @@ func (s *Store) AddObservation(p AddObservationParams) (int64, error) {
title := stripPrivateTags(p.Title)
content := stripPrivateTags(p.Content)
+ // Reject empty titles before touching the DB. This mirrors the rule
+ // ValidateSyncMutationPayload enforces on the sync path (observation
+ // upserts require a non-empty title). We validate the post-strip value
+ // because that is exactly what gets persisted and later pushed to the
+ // cloud server. Without this, an empty title is accepted locally and the
+ // failure only surfaces later when sync rejects the mutation, silently
+ // blocking the queue with no feedback to the caller. See issue #459.
+ if strings.TrimSpace(title) == "" {
+ return 0, ErrEmptyObservationTitle
+ }
+
if len(content) > s.cfg.MaxObservationLength {
content = content[:s.cfg.MaxObservationLength] + "... [truncated]"
}
@@ -2897,6 +2916,16 @@ func (s *Store) UpdateObservation(id int64, p UpdateObservationParams) (*Observa
topicKey = normalizeTopicKey(*p.TopicKey)
}
+ // Reject empty titles before persisting. We validate the final post-strip
+ // value because that is what gets written and enqueued as a sync upsert.
+ // Without this, mem_update / PATCH /observations/{id} could persist an
+ // empty title and queue a mutation the cloud server rejects, silently
+ // stalling the sync queue — the same class AddObservation guards against
+ // (issue #459).
+ if strings.TrimSpace(title) == "" {
+ return ErrEmptyObservationTitle
+ }
+
if _, err := s.execHook(tx,
`UPDATE observations
SET type = ?,
diff --git a/internal/store/store_test.go b/internal/store/store_test.go
index dd1724b2..382925c7 100644
--- a/internal/store/store_test.go
+++ b/internal/store/store_test.go
@@ -8576,3 +8576,206 @@ func TestMostRecentActiveSessionScopedByProject(t *testing.T) {
t.Fatalf("expected no active session for engram when only 'other' has one, got ok=%v", ok)
}
}
+
+func TestAddObservationRejectsEmptyTitle(t *testing.T) {
+ cases := []struct {
+ name string
+ title string
+ }{
+ {"empty string", ""},
+ {"only spaces", " "},
+ {"only tabs and newlines", "\t\n \n"},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ s := newTestStore(t)
+ _, err := s.AddObservation(AddObservationParams{
+ SessionID: "s1",
+ Type: "decision",
+ Title: tc.title,
+ Content: "some content",
+ Project: "engram",
+ Scope: "project",
+ })
+ if err == nil {
+ t.Fatal("expected error for empty title, got nil")
+ }
+ if !strings.Contains(err.Error(), "title is required") {
+ t.Errorf("expected 'title is required' in error, got: %v", err)
+ }
+ })
+ }
+}
+
+func TestAddObservationAcceptsNonEmptyTitle(t *testing.T) {
+ s := newTestStore(t)
+ if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
+ t.Fatalf("create session: %v", err)
+ }
+ id, err := s.AddObservation(AddObservationParams{
+ SessionID: "s1",
+ Type: "decision",
+ Title: "Valid title",
+ Content: "some content",
+ Project: "engram",
+ Scope: "project",
+ })
+ if err != nil {
+ t.Fatalf("unexpected error for valid title: %v", err)
+ }
+ if id == 0 {
+ t.Fatal("expected a non-zero observation id")
+ }
+}
+
+// TestAddObservationValidatesPostStripTitle pins the contract that the
+// non-empty check runs on the value *after* stripPrivateTags, which is what
+// actually gets persisted and pushed to the cloud (see issue #459). A title
+// made only of private tags is NOT empty post-strip: stripPrivateTags replaces
+// the tags with "[REDACTED]", so the observation is accepted and persisted with
+// that title. This guards against a refactor that moves validation before the
+// strip and silently changes the persisted value.
+func TestAddObservationValidatesPostStripTitle(t *testing.T) {
+ s := newTestStore(t)
+ if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
+ t.Fatalf("create session: %v", err)
+ }
+ id, err := s.AddObservation(AddObservationParams{
+ SessionID: "s1",
+ Type: "decision",
+ Title: "secret",
+ Content: "some content",
+ Project: "engram",
+ Scope: "project",
+ })
+ if err != nil {
+ t.Fatalf("private-only title strips to a non-empty placeholder; expected accept, got: %v", err)
+ }
+ obs, err := s.GetObservation(id)
+ if err != nil {
+ t.Fatalf("get observation: %v", err)
+ }
+ if obs.Title != "[REDACTED]" {
+ t.Errorf("expected persisted title %q, got %q", "[REDACTED]", obs.Title)
+ }
+}
+
+// TestUpdateObservationRejectsEmptyTitle pins the same invariant as
+// TestAddObservationRejectsEmptyTitle for the update path. mem_update and
+// PATCH /observations/{id} funnel through UpdateObservation; an explicit empty
+// or whitespace-only title must be rejected before it is persisted and enqueued
+// as a sync upsert the cloud server rejects (issue #459). The original title
+// must be left untouched.
+func TestUpdateObservationRejectsEmptyTitle(t *testing.T) {
+ cases := []struct {
+ name string
+ title string
+ }{
+ {"empty string", ""},
+ {"only spaces", " "},
+ {"only tabs and newlines", "\t\n \n"},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ s := newTestStore(t)
+ if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
+ t.Fatalf("create session: %v", err)
+ }
+ id, err := s.AddObservation(AddObservationParams{
+ SessionID: "s1",
+ Type: "decision",
+ Title: "Original title",
+ Content: "some content",
+ Project: "engram",
+ Scope: "project",
+ })
+ if err != nil {
+ t.Fatalf("seed observation: %v", err)
+ }
+
+ _, err = s.UpdateObservation(id, UpdateObservationParams{Title: &tc.title})
+ if err == nil {
+ t.Fatal("expected error for empty title, got nil")
+ }
+ if !errors.Is(err, ErrEmptyObservationTitle) {
+ t.Errorf("expected ErrEmptyObservationTitle, got: %v", err)
+ }
+
+ // The original title must survive the rejected update.
+ obs, err := s.GetObservation(id)
+ if err != nil {
+ t.Fatalf("get observation: %v", err)
+ }
+ if obs.Title != "Original title" {
+ t.Errorf("expected title unchanged %q, got %q", "Original title", obs.Title)
+ }
+ })
+ }
+}
+
+// TestUpdateObservationValidatesPostStripTitle mirrors the AddObservation
+// post-strip contract on the update path: a title made only of private tags
+// strips to "[REDACTED]" (not empty), so the update is accepted and persisted
+// with that placeholder.
+func TestUpdateObservationValidatesPostStripTitle(t *testing.T) {
+ s := newTestStore(t)
+ if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
+ t.Fatalf("create session: %v", err)
+ }
+ id, err := s.AddObservation(AddObservationParams{
+ SessionID: "s1",
+ Type: "decision",
+ Title: "Original title",
+ Content: "some content",
+ Project: "engram",
+ Scope: "project",
+ })
+ if err != nil {
+ t.Fatalf("seed observation: %v", err)
+ }
+
+ newTitle := "secret"
+ if _, err := s.UpdateObservation(id, UpdateObservationParams{Title: &newTitle}); err != nil {
+ t.Fatalf("private-only title strips to a non-empty placeholder; expected accept, got: %v", err)
+ }
+ obs, err := s.GetObservation(id)
+ if err != nil {
+ t.Fatalf("get observation: %v", err)
+ }
+ if obs.Title != "[REDACTED]" {
+ t.Errorf("expected persisted title %q, got %q", "[REDACTED]", obs.Title)
+ }
+}
+
+// TestUpdateObservationAcceptsContentOnlyUpdate guards that the title check does
+// not regress content-only updates: when Title is nil the existing (valid)
+// title is preserved and the update succeeds.
+func TestUpdateObservationAcceptsContentOnlyUpdate(t *testing.T) {
+ s := newTestStore(t)
+ if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
+ t.Fatalf("create session: %v", err)
+ }
+ id, err := s.AddObservation(AddObservationParams{
+ SessionID: "s1",
+ Type: "decision",
+ Title: "Original title",
+ Content: "some content",
+ Project: "engram",
+ Scope: "project",
+ })
+ if err != nil {
+ t.Fatalf("seed observation: %v", err)
+ }
+
+ newContent := "updated content"
+ updated, err := s.UpdateObservation(id, UpdateObservationParams{Content: &newContent})
+ if err != nil {
+ t.Fatalf("content-only update should succeed, got: %v", err)
+ }
+ if updated.Title != "Original title" {
+ t.Errorf("expected title preserved %q, got %q", "Original title", updated.Title)
+ }
+ if updated.Content != newContent {
+ t.Errorf("expected content %q, got %q", newContent, updated.Content)
+ }
+}