diff --git a/apps/web/src/components/shows/ShowLayout.tsx b/apps/web/src/components/shows/ShowLayout.tsx index a409eed..37fd367 100644 --- a/apps/web/src/components/shows/ShowLayout.tsx +++ b/apps/web/src/components/shows/ShowLayout.tsx @@ -36,6 +36,7 @@ import type { ChatChannelId, ShowId, SongId } from "@showtime/contracts"; import { useAtomValue } from "@effect/atom-react"; import { songAtoms } from "@/client"; import { useCreateSong } from "@/components/songs/useCreateSong"; +import { createdSongHandoff } from "@/components/songs/CreatedSongHandoff"; import { ShowPageAction } from "./ShowPageAction"; import { ProfileSwitcher } from "@/components/profiles/ProfileSwitcher"; import { ChatDrawer, ChatUnreadBadge } from "@/components/chats/ChatDrawer"; @@ -53,6 +54,7 @@ export function ShowLayout() { const params = useParams({ strict: false }); const currentSongId = typeof params.songId === "string" ? (params.songId as SongId) : undefined; const songsResult = useAtomValue(songAtoms(typedShowId).songs); + const syncedSongsResult = useAtomValue(songAtoms(typedShowId).syncedSongs); const pathname = useRouterState({ select: (state) => state.location.pathname }); const isAllSongsRoute = /\/setlist\/?$/.test(pathname); const songs = AsyncResult.isSuccess(songsResult) @@ -60,6 +62,11 @@ export function ShowLayout() { : AsyncResult.isFailure(songsResult) ? (Option.getOrUndefined(songsResult.previousSuccess)?.value ?? []) : []; + React.useEffect(() => { + if (AsyncResult.isSuccess(syncedSongsResult)) { + createdSongHandoff.reconcile(typedShowId, syncedSongsResult.value, syncedSongsResult); + } + }, [syncedSongsResult, typedShowId]); const songCreator = useCreateSong(typedShowId, currentSongId); return ( diff --git a/apps/web/src/components/songs/CreatedSongHandoff.test.ts b/apps/web/src/components/songs/CreatedSongHandoff.test.ts new file mode 100644 index 0000000..84ec195 --- /dev/null +++ b/apps/web/src/components/songs/CreatedSongHandoff.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vite-plus/test"; +import { Schema } from "effect"; +import { Song, SongName, type ShowId, type SongId } from "@showtime/contracts"; +import { makeCreatedSongHandoff } from "./CreatedSongHandoff"; + +const showId = "show_0123456789abcdef" as ShowId; +const song = Schema.decodeUnknownSync(Song)({ + id: "song_0123456789abcdef" as SongId, + name: "", + artist: "", + mixAssignments: [], + createdAt: "2026-07-24T12:00:00.000Z", + updatedAt: "2026-07-24T12:00:00.000Z", +}); + +describe("CreatedSongHandoff", () => { + it("retains a confirmed creation until the streamed snapshot observes it", () => { + const handoff = makeCreatedSongHandoff(); + const baselineSnapshot = {}; + + handoff.remember(showId, song, undefined, baselineSnapshot); + expect(handoff.find(showId, song.id)).toBe(song); + + handoff.reconcile(showId, [], baselineSnapshot); + expect(handoff.find(showId, song.id)).toBe(song); + + handoff.reconcile(showId, [{ ...song, pending: true }], {}); + expect(handoff.find(showId, song.id)).toBe(song); + + handoff.reconcile(showId, [song], {}); + expect(handoff.find(showId, song.id)).toBeUndefined(); + }); + + it("can forget a creation after it is deleted before reconciliation", () => { + const handoff = makeCreatedSongHandoff(); + + handoff.remember(showId, song, undefined, {}); + handoff.forget(showId, song.id); + + expect(handoff.find(showId, song.id)).toBeUndefined(); + }); + + it("preserves the intended insertion position while the snapshot is delayed", () => { + const handoff = makeCreatedSongHandoff(); + const anchor = { + ...song, + id: "song_fedcba9876543210" as SongId, + name: SongName.make("Anchor"), + }; + + handoff.remember(showId, song, anchor.id, {}); + + expect(handoff.provisionalNumber(showId, song.id, [anchor])).toBe(2); + }); + + it("expires a confirmed creation when a newer authoritative snapshot omits it", () => { + const handoff = makeCreatedSongHandoff(); + const baselineSnapshot = {}; + + handoff.remember(showId, song, undefined, baselineSnapshot); + handoff.reconcile(showId, [], {}); + + expect(handoff.find(showId, song.id)).toBeUndefined(); + }); +}); diff --git a/apps/web/src/components/songs/CreatedSongHandoff.ts b/apps/web/src/components/songs/CreatedSongHandoff.ts new file mode 100644 index 0000000..5921270 --- /dev/null +++ b/apps/web/src/components/songs/CreatedSongHandoff.ts @@ -0,0 +1,63 @@ +import type { ShowId, Song, SongId } from "@showtime/contracts"; + +const keyFor = (showId: ShowId, songId: SongId) => `${showId}:${songId}`; + +export const makeCreatedSongHandoff = () => { + const confirmed = new Map< + string, + { + readonly song: Song; + readonly insertAfterSongId?: SongId; + readonly baselineSnapshot: object; + } + >(); + + const remember = ( + showId: ShowId, + song: Song, + insertAfterSongId: SongId | undefined, + baselineSnapshot: object, + ) => { + confirmed.set(keyFor(showId, song.id), { song, insertAfterSongId, baselineSnapshot }); + }; + + const find = (showId: ShowId, songId: SongId) => confirmed.get(keyFor(showId, songId))?.song; + + const provisionalNumber = (showId: ShowId, songId: SongId, syncedSongs: ReadonlyArray) => { + const entry = confirmed.get(keyFor(showId, songId)); + if (!entry?.insertAfterSongId) return syncedSongs.length + 1; + const anchorIndex = syncedSongs.findIndex((song) => song.id === entry.insertAfterSongId); + return anchorIndex < 0 ? syncedSongs.length + 1 : anchorIndex + 2; + }; + + const forget = (showId: ShowId, songId: SongId) => { + confirmed.delete(keyFor(showId, songId)); + }; + + const reconcile = ( + showId: ShowId, + songs: ReadonlyArray, + snapshot: object, + ) => { + if (songs.some((song) => song.pending)) return; + + const syncedIds = new Set(songs.map((song) => song.id)); + for (const [key, entry] of confirmed) { + if ( + key.startsWith(`${showId}:`) && + (syncedIds.has(entry.song.id) || snapshot !== entry.baselineSnapshot) + ) { + confirmed.delete(key); + } + } + }; + + return { remember, find, forget, provisionalNumber, reconcile } as const; +}; + +/** + * Bridges the short interval between a successful create response and the next + * full songs snapshot. Entries are discarded as soon as that snapshot observes + * them, so later deletions cannot be hidden by stale client state. + */ +export const createdSongHandoff = makeCreatedSongHandoff(); diff --git a/apps/web/src/components/songs/useCreateSong.ts b/apps/web/src/components/songs/useCreateSong.ts index a44f7ac..7513cc4 100644 --- a/apps/web/src/components/songs/useCreateSong.ts +++ b/apps/web/src/components/songs/useCreateSong.ts @@ -1,23 +1,35 @@ import * as React from "react"; import { Exit } from "effect"; import { useNavigate } from "@tanstack/react-router"; -import { type ShowId, type SongArtist, type SongId, type SongName } from "@showtime/contracts"; -import { useAtomSet } from "@effect/atom-react"; +import { + makeClientId, + songIdPrefix, + type ShowId, + type SongArtist, + type SongId, + type SongName, +} from "@showtime/contracts"; +import { useAtomSet, useAtomValue } from "@effect/atom-react"; import { rpcErrorMessageFromCause, songAtoms, songsRpcReactivityKey } from "@/client"; +import { createdSongHandoff } from "./CreatedSongHandoff"; export function useCreateSong(showId: ShowId, insertAfterSongId?: SongId) { const navigate = useNavigate(); + const songsSnapshot = useAtomValue(songAtoms(showId).syncedSongs); const create = useAtomSet(songAtoms(showId).create, { mode: "promiseExit" }); const [isCreating, setIsCreating] = React.useState(false); const [error, setError] = React.useState(); const createSong = async () => { if (isCreating) return; + const baselineSnapshot = songsSnapshot; setIsCreating(true); setError(undefined); + const id = makeClientId(songIdPrefix) as SongId; const result = await create({ payload: { showId, + id, name: "" as SongName, artist: "" as SongArtist, insertAfterSongId, @@ -29,10 +41,15 @@ export function useCreateSong(showId: ShowId, insertAfterSongId?: SongId) { setIsCreating(false); return; } - await navigate({ - to: "/shows/$showId/setlist/$songId", - params: { showId, songId: result.value.id }, - }); + createdSongHandoff.remember(showId, result.value, insertAfterSongId, baselineSnapshot); + try { + await navigate({ + to: "/shows/$showId/setlist/$songId", + params: { showId, songId: result.value.id }, + }); + } catch { + setError("The song was added, but its page could not be opened. Open it from the setlist."); + } setIsCreating(false); }; diff --git a/apps/web/src/routes/shows/$showId/setlist/$songId.tsx b/apps/web/src/routes/shows/$showId/setlist/$songId.tsx index 558b641..721dca4 100644 --- a/apps/web/src/routes/shows/$showId/setlist/$songId.tsx +++ b/apps/web/src/routes/shows/$showId/setlist/$songId.tsx @@ -1,7 +1,7 @@ import * as React from "react"; import { createFileRoute, useNavigate } from "@tanstack/react-router"; import { AsyncResult } from "effect/unstable/reactivity"; -import { Exit } from "effect"; +import { Exit, Option } from "effect"; import { AlertCircleIcon, EllipsisIcon, MusicIcon, Trash2Icon } from "lucide-react"; import { mainMixId, @@ -51,7 +51,8 @@ import { useAtomSet, useAtomValue } from "@effect/atom-react"; import { mixAtoms } from "@/client"; import { microphoneAtoms } from "@/client"; import { songAtoms, songsRpcReactivityKey } from "@/client"; -import { rpcErrorMessageFromCause } from "@/client"; +import { asyncResultValueOrElse, rpcErrorMessageFromCause } from "@/client"; +import { createdSongHandoff } from "@/components/songs/CreatedSongHandoff"; export const Route = createFileRoute("/shows/$showId/setlist/$songId")({ component: RouteComponent, @@ -63,13 +64,19 @@ function RouteComponent() { const songsResult = useAtomValue(songAtoms(typedShowId).songs); const mixesResult = useAtomValue(mixAtoms(typedShowId).mixes); const microphonesResult = useAtomValue(microphoneAtoms(typedShowId).microphones); - const songs = AsyncResult.getOrElse(songsResult, () => []); + const songs = asyncResultValueOrElse(songsResult, () => []); const songIndex = songs.findIndex((song) => song.id === songId); - const song = songs[songIndex]; - const mixes = AsyncResult.isSuccess(mixesResult) ? mixesResult.value : []; - const microphones = AsyncResult.isSuccess(microphonesResult) ? microphonesResult.value : []; + const song = songs[songIndex] ?? createdSongHandoff.find(typedShowId, songId as SongId); + const songNumber = + songIndex >= 0 + ? songIndex + 1 + : createdSongHandoff.provisionalNumber(typedShowId, songId as SongId, songs); + const mixes = asyncResultValueOrElse(mixesResult, () => []); + const microphones = asyncResultValueOrElse(microphonesResult, () => []); + const hasMixes = Option.isSome(AsyncResult.value(mixesResult)); + const hasMicrophones = Option.isSome(AsyncResult.value(microphonesResult)); - if (AsyncResult.isInitial(songsResult)) { + if (AsyncResult.isInitial(songsResult) && !song) { return ( @@ -81,7 +88,7 @@ function RouteComponent() { ); } - if (AsyncResult.isFailure(songsResult) && songs.length === 0) { + if (AsyncResult.isFailure(songsResult) && songs.length === 0 && !song) { return ( @@ -107,12 +114,28 @@ function RouteComponent() { ); } + if (!hasMixes || !hasMicrophones) { + const failed = + (AsyncResult.isFailure(mixesResult) && !hasMixes) || + (AsyncResult.isFailure(microphonesResult) && !hasMicrophones); + return ( + + + {failed ? : } + + {failed ? "Song details could not be loaded" : "Loading song details"} + + {failed && Check your connection and try again.} + + + ); + } return ( @@ -548,6 +571,7 @@ function SongDeleteDialog({ setIsDeleting(false); return; } + createdSongHandoff.forget(showId, songId); onOpenChange(false); void navigate({ to: "/shows/$showId/setlist", params: { showId } }); }; diff --git a/packages/backend/src/rpc/Rpc.ts b/packages/backend/src/rpc/Rpc.ts index 3596468..fd9454c 100644 --- a/packages/backend/src/rpc/Rpc.ts +++ b/packages/backend/src/rpc/Rpc.ts @@ -122,10 +122,10 @@ const handlers = ShowtimeRpcs.toLayer( "mixes.delete": ({ showId, id }) => sync.mutation(mixesSyncKey(showId), mixes.delete({ showId, id })), "songs.list": ({ showId }) => sync.query(songsSyncKey(showId), songs.list(showId)), - "songs.create": ({ showId, name, artist, insertAfterSongId }) => + "songs.create": ({ showId, id, name, artist, insertAfterSongId }) => sync.mutation( songsSyncKey(showId), - songs.create({ showId, name, artist, insertAfterSongId }), + songs.create({ showId, id, name, artist, insertAfterSongId }), ), "songs.edit": ({ showId, id, name, artist, notes, mixAssignments, microphoneNames }) => sync.mutation( diff --git a/packages/backend/src/songs/SongService.test.ts b/packages/backend/src/songs/SongService.test.ts index 396dff2..97bda2e 100644 --- a/packages/backend/src/songs/SongService.test.ts +++ b/packages/backend/src/songs/SongService.test.ts @@ -4,7 +4,7 @@ import { mkdtemp, rm } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vite-plus/test"; -import type { SongArtist, SongName } from "@showtime/contracts"; +import type { SongArtist, SongId, SongName } from "@showtime/contracts"; import * as Ids from "../ids/Ids.js"; import * as ShowDiscovery from "../shows/ShowDiscovery.js"; import * as ShowFile from "../shows/ShowFile.js"; @@ -130,6 +130,25 @@ describe("SongService", () => { expect(result.map((song) => song.name)).toEqual(["First", "Inserted", "Second"]); }); + it("returns the existing song when a client-generated create request is retried", async () => { + const home = await mkdtemp(path.join(os.tmpdir(), "showtime-home-")); + tempHomes.add(home); + const id = "song_0123456789abcdef" as SongId; + const result = await Effect.runPromise( + Effect.gen(function* () { + const shows = yield* ShowService; + const songs = yield* SongService; + const show = yield* shows.create({ name: "Festival", color: "sky" }); + const first = yield* songs.create({ showId: show.id, id, ...songInput("First") }); + const retry = yield* songs.create({ showId: show.id, id, ...songInput("First") }); + return { first, retry, listed: yield* songs.list(show.id) }; + }).pipe(Effect.provide(makeLayer(home))), + ); + + expect(result.retry).toEqual(result.first); + expect(result.listed).toEqual([result.first]); + }); + it("stores a song-specific microphone name and removes it when it matches the inherited name", async () => { const home = await mkdtemp(path.join(os.tmpdir(), "showtime-home-")); tempHomes.add(home); diff --git a/packages/backend/src/songs/SongService.ts b/packages/backend/src/songs/SongService.ts index 386b4de..95d5e0f 100644 --- a/packages/backend/src/songs/SongService.ts +++ b/packages/backend/src/songs/SongService.ts @@ -19,6 +19,7 @@ interface SongServiceShape { readonly list: (showId: ShowId) => Effect.Effect, RpcError>; readonly create: (params: { readonly showId: ShowId; + readonly id?: SongId; readonly name: SongName; readonly artist: SongArtist; readonly insertAfterSongId?: SongId; @@ -59,7 +60,7 @@ const make = Effect.fnUntraced(function* () { }); const create: SongServiceShape["create"] = Effect.fnUntraced(function* (params) { - const id = yield* ids.makeSongId; + const id = params.id ?? (yield* ids.makeSongId); const name = yield* decodeSongName(params.name.trim()).pipe( Effect.mapError(toRpcError("Invalid song name.")), ); @@ -75,8 +76,14 @@ const make = Effect.fnUntraced(function* () { createdAt: now, updatedAt: now, }; - yield* repository + const { document: updated } = yield* repository .update(params.showId, (document) => { + // The client owns the ID for new create requests. If a response is lost + // and the RPC is retried after persistence, return the original song + // instead of inserting a duplicate. + if (document.songs.some((item) => item.id === id && item.deletedAt === undefined)) { + return document; + } if ( params.insertAfterSongId !== undefined && !document.songs.some( @@ -90,7 +97,7 @@ const make = Effect.fnUntraced(function* () { return { ...document, songs: songs.value }; }) .pipe(Effect.mapError(toRpcError("Could not add song."))); - return song; + return updated.songs.find((item) => item.id === id && item.deletedAt === undefined)!; }); const edit: SongServiceShape["edit"] = Effect.fnUntraced(function* (params) { diff --git a/packages/contracts/src/ids.test.ts b/packages/contracts/src/ids.test.ts index 50bac00..f9d43fc 100644 --- a/packages/contracts/src/ids.test.ts +++ b/packages/contracts/src/ids.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vite-plus/test"; import { Schema } from "effect"; -import { idSuffixLength, makeTemporaryId } from "./ids.js"; +import { idSuffixLength, makeClientId, makeTemporaryId } from "./ids.js"; import { ShowId } from "./show.js"; const decode = Schema.decodeUnknownSync(ShowId); @@ -32,3 +32,11 @@ describe("makeTemporaryId", () => { expect(id).toMatch(new RegExp(`^pending_[0-9a-z]{${idSuffixLength}}$`)); }); }); + +describe("makeClientId", () => { + it("generates stable-format IDs suitable for create request payloads", () => { + const id = makeClientId("song_"); + + expect(id).toMatch(new RegExp(`^song_[0-9a-z]{${idSuffixLength}}$`)); + }); +}); diff --git a/packages/contracts/src/ids.ts b/packages/contracts/src/ids.ts index 721fa65..579cd63 100644 --- a/packages/contracts/src/ids.ts +++ b/packages/contracts/src/ids.ts @@ -9,3 +9,23 @@ export const makeTemporaryId = (prefix: Prefix): `${Prefi return `${prefix}${suffix}`; }; + +/** + * Generates a stable entity ID on the client so retried create requests can be + * made idempotent. `crypto.getRandomValues` is available in supported browsers + * and Node; the fallback keeps local/non-secure development environments usable. + */ +export const makeClientId = (prefix: Prefix): `${Prefix}${string}` => { + const randomValues = globalThis.crypto?.getRandomValues(new Uint32Array(idSuffixLength)); + const suffix = Array.from( + { length: idSuffixLength }, + (_, index) => + idAlphabet[ + randomValues + ? randomValues[index]! % idAlphabet.length + : Math.floor(Math.random() * idAlphabet.length) + ], + ).join(""); + + return `${prefix}${suffix}`; +}; diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 6965ca9..3f107cd 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -218,6 +218,9 @@ export const ShowtimeRpcs = EffectRpcGroup.make( Rpc.make("songs.create", { payload: { showId: ShowId, + // Optional so a newly updated backend remains compatible with clients + // that were loaded before client-generated IDs were introduced. + id: Schema.optional(SongId), name: SongName, artist: SongArtist, insertAfterSongId: Schema.optional(SongId), diff --git a/packages/frontend/src/react/AsyncResult.test.ts b/packages/frontend/src/react/AsyncResult.test.ts index d9582d0..683f8a1 100644 --- a/packages/frontend/src/react/AsyncResult.test.ts +++ b/packages/frontend/src/react/AsyncResult.test.ts @@ -1,7 +1,7 @@ import { Option } from "effect"; import { AsyncResult } from "effect/unstable/reactivity"; import { describe, expect, it } from "vite-plus/test"; -import { isFailureWithoutValue } from "./AsyncResult.js"; +import { asyncResultValueOrElse, isFailureWithoutValue } from "./AsyncResult.js"; describe("isFailureWithoutValue", () => { it("keeps a failed refresh available when it retained an empty value", () => { @@ -22,3 +22,20 @@ describe("isFailureWithoutValue", () => { expect(isFailureWithoutValue(AsyncResult.initial())).toBe(false); }); }); + +describe("asyncResultValueOrElse", () => { + it("uses the previous success while a refresh is failed", () => { + const previousSuccess = AsyncResult.success, string>(["available"]); + const result = AsyncResult.fail>("offline", { + previousSuccess: Option.some(previousSuccess), + }); + + expect(asyncResultValueOrElse(result, () => [])).toEqual(["available"]); + }); + + it("uses the fallback before any value is available", () => { + expect( + asyncResultValueOrElse(AsyncResult.initial, string>(), () => []), + ).toEqual([]); + }); +}); diff --git a/packages/frontend/src/react/AsyncResult.ts b/packages/frontend/src/react/AsyncResult.ts index caf853c..2cdb054 100644 --- a/packages/frontend/src/react/AsyncResult.ts +++ b/packages/frontend/src/react/AsyncResult.ts @@ -4,3 +4,10 @@ import { AsyncResult } from "effect/unstable/reactivity"; export function isFailureWithoutValue(result: AsyncResult.AsyncResult): boolean { return AsyncResult.isFailure(result) && Option.isNone(AsyncResult.value(result)); } + +export function asyncResultValueOrElse( + result: AsyncResult.AsyncResult, + orElse: () => A, +): A { + return Option.getOrElse(AsyncResult.value(result), orElse); +} diff --git a/packages/frontend/src/songs/SongAtoms.ts b/packages/frontend/src/songs/SongAtoms.ts index 849e299..76ec9fa 100644 --- a/packages/frontend/src/songs/SongAtoms.ts +++ b/packages/frontend/src/songs/SongAtoms.ts @@ -31,7 +31,7 @@ export const makeSongAtoms = (RpcClient: ShowtimeRpcClient, options?: StreamingR if (!AsyncResult.isSuccess(current)) return current; const now = DateTime.nowUnsafe(); const song: SongListItem = { - id: makeTemporarySongId(), + id: input.payload.id ?? makeTemporarySongId(), name: input.payload.name.trim() as Song["name"], artist: input.payload.artist.trim() as Song["artist"], mixAssignments: [], @@ -96,7 +96,7 @@ export const makeSongAtoms = (RpcClient: ShowtimeRpcClient, options?: StreamingR fn: reorderSongsMutation, }), ); - return { songs, create, edit, delete: deleteSong, reorder } as const; + return { syncedSongs: query, songs, create, edit, delete: deleteSong, reorder } as const; }); return { songAtoms } as const;