Merge pull request #474 from trails-cool/fix/tombstone-aware-retraction

fix(journal): retract federated activities only on public→non-public

apps/journal/app/lib/activities.server.ts

@@ -6,7 +6,10 @@ import type { Visibility } from "@trails-cool/db/schema/journal";
 import { validateGpx, writeGeom } from "./gpx-save.server.ts";
 import type { GpxData } from "./gpx-save.server.ts";
 import { enqueueOptional } from "./boss.server.ts";
-import { enqueueActivityDeliveries } from "./federation-delivery.server.ts";
+import {
+  enqueueActivityDeliveries,
+  visibilityTransitionAction,
+} from "./federation-delivery.server.ts";
 
 export interface ActivityInput {
   name: string;
@@ -26,12 +29,21 @@ export async function updateActivityVisibility(
   visibility: Visibility,
 ): Promise<boolean> {
   const db = getDb();
-  const result = await db
+  // Read the previous visibility first: the federation action depends on
+  // the *transition*, not the new value alone (a gratuitous Delete
+  // permanently tombstones the object's URI on Mastodon — see
+  // visibilityTransitionAction).
+  const [existing] = await db
+    .select({ visibility: activities.visibility })
+    .from(activities)
+    .where(and(eq(activities.id, id), eq(activities.ownerId, ownerId)))
+    .limit(1);
+  if (!existing) return false;
+
+  await db
     .update(activities)
     .set({ visibility })
-    .where(and(eq(activities.id, id), eq(activities.ownerId, ownerId)))
-    .returning({ id: activities.id });
-  if (result.length === 0) return false;
+    .where(and(eq(activities.id, id), eq(activities.ownerId, ownerId)));
 
   // Notify followers when an activity becomes public. The unique
   // (recipient, type, subject_id) partial index makes the fan-out
@@ -39,16 +51,14 @@ export async function updateActivityVisibility(
   // followers (only the first transition per activity emits).
   if (visibility === "public") {
     await enqueueOptional("notifications-fanout", { activityId: id }, { source: "updateActivityVisibility" });
-    // Federation: newly-public activities push to remote followers as
-    // Create(Note) (spec 5.3/5.6 — publish-on-flip is the "update"
-    // remotes care about; the delivery job re-checks visibility).
-    await enqueueActivityDeliveries(ownerId, id, "create");
-  } else {
-    // Was the activity possibly public before? Retract from remotes —
-    // a Delete for an object a remote never saw is acknowledged and
-    // dropped, so over-sending here is harmless and avoids tracking
-    // previous visibility (spec 5.6, basic update/delete fan-out).
-    await enqueueActivityDeliveries(ownerId, id, "delete");
+  }
+
+  // Federation: Create on (re-)publish, Delete(Tombstone) only when the
+  // activity actually was public before — remotes never saw anything
+  // else, and an unnecessary Delete poisons the URI forever.
+  const action = visibilityTransitionAction(existing.visibility, visibility);
+  if (action !== null) {
+    await enqueueActivityDeliveries(ownerId, id, action);
   }
 
   return true;

apps/journal/app/lib/federation-delivery.server.test.ts

@@ -0,0 +1,28 @@
+import { describe, it, expect, vi } from "vitest";
+
+// The db/boss imports in the module under test are irrelevant for the
+// pure transition helper; stub them so importing the module is cheap.
+vi.mock("./db.ts", () => ({ getDb: () => ({}) }));
+vi.mock("./boss.server.ts", () => ({ enqueueOptional: vi.fn() }));
+
+const { visibilityTransitionAction } = await import("./federation-delivery.server.ts");
+
+describe("visibilityTransitionAction", () => {
+  it("publishes on any transition to public (including re-publish)", () => {
+    expect(visibilityTransitionAction("private", "public")).toBe("create");
+    expect(visibilityTransitionAction("unlisted", "public")).toBe("create");
+    expect(visibilityTransitionAction("public", "public")).toBe("create");
+  });
+
+  it("retracts only when leaving public", () => {
+    expect(visibilityTransitionAction("public", "private")).toBe("delete");
+    expect(visibilityTransitionAction("public", "unlisted")).toBe("delete");
+  });
+
+  it("stays silent for non-public to non-public — a Delete would tombstone the URI", () => {
+    expect(visibilityTransitionAction("private", "unlisted")).toBeNull();
+    expect(visibilityTransitionAction("unlisted", "private")).toBeNull();
+    expect(visibilityTransitionAction("private", "private")).toBeNull();
+    expect(visibilityTransitionAction("unlisted", "unlisted")).toBeNull();
+  });
+});

apps/journal/app/lib/federation-delivery.server.ts

@@ -12,6 +12,32 @@ import { activityObjectIri } from "./federation-objects.server.ts";
 
 export type DeliveryAction = "create" | "delete";
 
+/**
+ * Which federation action (if any) a visibility change warrants.
+ *
+ * The asymmetry matters: Mastodon records a `Delete` as a permanent
+ * tombstone — a later `Create` for the same URI is silently refused
+ * forever. So a retraction must only go out when remotes could
+ * actually have the object (it was public), never "just in case"
+ * (learned on the 2026-06-07 staging soak, where an unlisted
+ * activity's no-op save tombstoned its URI before its first real
+ * publish).
+ *
+ * - → public: push Create. Also for public→public — re-pushing is
+ *   harmless (remotes dedupe by id) and doubles as back-delivery.
+ * - public → non-public: push Delete(Tombstone) — remotes saw it.
+ * - non-public → non-public: nothing — remotes never had it, and a
+ *   Delete would poison the URI for any future publish.
+ */
+export function visibilityTransitionAction(
+  previous: string,
+  next: string,
+): DeliveryAction | null {
+  if (next === "public") return "create";
+  if (previous === "public") return "delete";
+  return null;
+}
+
 export interface DeliveryPayload {
   action: DeliveryAction;
   /** Present for `create` — the job re-reads the row at delivery time. */

openspec/changes/social-federation/design.md

@@ -135,6 +135,15 @@ Inbound signature verification uses the actor's public key from their actor obje
   (task 5.1): Fedify builds collection-level responses (outbox
   OrderedCollection) from counter/cursors without consulting the page
   dispatcher, so the dispatcher's `null` → 404 contract doesn't cover them.
+- **Retractions are transition-aware; tombstones are forever** (learned on
+  the 2026-06-07 soak). Mastodon records a received `Delete` as a permanent
+  tombstone for that URI — later `Create`s for the same URI are silently
+  refused. Push delivery therefore sends `Delete` only on an actual
+  public→non-public transition (`visibilityTransitionAction`), never "just
+  in case". Consequence worth surfacing user-facing eventually: if a user
+  un-publishes a federated activity and re-publishes it later, remotes that
+  processed the retraction will not show it again — same semantics as
+  Mastodon's own delete-and-redraft.
 
 ## Open Questions