fix: resolve file deletion conflict, empty paths, timestamp comparison and event mutation (#149)

lib/db/db-client.ts

@@ -107,25 +107,25 @@ const initializer = combine(databaseSchema.parse({}), (set, get) => ({
       ]
 
       // Emit FILE_CREATED for new path
-      state.events.push({
+      const createdEvent: FileServerEvent = {
         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({
+      const deletedEvent: FileServerEvent = {
         event_id: (state.idCounter + 1).toString(),
         event_type: "FILE_DELETED",
         file_path: normOld,
         created_at: new Date().toISOString(),
         initiator: opts.initiator,
-      })
+      }
 
       return {
         files,
-        events: state.events,
+        events: [...state.events, createdEvent, deletedEvent],
         idCounter: state.idCounter + 2,
       }
     })
@@ -192,7 +192,8 @@ const initializer = combine(databaseSchema.parse({}), (set, get) => ({
     let events = state.events
 
     if (since) {
-      events = events.filter((e) => e.created_at > since)
+      const sinceTime = Date.parse(since)
+      events = events.filter((e) => Date.parse(e.created_at) > sinceTime)
     }
 
     if (event_type) {

routes/files/delete.ts

@@ -12,6 +12,13 @@ export default withRouteSpec({
 })(async (req, ctx) => {
   const { file_id, file_path, initiator } = req.commonParams
 
+  if (file_id && file_path) {
+    return ctx.json(
+      { error: "Provide only one of file_id or file_path" },
+      { status: 400 },
+    )
+  }
+
   if (!file_id && !file_path) {
     return ctx.json(
       { error: "Either file_id or file_path must be provided" },

routes/files/rename.ts

@@ -1,5 +1,6 @@
 import { withRouteSpec } from "lib/middleware/with-winter-spec"
 import { z } from "zod"
+import { normalizePath } from "lib/utils/normalize-path"
 
 export default withRouteSpec({
   methods: ["POST"],
@@ -21,6 +22,9 @@ export default withRouteSpec({
   }),
 })(async (req, ctx) => {
   const body = await req.json()
+  if (!normalizePath(body.new_file_path)) {
+    return new Response("new_file_path must name a file", { status: 400 })
+  }
 
   // First check if the old file exists
   const oldFile = ctx.db.getFileByPath(body.old_file_path)

routes/files/upsert.ts

@@ -1,5 +1,6 @@
 import { withRouteSpec } from "lib/middleware/with-winter-spec"
 import { z } from "zod"
+import { normalizePath } from "lib/utils/normalize-path"
 
 export default withRouteSpec({
   methods: ["POST"],
@@ -31,6 +32,9 @@ export default withRouteSpec({
   }),
 })(async (req, ctx) => {
   const body = await req.json()
+  if (!normalizePath(body.file_path)) {
+    return new Response("file_path must name a file", { status: 400 })
+  }
   const file = ctx.db.upsertFile(body, { initiator: body.initiator })
   return ctx.json({ file })
 })

tests/rename-events.test.ts

@@ -0,0 +1,23 @@
+import { test, expect } from "bun:test"
+import { createDatabase } from "lib/db/db-client"
+
+test("rename preserves previous events snapshots", () => {
+  const db = createDatabase()
+  db.upsertFile(
+    {
+      file_path: "before.txt",
+      text_content: "content",
+      created_at: "2025-01-01T00:00:00Z",
+    },
+    {},
+  )
+  const previous = db.events
+  const snapshot = [...previous]
+  db.renameFile("before.txt", "after.txt", {})
+  expect(previous).toEqual(snapshot)
+  expect(db.events).not.toBe(previous)
+  expect(db.events.slice(-2).map((event) => event.event_type)).toEqual([
+    "FILE_CREATED",
+    "FILE_DELETED",
+  ])
+})

tests/routes/empty-file-path.test.ts

@@ -0,0 +1,31 @@
+import { test, expect } from "bun:test"
+import { getTestServer } from "tests/fixtures/get-test-server"
+
+test("creating or renaming a file to the root is rejected", async () => {
+  const { axios } = await getTestServer()
+  for (const file_path of ["", "/"]) {
+    await expect(
+      axios.post("/files/upsert", {
+        file_path,
+        text_content: "unreachable",
+      }),
+    ).rejects.toMatchObject({ status: 400 })
+  }
+  await axios.post("/files/upsert", {
+    file_path: "valid.txt",
+    text_content: "retained",
+  })
+  await expect(
+    axios.post("/files/rename", {
+      old_file_path: "valid.txt",
+      new_file_path: "/",
+    }),
+  ).rejects.toMatchObject({ status: 400 })
+  expect(
+    (
+      await axios.get("/files/get", {
+        params: { file_path: "valid.txt" },
+      })
+    ).data.file.text_content,
+  ).toBe("retained")
+})

tests/routes/event-timestamp.test.ts

@@ -0,0 +1,25 @@
+import { test, expect } from "bun:test"
+import { getTestServer } from "tests/fixtures/get-test-server"
+
+test("since compares instants rather than timestamp strings", async () => {
+  const { axios } = await getTestServer()
+  const created = await axios.post("/events/create", {
+    event_type: "FILE_UPDATED",
+    file_path: "test.txt",
+  })
+  const instant = Date.parse(created.data.event.created_at)
+  const earlierOffset = new Date(instant + 2 * 3600000 - 1)
+    .toISOString()
+    .replace("Z", "+02:00")
+  const equalOffset = new Date(instant - 3600000)
+    .toISOString()
+    .replace("Z", "-01:00")
+  const later = await axios.get("/events/list", {
+    params: { since: earlierOffset },
+  })
+  expect(later.data.event_list).toHaveLength(1)
+  const equal = await axios.get("/events/list", {
+    params: { since: equalOffset },
+  })
+  expect(equal.data.event_list).toHaveLength(0)
+})

tests/routes/selector-conflict.test.ts

@@ -0,0 +1,22 @@
+import { test, expect } from "bun:test"
+import { getTestServer } from "tests/fixtures/get-test-server"
+
+test("a conflicting delete selector cannot remove two files", async () => {
+  const { axios } = await getTestServer()
+  const first = await axios.post("/files/upsert", {
+    file_path: "first.txt",
+    text_content: "first",
+  })
+  await axios.post("/files/upsert", {
+    file_path: "second.txt",
+    text_content: "second",
+  })
+  await expect(
+    axios.post("/files/delete", {
+      file_id: first.data.file.file_id,
+      file_path: "second.txt",
+    }),
+  ).rejects.toMatchObject({ status: 400 })
+  expect((await axios.get("/files/list")).data.file_list).toHaveLength(2)
+  expect((await axios.get("/events/list")).data.event_list).toHaveLength(2)
+})