oficina / memoria Public

B10 review: pin the edited replay, refuse uuid.Nil, amend the spec

Fable's review of #14:
1. The "an edited retry revises the same note" claim was untested; a
   title-in-key mutation stayed green. New subtest: same client_id with a
   changed title and body returns the same id, stores nothing new, bumps
   the revision by one. Red under key = client_id + "-" + title.
2. uuid.Nil parses and is 36 characters, so a client shipping a zero UUID
   would fold every note into one. Refused with 400; red when the check
   is removed.
3. Spec §"Native Apple clients": the note key is now client-chosen per
   draft, and the retry/edit semantics are written down.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 50544853560518ef7ec72ef35b012bfa27ede7a5 parent 3358baf
3 files changed, +40 −5
  • docs/superpowers/specs/2026-09-03-memoria-design.md +6 −1
  • internal/memoria/native.go +5 −2
  • internal/memoria/native_test.go +29 −2

CI passed — run details

CI log
=== vet ===
go vet ./...
=== fmt ===
=== test ===
go test ./...
?   	amadan.net/oficina/memoria/cmd/memoria	[no test files]
ok  	amadan.net/oficina/memoria/internal/memoria	0.701s
ok  	amadan.net/oficina/memoria/internal/memoria/chunk	0.003s
?   	amadan.net/oficina/memoria/internal/memoria/icons	[no test files]
ok  	amadan.net/oficina/memoria/internal/memoria/index	0.031s
ok  	amadan.net/oficina/memoria/internal/memoria/notion	0.016s
ok  	amadan.net/oficina/memoria/internal/memoria/query	0.003s
ok  	amadan.net/oficina/memoria/internal/memoria/seal	0.002s
ok  	amadan.net/oficina/memoria/internal/memoria/sibling	3.018s
ok  	amadan.net/oficina/memoria/internal/memoriatest	1.310s
?   	amadan.net/oficina/memoria/tools/bench	[no test files]
?   	amadan.net/oficina/memoria/tools/drive	[no test files]
?   	amadan.net/oficina/memoria/tools/seed	[no test files]
?   	amadan.net/oficina/memoria/tools/vendorassets	[no test files]
=== migration-check ===
go run github.com/carlosframework/rastrillo/cmd/rastrillo migration check
rastrillo migration check: models and migrations agree
diff --git a/docs/superpowers/specs/2026-09-03-memoria-design.md b/docs/superpowers/specs/2026-09-03-memoria-design.md
index 06010b5..5efccc2 100644
--- a/docs/superpowers/specs/2026-09-03-memoria-design.md
+++ b/docs/superpowers/specs/2026-09-03-memoria-design.md
@@ -4059,7 +4059,12 @@ sibling authorization rule is introduced. Only filtered result DTOs are returned
with highlight markers removed for native text. Requests/responses are not cached.
Native note creation uses POST `/api/native/notes` and the existing Ingest store,
-with an opaque unique source key so two same-title notes never replace each other.
+keyed by a `client_id` the client mints once per draft (a canonical, non-nil
+UUID; the server mints one only for an older client that sends none). Two
+same-title notes therefore never replace each other, and a Save retried after
+a lost response replays the same draft: Ingest's same-key-same-body path
+returns the first id, and an edited retry revises that one note rather than
+adding a second (B10, 2026-09-23).
This resolves the independent review finding that the web paste endpoint upserts
by title. Notes retain the existing team-sharing policy. The local model is an optional, explicit
Apple Foundation Models call on a supported device, never an external provider.
diff --git a/internal/memoria/native.go b/internal/memoria/native.go
index 35b0bcd..0aaffa5 100644
--- a/internal/memoria/native.go
+++ b/internal/memoria/native.go
@@ -90,7 +90,7 @@ func (a *app) nativeSearch(w http.ResponseWriter, r *http.Request) {
// that note rather than adding another beside it, which is the point: it
// is still one draft.
//
-// It must be a UUID, and it is canonicalised before use, so "A1…" and
+// It must be a canonical, non-nil UUID, and it is canonicalised before use, so "A1…" and
// "a1…" are one key and a free-form string cannot choose its own key.
// Absent, the server mints one as before, so older clients keep working
// — and keep the old duplicate-on-retry behaviour.
@@ -110,7 +110,10 @@ func (a *app) nativeNote(w http.ResponseWriter, r *http.Request) {
key := uuid.NewString()
if in.ClientID != "" {
parsed, err := uuid.Parse(in.ClientID)
- if err != nil || len(in.ClientID) != 36 {
+ // uuid.Nil parses and is 36 characters, but it is the value a
+ // struct default or a failed decode produces: every such client
+ // would converge on one note that revises forever.
+ if err != nil || len(in.ClientID) != 36 || parsed == uuid.Nil {
http.Error(w, "The note's client id is not a UUID", 400)
return
}
diff --git a/internal/memoria/native_test.go b/internal/memoria/native_test.go
index 2a5f0c3..49c6111 100644
--- a/internal/memoria/native_test.go
+++ b/internal/memoria/native_test.go
@@ -214,7 +214,9 @@ func TestNativeNotesSameTitleCreatesDistinctDocuments(t *testing.T) {
// stored. Mutations verified red 2026-09-23: keying on a server-minted
// uuid again (the old behaviour) -> replay returns a second id; keying on
// the raw client_id -> the uppercase replay makes a second note; dropping
-// the 36-character check -> braced and urn: forms are accepted.
+// the 36-character check -> braced and urn: forms are accepted; the
+// title folded into the key (Fable's M1) -> the edited replay makes a
+// second note; the uuid.Nil refusal removed -> the zero UUID is accepted.
func TestNativeNoteReplayWithSameClientIDIsANoOp(t *testing.T) {
f := newHomeRouterFixture(t)
f.local(t)
@@ -276,7 +278,32 @@ func TestNativeNoteReplayWithSameClientIDIsANoOp(t *testing.T) {
t.Fatalf("no client_id: ids %d and %d, want two distinct notes", l1, l2)
}
- for _, bad := range []string{"not-a-uuid", "{" + draft + "}", "urn:uuid:" + draft, strings.ReplaceAll(draft, "-", "")} {
+ // The other half of B10: the phone lost the response AND the person
+ // then fixed a typo before pressing Save again. Still one draft, so
+ // still one note — revised, not duplicated.
+ t.Run("an edited replay revises the same note", func(t *testing.T) {
+ revision := func(id int64) int {
+ t.Helper()
+ var d Document
+ if err := f.app.db.G.First(&d, id).Error; err != nil {
+ t.Fatal(err)
+ }
+ return d.Revision
+ }
+ was, count := revision(first), documents()
+ edited := map[string]string{"title": "Standup (fixed)", "body": "replayed nebula content, typo fixed", "client_id": draft}
+ if code, id := post(edited); code != 200 || id != first {
+ t.Fatalf("edited replay: status %d id %d, want the first id %d", code, id, first)
+ }
+ if n := documents(); n != count {
+ t.Fatalf("edited replay stored %d more document(s)", n-count)
+ }
+ if now := revision(first); now != was+1 {
+ t.Fatalf("revision %d -> %d, want a bump of one", was, now)
+ }
+ })
+
+ for _, bad := range []string{"00000000-0000-0000-0000-000000000000", "not-a-uuid", "{" + draft + "}", "urn:uuid:" + draft, strings.ReplaceAll(draft, "-", "")} {
if code, _ := post(map[string]string{"title": "x", "body": "y", "client_id": bad}); code != 400 {
t.Errorf("client_id %q: status %d, want 400", bad, code)
}