diff --git a/lib/db/db-client.ts b/lib/db/db-client.ts index 977b5dd..6d6a247 100644 --- a/lib/db/db-client.ts +++ b/lib/db/db-client.ts @@ -106,26 +106,27 @@ const initializer = combine(databaseSchema.parse({}), (set, get) => ({ ...state.files.slice(fileIndex + 1), ] - // Emit FILE_CREATED for new path - state.events.push({ - event_id: (state.idCounter + 0).toString(), - event_type: "FILE_CREATED", - file_path: normNew, - created_at: new Date().toISOString(), - initiator: opts.initiator, - }) - // Emit FILE_DELETED for old path - state.events.push({ - event_id: (state.idCounter + 1).toString(), - event_type: "FILE_DELETED", - file_path: normOld, - created_at: new Date().toISOString(), - initiator: opts.initiator, - }) - + // Build a fresh events array so callers holding the previous array + // do not observe it changing underneath them. return { files, - events: state.events, + events: [ + ...state.events, + { + event_id: (state.idCounter + 0).toString(), + event_type: "FILE_CREATED" as const, + file_path: normNew, + created_at: new Date().toISOString(), + initiator: opts.initiator, + }, + { + event_id: (state.idCounter + 1).toString(), + event_type: "FILE_DELETED" as const, + file_path: normOld, + created_at: new Date().toISOString(), + initiator: opts.initiator, + }, + ], idCounter: state.idCounter + 2, } }) @@ -192,7 +193,10 @@ const initializer = combine(databaseSchema.parse({}), (set, get) => ({ let events = state.events if (since) { - events = events.filter((e) => e.created_at > since) + // Compare absolute instants so equivalent timestamps with different + // timezone offsets filter identically (string comparison would not). + const sinceMs = new Date(since).getTime() + events = events.filter((e) => new Date(e.created_at).getTime() > sinceMs) } if (event_type) { diff --git a/routes/files/delete.ts b/routes/files/delete.ts index 74f58ba..992d3eb 100644 --- a/routes/files/delete.ts +++ b/routes/files/delete.ts @@ -12,6 +12,15 @@ export default withRouteSpec({ })(async (req, ctx) => { const { file_id, file_path, initiator } = req.commonParams + // Deleting by either selector uses OR matching, so accepting both could + // remove two different files while only reporting a single deletion. + if (file_id && file_path) { + return ctx.json( + { error: "Provide either file_id or file_path, not both" }, + { status: 400 }, + ) + } + if (!file_id && !file_path) { return ctx.json( { error: "Either file_id or file_path must be provided" }, diff --git a/routes/files/rename.ts b/routes/files/rename.ts index 35ca26e..7563a89 100644 --- a/routes/files/rename.ts +++ b/routes/files/rename.ts @@ -1,4 +1,5 @@ import { withRouteSpec } from "lib/middleware/with-winter-spec" +import { normalizePath } from "lib/utils/normalize-path" import { z } from "zod" export default withRouteSpec({ @@ -22,6 +23,12 @@ export default withRouteSpec({ })(async (req, ctx) => { const body = await req.json() + // A normalized empty path ("", "/") does not address a file and would + // leave an unreachable entry behind, so fail fast without mutating state. + if (normalizePath(body.new_file_path) === "") { + return ctx.json({ file: null }, { status: 400 }) + } + // First check if the old file exists const oldFile = ctx.db.getFileByPath(body.old_file_path) if (!oldFile) { diff --git a/routes/files/upsert.ts b/routes/files/upsert.ts index 44a1812..c3f0964 100644 --- a/routes/files/upsert.ts +++ b/routes/files/upsert.ts @@ -1,4 +1,5 @@ import { withRouteSpec } from "lib/middleware/with-winter-spec" +import { normalizePath } from "lib/utils/normalize-path" import { z } from "zod" export default withRouteSpec({ @@ -31,6 +32,14 @@ export default withRouteSpec({ }), })(async (req, ctx) => { const body = await req.json() + // normalizePath maps "" and "/" to "". Those do not address a file and + // would create entries that file lookup (which treats "" as missing) can't + // retrieve, so reject them before mutating state. + if (normalizePath(body.file_path) === "") { + return new Response("file_path must not be empty or root", { + status: 400, + }) + } const file = ctx.db.upsertFile(body, { initiator: body.initiator }) return ctx.json({ file }) }) diff --git a/tests/rename-immutability.test.ts b/tests/rename-immutability.test.ts new file mode 100644 index 0000000..f5d0f01 --- /dev/null +++ b/tests/rename-immutability.test.ts @@ -0,0 +1,33 @@ +import { expect, test } from "bun:test" +import { createDatabase } from "lib/db/db-client" + +test("rename does not mutate previously observed event arrays", () => { + const db = createDatabase() + db.upsertFile( + { + file_path: "source.txt", + text_content: "data", + created_at: "2025-06-01T00:00:00.000Z", + }, + {}, + ) + + const observed = db.events + const observedCopy = [...observed] + const observedLength = observed.length + + const renamed = db.renameFile("source.txt", "dest.txt", {}) + expect(renamed?.file_path).toBe("dest.txt") + + // The previously held reference must be untouched. + expect(observed).toHaveLength(observedLength) + expect(observed).toEqual(observedCopy) + + // The store must expose a new array containing the two rename events. + expect(db.events).not.toBe(observed) + expect(db.events).toHaveLength(observedLength + 2) + expect(db.events.slice(-2).map((e) => e.event_type)).toEqual([ + "FILE_CREATED", + "FILE_DELETED", + ]) +}) diff --git a/tests/routes/delete-selector-conflict.test.ts b/tests/routes/delete-selector-conflict.test.ts new file mode 100644 index 0000000..bf40205 --- /dev/null +++ b/tests/routes/delete-selector-conflict.test.ts @@ -0,0 +1,37 @@ +import { expect, test } from "bun:test" +import { getTestServer } from "tests/fixtures/get-test-server" + +test("delete with mismatched id and path does not remove multiple files", async () => { + const { axios } = await getTestServer() + + const alpha = await axios.post("/files/upsert", { + file_path: "alpha.txt", + text_content: "alpha", + }) + await axios.post("/files/upsert", { + file_path: "beta.txt", + text_content: "beta", + }) + + const eventsBefore = (await axios.get("/events/list")).data.event_list.length + + // The id points at alpha.txt while the path points at beta.txt. + // This must be rejected instead of deleting both files. + let status: number | undefined + try { + await axios.post("/files/delete", { + file_id: alpha.data.file.file_id, + file_path: "beta.txt", + }) + } catch (err: any) { + status = err?.status ?? err?.response?.status + } + + expect(status).toBe(400) + + const remaining = (await axios.get("/files/list")).data.file_list + expect(remaining).toHaveLength(2) + + const eventsAfter = (await axios.get("/events/list")).data.event_list + expect(eventsAfter).toHaveLength(eventsBefore) +}) diff --git a/tests/routes/events-since-timezone.test.ts b/tests/routes/events-since-timezone.test.ts new file mode 100644 index 0000000..cd8ff9b --- /dev/null +++ b/tests/routes/events-since-timezone.test.ts @@ -0,0 +1,43 @@ +import { expect, test } from "bun:test" +import { getTestServer } from "tests/fixtures/get-test-server" + +// Format an absolute instant using an explicit numeric timezone offset, +// e.g. offsetHours=+3 -> "+03:00". The wall-clock is shifted so the +// resulting string denotes the same instant as `instantMs`. +function formatWithOffset(instantMs: number, offsetHours: number): string { + const sign = offsetHours >= 0 ? "+" : "-" + const abs = Math.abs(offsetHours) + const hh = String(Math.floor(abs)).padStart(2, "0") + const mm = String(Math.round((abs % 1) * 60)).padStart(2, "0") + const wallClock = new Date(instantMs + offsetHours * 3_600_000) + .toISOString() + .slice(0, 23) + return `${wallClock}${sign}${hh}:${mm}` +} + +test("events/list since compares absolute time, not raw strings", async () => { + const { axios } = await getTestServer() + + const created = await axios.post("/events/create", { + event_type: "FILE_UPDATED", + file_path: "tz-check.txt", + }) + const eventInstant = Date.parse(created.data.event.created_at) + expect(Number.isNaN(eventInstant)).toBe(false) + + // A `since` 1ms before the event, rendered with a +03:00 wall-clock that + // sorts lexicographically *after* the stored "Z" timestamp. + const justBefore = formatWithOffset(eventInstant - 1, 3) + const shouldContain = await axios.get("/events/list", { + params: { since: justBefore }, + }) + expect(shouldContain.data.event_list).toHaveLength(1) + + // The exact same instant rendered with a -02:00 offset. String comparison + // would treat it as earlier and wrongly include the event. + const sameInstant = formatWithOffset(eventInstant, -2) + const shouldExclude = await axios.get("/events/list", { + params: { since: sameInstant }, + }) + expect(shouldExclude.data.event_list).toHaveLength(0) +}) diff --git a/tests/routes/root-path-validation.test.ts b/tests/routes/root-path-validation.test.ts new file mode 100644 index 0000000..89afe61 --- /dev/null +++ b/tests/routes/root-path-validation.test.ts @@ -0,0 +1,43 @@ +import { expect, test } from "bun:test" +import { getTestServer } from "tests/fixtures/get-test-server" + +test("empty and root paths are rejected for upsert and rename", async () => { + const { axios } = await getTestServer() + + for (const badPath of ["", "/"]) { + let upsertStatus: number | undefined + try { + await axios.post("/files/upsert", { + file_path: badPath, + text_content: "should not exist", + }) + } catch (err: any) { + upsertStatus = err?.status ?? err?.response?.status + } + expect(upsertStatus).toBe(400) + } + + await axios.post("/files/upsert", { + file_path: "keeper.txt", + text_content: "keep me", + }) + + let renameStatus: number | undefined + for (const badNewPath of ["", "/"]) { + renameStatus = undefined + try { + await axios.post("/files/rename", { + old_file_path: "keeper.txt", + new_file_path: badNewPath, + }) + } catch (err: any) { + renameStatus = err?.status ?? err?.response?.status + } + expect(renameStatus).toBe(400) + } + + const kept = await axios.get("/files/get", { + params: { file_path: "keeper.txt" }, + }) + expect(kept.data.file?.text_content).toBe("keep me") +})