diff --git a/apps/journal/app/lib/activities.server.ts b/apps/journal/app/lib/activities.server.ts index fb2f9ae..7705225 100644 --- a/apps/journal/app/lib/activities.server.ts +++ b/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 { 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; diff --git a/apps/journal/app/lib/federation-delivery.server.test.ts b/apps/journal/app/lib/federation-delivery.server.test.ts new file mode 100644 index 0000000..2d000ae --- /dev/null +++ b/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(); + }); +}); diff --git a/apps/journal/app/lib/federation-delivery.server.ts b/apps/journal/app/lib/federation-delivery.server.ts index e4c1f31..56d0f09 100644 --- a/apps/journal/app/lib/federation-delivery.server.ts +++ b/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. */ diff --git a/openspec/changes/social-federation/design.md b/openspec/changes/social-federation/design.md index 15f9bd4..a104644 100644 --- a/openspec/changes/social-federation/design.md +++ b/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