fix(journal): retract federated activities only on public→non-public
A received Delete is recorded by Mastodon as a permanent tombstone — later Creates for the same URI are silently refused forever. The old code sent Delete on every non-public save 'just in case' (the comment even called over-sending harmless); on the 2026-06-07 soak this tombstoned an unlisted activity's URI before its first real publish, making its later flip to public invisible on the remote with no error anywhere. - visibilityTransitionAction(previous, next): Create on any transition to public (re-publish doubles as back-delivery; remotes dedupe by id), Delete only when leaving public, nothing for non-public→non-public. - updateActivityVisibility reads the previous visibility and acts on the transition. - design.md documents the tombstone permanence + the user-facing consequence (un-publish then re-publish won't resurrect the post on remotes that processed the retraction — same as Mastodon's own delete-and-redraft). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b0bfed4d4a
commit
20be961177
4 changed files with 88 additions and 15 deletions
|
|
@ -6,7 +6,10 @@ import type { Visibility } from "@trails-cool/db/schema/journal";
|
||||||
import { validateGpx, writeGeom } from "./gpx-save.server.ts";
|
import { validateGpx, writeGeom } from "./gpx-save.server.ts";
|
||||||
import type { GpxData } from "./gpx-save.server.ts";
|
import type { GpxData } from "./gpx-save.server.ts";
|
||||||
import { enqueueOptional } from "./boss.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 {
|
export interface ActivityInput {
|
||||||
name: string;
|
name: string;
|
||||||
|
|
@ -26,12 +29,21 @@ export async function updateActivityVisibility(
|
||||||
visibility: Visibility,
|
visibility: Visibility,
|
||||||
): Promise<boolean> {
|
): Promise<boolean> {
|
||||||
const db = getDb();
|
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)
|
.update(activities)
|
||||||
.set({ visibility })
|
.set({ visibility })
|
||||||
.where(and(eq(activities.id, id), eq(activities.ownerId, ownerId)))
|
.where(and(eq(activities.id, id), eq(activities.ownerId, ownerId)));
|
||||||
.returning({ id: activities.id });
|
|
||||||
if (result.length === 0) return false;
|
|
||||||
|
|
||||||
// Notify followers when an activity becomes public. The unique
|
// Notify followers when an activity becomes public. The unique
|
||||||
// (recipient, type, subject_id) partial index makes the fan-out
|
// (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).
|
// followers (only the first transition per activity emits).
|
||||||
if (visibility === "public") {
|
if (visibility === "public") {
|
||||||
await enqueueOptional("notifications-fanout", { activityId: id }, { source: "updateActivityVisibility" });
|
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).
|
// Federation: Create on (re-)publish, Delete(Tombstone) only when the
|
||||||
await enqueueActivityDeliveries(ownerId, id, "create");
|
// activity actually was public before — remotes never saw anything
|
||||||
} else {
|
// else, and an unnecessary Delete poisons the URI forever.
|
||||||
// Was the activity possibly public before? Retract from remotes —
|
const action = visibilityTransitionAction(existing.visibility, visibility);
|
||||||
// a Delete for an object a remote never saw is acknowledged and
|
if (action !== null) {
|
||||||
// dropped, so over-sending here is harmless and avoids tracking
|
await enqueueActivityDeliveries(ownerId, id, action);
|
||||||
// previous visibility (spec 5.6, basic update/delete fan-out).
|
|
||||||
await enqueueActivityDeliveries(ownerId, id, "delete");
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return true;
|
return true;
|
||||||
|
|
|
||||||
28
apps/journal/app/lib/federation-delivery.server.test.ts
Normal file
28
apps/journal/app/lib/federation-delivery.server.test.ts
Normal file
|
|
@ -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();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -12,6 +12,32 @@ import { activityObjectIri } from "./federation-objects.server.ts";
|
||||||
|
|
||||||
export type DeliveryAction = "create" | "delete";
|
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 {
|
export interface DeliveryPayload {
|
||||||
action: DeliveryAction;
|
action: DeliveryAction;
|
||||||
/** Present for `create` — the job re-reads the row at delivery time. */
|
/** Present for `create` — the job re-reads the row at delivery time. */
|
||||||
|
|
|
||||||
|
|
@ -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
|
(task 5.1): Fedify builds collection-level responses (outbox
|
||||||
OrderedCollection) from counter/cursors without consulting the page
|
OrderedCollection) from counter/cursors without consulting the page
|
||||||
dispatcher, so the dispatcher's `null` → 404 contract doesn't cover them.
|
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
|
## Open Questions
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue