Protect unsaved edits and concurrent project saves

cloudflare/library.test.ts

@@ -8,8 +8,34 @@ let runtime: Miniflare;
 type CallResult<T> = { result?: T; error?: string };
 type DraftResult = {
   ok: boolean; workspace: string; name: string; source: string; source_hash: string;
+  revision_token: string;
   server_source: string | null; state: Record<string, unknown>; path: string;
 };
+
+test("full-project conditional writes guard server, helper, dependency and lock changes atomically", async () => {
+  const request = <T>(method: string, input: unknown) => call<T>(method, input, "conditional");
+  const rejects = (method: string, input: unknown) => fails(method, input, "Project changed since", "conditional");
+  const identity = { workspace: "conditional-project", name: "report" };
+  const project = { files: { "lib/value.ts": "export const value = 1;" }, dependencies: {}, lock: {} };
+  let current = await request<DraftResult>("writeDraft", { ...identity, source: "client\n", server_source: "server", project, expected_revision: null });
+  expect((await request<DraftResult>("preview", identity)).revision_token).toBe(current.revision_token);
+  await rejects("writeDraft", { ...identity, source: "overwrite", expected_revision: null });
+  for (const change of [
+    { server_source: "changed server" },
+    { project: { ...project, files: { "lib/value.ts": "export const value = 2;" } } },
+    { project: { ...project, dependencies: { example: "1.0.0" }, lock: { "node_modules/example/index.js": "version one" } } },
+    { project: { ...project, dependencies: { example: "1.0.0" }, lock: { "node_modules/example/index.js": "changed lock" } } },
+  ]) {
+    const previous = current;
+    current = await request<DraftResult>("writeDraft", { ...identity, source: "client\n", ...change });
+    expect(current.revision_token).not.toBe(previous.revision_token);
+    await rejects("writeDraft", { ...identity, source: "stale browser", server_source: "stale server", project, expected_revision: previous.revision_token });
+    expect(await request("preview", identity)).toMatchObject({ source: current.source, server_source: current.server_source, revision_token: current.revision_token });
+  }
+  const results = await Promise.allSettled(["first", "second"].map(source => request("writeDraft", { ...identity, source, expected_revision: current.revision_token })));
+  expect(results.filter(result => result.status === "fulfilled")).toHaveLength(1);
+  expect(results.filter(result => result.status === "rejected")).toHaveLength(1);
+});
 type VersionResult = {
   version: { id: string; revision: number; source: string; server_source: string | null; workspace: string; name: string };
   event: { id: string; version_id: string; mode: string; initial_state: string };

cloudflare/library.ts

@@ -1,7 +1,7 @@
 import { DurableObject } from "cloudflare:workers";
 import { createHash } from "node:crypto";
 import { ProjectStorage } from "./project-storage";
-import { emptyProject, normalizeProject, type ArtifactProject } from "./project";
+import { assertProjectRevision, emptyProject, normalizeProject, projectRevision, type ArtifactProject } from "./project";
 import type { HistoryEntry, ServeEvent, Version } from "../src/historyTypes";
 
 export const MAX_SOURCE_BYTES = 256 * 1024;
@@ -218,13 +218,14 @@ export class ArtifactLibrary extends DurableObject<unknown> {
     }));
   }
 
