From 0807ec2b7de7f0e994ecce849ecc7cd57e2b532d Mon Sep 17 00:00:00 2001 From: Daniel Rosales <111561081+dnlrsls@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:46:55 -0500 Subject: [PATCH] fix(store): quarantine invalid pulled prompt delete identity --- docs/ARCHITECTURE.md | 2 +- internal/store/store.go | 7 +++++- internal/store/store_test.go | 41 +++++++++++++++++++++++++++++++++--- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b9e830d93..64c87fc40 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -2,7 +2,7 @@ # Architecture -Local `POST /prompts` accepts optional `source_inbox_id` alongside `session_id`, `content`, and `project`. A nonempty ID identifies one prompt within its session: replay returns the existing prompt ID with the same `201` and `{"id":…, "status":"saved"}` response, without another sync mutation or write notification. Distinct IDs may contain identical text. Omitting the ID continues to append a new prompt on every call. Project ownership checks still apply before replay. Prompt sync upserts and exports preserve the optional identity, so replay after sync or import returns the existing prompt without a new mutation. Older payloads without the field remain valid. A pulled prompt upsert cannot move an established nonempty inbox identity to a different session or replace it with another nonempty ID for the same sync ID; it fails with an identity conflict. A missing payload ID retains the existing identity in the same session; a legacy row with no established identity may move sessions or acquire an ID. Import adoption of an inbox identity for the same sync ID refuses cross-project reassignment, comparing canonical effective projects (including session inheritance for blank prompt projects). Deletion tombstones retain optional inbox identity through sync and direct backup. A pulled delete for a live prompt records the live row's session-and-inbox identity, not conflicting identity fields from its payload. Reusing a deleted session-and-inbox ID fails with HTTP `409 Conflict` (no ID, mutation, or write notification), including after restore; a different ID remains a new prompt. Pulled deletes without a live prompt also reject an inbox ID without a nonblank session ID; legacy deletes without an inbox ID remain valid. Sparse pulled deletes inherit project ownership from the live prompt first (which can differ from its session), then the live session or active deleted-session tombstone; project exports recover ownership from an active deleted-session tombstone after that session is removed and emit that resolved project for legacy blank-project tombstones, preserving ownership through backup import. Unscoped exports also emit the resolved project so full backup restores retain that ownership. Backup import rejects tombstones carrying an inbox ID without a session ID; legacy tombstones without an inbox ID remain valid. +Local `POST /prompts` accepts optional `source_inbox_id` alongside `session_id`, `content`, and `project`. A nonempty ID identifies one prompt within its session: replay returns the existing prompt ID with the same `201` and `{"id":…, "status":"saved"}` response, without another sync mutation or write notification. Distinct IDs may contain identical text. Omitting the ID continues to append a new prompt on every call. Project ownership checks still apply before replay. Prompt sync upserts and exports preserve the optional identity, so replay after sync or import returns the existing prompt without a new mutation. Older payloads without the field remain valid. A pulled prompt upsert cannot move an established nonempty inbox identity to a different session or replace it with another nonempty ID for the same sync ID; it fails with an identity conflict. A missing payload ID retains the existing identity in the same session; a legacy row with no established identity may move sessions or acquire an ID. Import adoption of an inbox identity for the same sync ID refuses cross-project reassignment, comparing canonical effective projects (including session inheritance for blank prompt projects). Deletion tombstones retain optional inbox identity through sync and direct backup. A pulled delete for a live prompt records the live row's session-and-inbox identity, not conflicting identity fields from its payload. Reusing a deleted session-and-inbox ID fails with HTTP `409 Conflict` (no ID, mutation, or write notification), including after restore; a different ID remains a new prompt. Pulled deletes without a live prompt quarantine an inbox ID without a nonblank session ID as dead-letter evidence and advance the pull cursor without writing a tombstone; legacy deletes without an inbox ID remain valid. Sparse pulled deletes inherit project ownership from the live prompt first (which can differ from its session), then the live session or active deleted-session tombstone; project exports recover ownership from an active deleted-session tombstone after that session is removed and emit that resolved project for legacy blank-project tombstones, preserving ownership through backup import. Unscoped exports also emit the resolved project so full backup restores retain that ownership. Backup import rejects tombstones carrying an inbox ID without a session ID; legacy tombstones without an inbox ID remain valid. - [How It Works](#how-it-works) - [Session Lifecycle](#session-lifecycle) diff --git a/internal/store/store.go b/internal/store/store.go index 87ec589df..09c6160cd 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -106,6 +106,8 @@ var ( ErrPulledSessionIdentityInvalid = errors.New("pulled session identity is invalid") // ErrPulledObservationIdentityInvalid identifies a pulled observation whose payload and mutation identities disagree. ErrPulledObservationIdentityInvalid = errors.New("pulled observation identity is invalid") + // ErrPulledPromptIdentityInvalid identifies a pulled prompt delete with an unusable keyed identity. + ErrPulledPromptIdentityInvalid = errors.New("pulled prompt identity is invalid") // ErrPulledSessionDirectoryInvalid identifies a pulled or imported session that // has no concrete directory and therefore cannot be admitted as cloud state. ErrPulledSessionDirectoryInvalid = errors.New("pulled session directory is invalid") @@ -352,6 +354,7 @@ const ( SyncSessionIdentityInvalidReasonCode = "sync_session_identity_invalid" SyncObservationIdentityInvalidReasonCode = "sync_observation_identity_invalid" + SyncPromptIdentityInvalidReasonCode = "sync_prompt_identity_invalid" SyncParentSessionMissingReasonCode = "pulled_parent_session_missing" // relationDeferredOuterProjectAuthoritativeReasonCode records that a deferred @@ -10552,6 +10555,8 @@ func pulledIdentityInvalidReasonCode(applyErr error) (string, bool) { return SyncSessionIdentityInvalidReasonCode, true case errors.Is(applyErr, ErrPulledObservationIdentityInvalid): return SyncObservationIdentityInvalidReasonCode, true + case errors.Is(applyErr, ErrPulledPromptIdentityInvalid): + return SyncPromptIdentityInvalidReasonCode, true default: return "", false } @@ -11247,7 +11252,7 @@ func (s *Store) applyPromptDeleteTx(tx *sql.Tx, payload syncPromptPayload) error payload.SourceInboxID = inboxID } if payload.SourceInboxID != "" && strings.TrimSpace(payload.SessionID) == "" { - return fmt.Errorf("delete prompt %q: session id is required for source inbox id", payload.SyncID) + return fmt.Errorf("%w: delete prompt %q: session id is required for source inbox id", ErrPulledPromptIdentityInvalid, payload.SyncID) } if payload.Project == nil || strings.TrimSpace(*payload.Project) == "" { owner := promptProject diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 1385806be..c6eeb70a9 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -1423,13 +1423,13 @@ func TestPulledSparsePromptDeletePrefersLivePromptProject(t *testing.T) { } } -func TestPulledPromptDeleteRejectsInboxWithoutSession(t *testing.T) { +func TestPulledPromptDeleteQuarantinesInboxWithoutSession(t *testing.T) { for _, session := range []string{"", " \t "} { t.Run(fmt.Sprintf("session_%q", session), func(t *testing.T) { s := newTestStore(t) payload := fmt.Sprintf(`{"sync_id":"bad-key","session_id":%q,"source_inbox_id":"inbox","deleted":true}`, session) - if err := s.ApplyPulledMutation(DefaultSyncTargetKey, SyncMutation{Seq: 1, Entity: SyncEntityPrompt, EntityKey: "bad-key", Op: SyncOpDelete, Payload: payload}); err == nil { - t.Fatal("accepted invalid inbox identity") + if err := s.ApplyPulledMutation(DefaultSyncTargetKey, SyncMutation{Seq: 1, Entity: SyncEntityPrompt, EntityKey: "bad-key", Op: SyncOpDelete, Payload: payload}); err != nil { + t.Fatalf("quarantine invalid inbox identity: %v", err) } if got := scalarInt(t, s, `SELECT count(*) FROM prompt_tombstones WHERE sync_id = ?`, "bad-key"); got != 0 { t.Fatalf("persisted invalid tombstone: %d", got) @@ -6350,6 +6350,41 @@ func TestApplyPulledChunkIsAtomicAndRetrySafe(t *testing.T) { } } +func TestApplyPulledPromptDeleteInvalidInboxIdentityQuarantinesAndContinues(t *testing.T) { + for _, session := range []string{"", " \t "} { + t.Run(fmt.Sprintf("session=%q", session), func(t *testing.T) { + s := newTestStore(t) + payload := fmt.Sprintf(`{"sync_id":"bad-prompt","session_id":%q,"source_inbox_id":"inbox","deleted":true}`, session) + invalid := SyncMutation{Seq: 1, Entity: SyncEntityPrompt, EntityKey: "bad-prompt", Op: SyncOpDelete, Payload: payload} + if err := s.ApplyPulledMutation(DefaultSyncTargetKey, invalid); err != nil { + t.Fatalf("invalid pull: %v", err) + } + if got := scalarInt(t, s, `SELECT COUNT(*) FROM prompt_tombstones WHERE sync_id = 'bad-prompt'`); got != 0 { + t.Fatalf("invalid tombstones=%d", got) + } + rows, err := s.ListDeferred(ListDeferredOptions{Status: "dead"}) + if err != nil || len(rows) != 1 || rows[0].PayloadRaw != payload || rows[0].ReasonCode != SyncPromptIdentityInvalidReasonCode || rows[0].RemoteSeq != 1 || rows[0].EntityKey != invalid.EntityKey || rows[0].Op != invalid.Op { + t.Fatalf("dead evidence=%+v, err=%v", rows, err) + } + state, err := s.GetSyncState(DefaultSyncTargetKey) + if err != nil || state.LastPulledSeq != 1 { + t.Fatalf("invalid cursor=%+v, err=%v", state, err) + } + valid := SyncMutation{Seq: 2, Entity: SyncEntityPrompt, EntityKey: "good-prompt", Op: SyncOpDelete, Payload: `{"sync_id":"good-prompt","session_id":"owner","source_inbox_id":"inbox","deleted":true}`} + if err := s.ApplyPulledMutation(DefaultSyncTargetKey, valid); err != nil { + t.Fatalf("valid pull: %v", err) + } + if got := scalarInt(t, s, `SELECT COUNT(*) FROM prompt_tombstones WHERE sync_id = 'good-prompt' AND session_id = 'owner' AND source_inbox_id = 'inbox'`); got != 1 { + t.Fatalf("valid keyed tombstones=%d", got) + } + state, err = s.GetSyncState(DefaultSyncTargetKey) + if err != nil || state.LastPulledSeq != 2 { + t.Fatalf("valid cursor=%+v, err=%v", state, err) + } + }) + } +} + func TestApplyPulledPromptDeleteCreatesTombstoneAndRemovesPrompt(t *testing.T) { s := newTestStore(t) if err := s.CreateSession("s-prompt", "engram", "/tmp/engram"); err != nil {