-  writeDraft(input: { workspace: string; name: string; source: string; server_source?: string | null; project?: ArtifactProject }) {
+  writeDraft(input: { workspace: string; name: string; source: string; server_source?: string | null; project?: ArtifactProject; expected_revision?: string | null }) {
     const workspace = workspaceName(input.workspace); const name = artifactName(input.name);
     const source = sourceText(input.source, true); const source_hash = sha256(source); const timestamp = now();
     const requestedServer = input.server_source === undefined ? undefined
       : input.server_source === null ? null : sourceText(input.server_source);
     return this.state.storage.transactionSync(() => {
       const current = first(this.sql.exec<DraftRow>("select * from drafts where workspace = ? and name = ?", workspace, name));
+      assertProjectRevision(input.expected_revision, current ? projectRevision(current.source, current.server_source, this.projectStorage.read(current.project)) : null);
       const server_source = requestedServer === undefined ? current?.server_source ?? null : requestedServer;
       const project = input.project === undefined ? current?.project ?? this.projectStorage.encode(emptyProject()) : this.projectStorage.encode(input.project);
       this.sql.exec(`insert into drafts (workspace,name,source,server_source,source_hash,project,state,created_at,updated_at)
@@ -234,7 +235,7 @@ export class ArtifactLibrary extends DurableObject<unknown> {
       workspace, name, source, server_source, source_hash, project, timestamp, timestamp);
       const saved = this.draft(workspace, name);
       return { ok: true, path: sourcePath(workspace, name), workspace, name, source, server_source: saved.server_source,
-        source_hash, project: this.projectStorage.read(saved.project), state: JSON.parse(saved.state) as Record<string, unknown> };
+        source_hash, project: this.projectStorage.read(saved.project), revision_token: projectRevision(source, saved.server_source, this.projectStorage.read(saved.project)), state: JSON.parse(saved.state) as Record<string, unknown> };
     });
   }
 
@@ -280,6 +281,7 @@ export class ArtifactLibrary extends DurableObject<unknown> {
     return {
       path: input.file ?? (part === "client" ? sourcePath(workspace, name) : serverSourcePath(workspace, name)),
       part, file: input.file, project, source_hash: input.file === undefined && part === "client" ? row.source_hash : sha256(selected),
+      revision_token: projectRevision(row.source, row.server_source, project),
       total_lines: lines.length,
       start_line: start, end_line: actualEnd, source: lines.slice(start - 1, actualEnd).join(""),
       next_line: actualEnd < lines.length ? actualEnd + 1 : null,
@@ -459,7 +461,7 @@ export class ArtifactLibrary extends DurableObject<unknown> {
     if (input.name) {
       if (input.event_id !== undefined) throw new Error("event_id requires version_id");
       const name = artifactName(input.name); const draft = this.draft(workspace, name);
-      return { workspace, name, path: sourcePath(workspace, name), source: draft.source, server_source: draft.server_source, project: this.projectStorage.read(draft.project),
+      return { workspace, name, path: sourcePath(workspace, name), source: draft.source, server_source: draft.server_source, project: this.projectStorage.read(draft.project), revision_token: projectRevision(draft.source, draft.server_source, this.projectStorage.read(draft.project)),
         state: JSON.parse(draft.state) as Record<string, unknown>, version_id: null, event_id: null, compiled_id: null };
     }
     const version = this.findVersion(workspace, input.version_id!);
@@ -468,7 +470,7 @@ export class ArtifactLibrary extends DurableObject<unknown> {
       ? first(this.sql.exec<ServeEvent>("select * from serve_events where id = ? and version_id = ?", input.event_id, version.id))
       : first(this.sql.exec<ServeEvent>("select * from serve_events where version_id = ? order by (mode = 'live') desc, rowid desc limit 1", version.id));
     if (input.event_id && !event) throw new Error("serve event not found for version");
-    return { workspace, name: version.name, path: version.source_path, source: version.source, server_source: version.server_source, project: version.project,
+    return { workspace, name: version.name, path: version.source_path, source: version.source, server_source: version.server_source, project: version.project, revision_token: projectRevision(version.source, version.server_source, version.project),
       state: event ? JSON.parse(event.initial_state) as Record<string, unknown> : {}, version_id: version.id, event_id: event?.id ?? null, compiled_id: version.compiled_id };
   }
 

cloudflare/project.ts

@@ -36,6 +36,16 @@ export function projectSourceHash(source: string, project: ArtifactProject): str
   return createHash("sha256").update(Object.values(project).every(value => Object.keys(value).length === 0) ? source : JSON.stringify([source, project])).digest("hex");
 }
 
+/** A full-project save must also detect server, helper and dependency changes. */
+export function projectRevision(source: string, server: string | null, project: ArtifactProject): string {
+  return createHash("sha256").update(JSON.stringify([source, server, normalizeProject(project)])).digest("hex");
+}
+
+export const REVISION_CONFLICT = "Project changed since it was loaded. Reload the saved project or compare it with your edits before saving.";
+export function assertProjectRevision(expected: string | null | undefined, actual: string | null): void {
+  if (expected !== undefined && expected !== actual) throw new Error(REVISION_CONFLICT);
+}
+
 type Manifest = { name: string; version: string; dependencies?: Record<string,string>; peerDependencies?: Record<string,string>; peerDependenciesMeta?: Record<string,{optional?:boolean}>; dist?: { tarball: string; integrity?: string } };
 type Registry = { versions: Record<string,Manifest> };
 const registryOrigin = "https://registry.npmjs.org";

cloudflare/script-service.ts

@@ -1,5 +1,5 @@
 import type { ScriptSchedule } from "./script-backend";
-import { resolveProject } from "./project";
+import { assertProjectRevision, resolveProject } from "./project";
 import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
 import { createHash } from "node:crypto";
 import type { ArtifactEdit } from "./library";
@@ -78,6 +78,10 @@ export class CloudScriptService {
       }
       case "script_remix": case "script_write": case "script_edit": case "script_restore": {
         const target = this.artifacts.target("script", tool === "script_remix" ? args.new_name as string : name);
+        if (tool === "script_write" && args.expected_revision !== undefined) {
+          const current = await scripts.readRange(input).catch(error => { if (error instanceof Error && error.message.includes("script not found")) return null; throw error; });
+          assertProjectRevision(args.expected_revision as string | null, current?.revision_token ?? null);
+        }
         // Read pending intent only after taking this operation's generation.
         // A concurrent explicit access change then either precedes this read or
         // supersedes this generation; it cannot be undone by stale metadata.
@@ -97,7 +101,7 @@ export class CloudScriptService {
         const destination = {...input,name:target.name};
         const previous = tool === "script_write" && args.project !== undefined ? await scripts.readRange(input).catch(error => { if (error instanceof Error && error.message.includes("script not found")) return undefined; throw error; }) : undefined;
         const project = tool === "script_write" && args.project !== undefined ? await resolveProject(args.project, previous?.project) : undefined;
-        const mutation = tool === "script_remix" ? await scripts.remix({workspace:this.artifacts.workspace,name:args.name as string | undefined,version_id:args.version_id as string | undefined,new_name:args.new_name as string}) : tool === "script_write" ? await scripts.writeDraft({ ...input, source: args.contents as string, project })
+        const mutation = tool === "script_remix" ? await scripts.remix({workspace:this.artifacts.workspace,name:args.name as string | undefined,version_id:args.version_id as string | undefined,new_name:args.new_name as string}) : tool === "script_write" ? await scripts.writeDraft({ ...input, source: args.contents as string, project, expected_revision: args.expected_revision as string | null | undefined })
           : tool === "script_edit" ? await scripts.editDraft({ ...input, file: args.file as string | undefined, edits: args.edits as ArtifactEdit[], expected_hash: args.expected_hash as string | undefined })
           : await scripts.restore({ workspace: this.artifacts.workspace, id: args.version_id as string });
         generation ??= await hosted.links.begin(target);

cloudflare/scripts.test.ts

@@ -15,6 +15,22 @@ async function call(method: string,input: unknown,library="alice") {
   return value.result;
 }
 const identity={workspace:"default",name:"handler"};
+test("conditional script saves guard the whole project and preserve history and activation on conflicts", async () => {
+  const input = { workspace: "conditional", name: "handler" };
+  const project = { files: { "lib/value.ts": "export const value = 1;" }, dependencies: {}, lock: {} };
+  const first = await call("writeDraft", { ...input, source: "original", project, expected_revision: null });
+  expect((await call("readRange", input)).revision_token).toBe(first.revision_token);
+  await call("activate", { ...input, source_hash: first.source_hash, code: "last working code" });
+  const second = await call("writeDraft", { ...input, source: "original", project: { ...project, files: { "lib/value.ts": "agent edit" } } });
+  await expect(call("writeDraft", { ...input, source: "stale browser", project, expected_revision: first.revision_token })).rejects.toThrow("Project changed since");
+  expect((await call("readRange", input)).revision_token).toBe(second.revision_token);
+  expect(await call("history", input)).toHaveLength(2);
+  expect((await call("active", input)).code).toBe("last working code");
+  await expect(call("writeDraft", { ...input, source: "name collision", expected_revision: null })).rejects.toThrow("Project changed since");
+  const writes = await Promise.allSettled(["first writer", "second writer"].map(source => call("writeDraft", { ...input, source, expected_revision: second.revision_token })));
+  expect(writes.filter(result => result.status === "fulfilled")).toHaveLength(1);
+  expect(writes.filter(result => result.status === "rejected")).toHaveLength(1);
+});
 test("versioned drafts, atomic edits, last valid activation, secrets and library isolation",async()=>{
   const first=await call("writeDraft",{...identity,source:"first source"});
   const firstActivation=await call("activate",{...identity,source_hash:first.source_hash,code:"valid-code"});

cloudflare/scripts.ts

@@ -2,7 +2,7 @@ import { ProjectStorage } from "./project-storage";
 import { DurableObject } from "cloudflare:workers";
 import { sha256, MAX_SOURCE_BYTES, type ArtifactEdit } from "./library";
 
-import { emptyProject, normalizeProject, projectSourceHash, type ArtifactProject } from "./project";
+import { assertProjectRevision, emptyProject, normalizeProject, projectRevision, projectSourceHash, type ArtifactProject } from "./project";
 
 const bytes = (value: string) => new TextEncoder().encode(value).byteLength;
 function text(value: unknown, label: string, max = 4096): string {
@@ -58,9 +58,16 @@ export class ScriptLibrary extends DurableObject<unknown> {
     this.sql.exec(`insert into scripts(workspace,name,source,source_hash,updated_at,project) values(?,?,?,?,?,?) on conflict(workspace,name) do update set source=excluded.source,source_hash=excluded.source_hash,updated_at=excluded.updated_at,project=excluded.project`,workspace,name,code,source_hash,updated_at,this.projectStorage.encode(project));
     const previous = this.sql.exec<{revision: number; source_hash: string}>("select revision,source_hash from script_versions where workspace=? and name=? order by revision desc limit 1",workspace,name).toArray()[0];
     if (previous?.source_hash !== source_hash || reason === "restore") this.sql.exec("insert into script_versions(id,workspace,name,revision,source,source_hash,created_at,reason,restored_from,project) values(?,?,?,?,?,?,?,?,?,?)",crypto.randomUUID(),workspace,name,(previous?.revision ?? 0)+1,code,source_hash,updated_at,reason,restoredFrom,this.projectStorage.encode(project));
-    return {ok:true,workspace,name,path:`${workspace}/${name}.script.ts`,source:code,project,source_hash,updated_at};
+    return {ok:true,workspace,name,path:`${workspace}/${name}.script.ts`,source:code,project,source_hash,revision_token:projectRevision(code,null,project),updated_at};
+  }
+  writeDraft(input: {workspace: string; name: string; source: string; project?: ArtifactProject; expected_revision?: string | null}) {
+    return this.ctx.storage.transactionSync(() => {
+      const {workspace,name} = key(input);
+      const current = this.sql.exec<Row>("select * from scripts where workspace=? and name=?",workspace,name).toArray()[0];
+      assertProjectRevision(input.expected_revision, current ? projectRevision(current.source,null,this.projectStorage.read(current.project)) : null);
+      return this.save(input,"edit");
+    });
   }
-  writeDraft(input: {workspace: string; name: string; source: string; project?: ArtifactProject}) { return this.ctx.storage.transactionSync(() => this.save(input,"edit")); }
   remix(input: {workspace: string; name?: string; version_id?: string; new_name: string}) {
     const {workspace,name} = key({workspace:input.workspace,name:input.new_name});
     if (Boolean(input.name) === Boolean(input.version_id)) throw new Error("provide name or version_id, but not both");
@@ -89,7 +96,7 @@ export class ScriptLibrary extends DurableObject<unknown> {
     const start = input.start_line ?? 1, end = input.end_line ?? start+199;
     if (!Number.isSafeInteger(start) || !Number.isSafeInteger(end) || start<1 || end<start || start>lines.length) throw new Error("invalid line range");
     const actualEnd = Math.min(end,lines.length);
-    return {workspace:row.workspace,name:row.name,path:input.file ?? `${row.workspace}/${row.name}.script.ts`,file:input.file,project,source:lines.slice(start-1,actualEnd).join(""),source_hash:input.file === undefined ? row.source_hash : sha256(selected),total_lines:lines.length,start_line:start,end_line:actualEnd,next_line:actualEnd<lines.length?actualEnd+1:null};
+    return {workspace:row.workspace,name:row.name,path:input.file ?? `${row.workspace}/${row.name}.script.ts`,file:input.file,project,source:lines.slice(start-1,actualEnd).join(""),source_hash:input.file === undefined ? row.source_hash : sha256(selected),revision_token:projectRevision(row.source,null,project),total_lines:lines.length,start_line:start,end_line:actualEnd,next_line:actualEnd<lines.length?actualEnd+1:null};
   }
   editDraft(input: {workspace: string; name: string; file?: string; edits: ArtifactEdit[]; expected_hash?: string}) {
     return this.ctx.storage.transactionSync(() => {
@@ -157,7 +164,8 @@ export class ScriptLibrary extends DurableObject<unknown> {
     const workspace=text(input.workspace,"workspace"), id=text(input.id,"version id");
     const row=this.sql.exec<VersionRow>("select * from script_versions where workspace=? and id=?",workspace,id).toArray()[0];
     if (!row) throw new Error("version not found in this workspace");
-    return {...row,project:this.projectStorage.read(row.project),origin:this.sql.exec<{source_name: string;source_version_id: string}>("select source_name,source_version_id from script_remix_origins where workspace=? and name=?",workspace,row.name).toArray()[0] ?? null};
+    const project = this.projectStorage.read(row.project);
+    return {...row,project,revision_token:projectRevision(row.source,null,project),origin:this.sql.exec<{source_name: string;source_version_id: string}>("select source_name,source_version_id from script_remix_origins where workspace=? and name=?",workspace,row.name).toArray()[0] ?? null};
   }
   restore(input: {workspace: string; id: string}) {
     return this.ctx.storage.transactionSync(() => {