From 320e10572b933296163e7b52012b1f8501fe91e3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 19 Sep 2026 14:07:58 -0700 Subject: [PATCH] feat(artifacts): version content digest and expected-version precondition Add nullable artifact_version.content_sha256 (0005_version_content_digest migration), computed at write time and mirrored onto artifact the same way metadata already is. A metadata/title-only revise carries the digest forward unchanged; existing rows serialize null with no backfill. Add optional expectedVersion on POST /artifacts/:id/versions, checked under the existing FOR UPDATE lock: a mismatch answers 409 with the current version and writes nothing. --- CHANGELOG.md | 19 ++++ README.md | 19 +++- .../reference-host/test/acceptance.test.ts | 17 +++ src/artifacts.test.ts | 104 +++++++++++++++++- src/artifacts.ts | 74 ++++++++++++- src/migrations.test.ts | 83 ++++++++++++++ src/migrations.ts | 22 ++++ src/mount.test.ts | 67 ++++++++++- src/mount.ts | 21 +++- src/schema.ts | 14 +++ src/uploads.test.ts | 21 ++++ src/uploads.ts | 6 + 12 files changed, 459 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d49377e..09465b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,25 @@ always called out under their own heading. ### Added +- `artifact_version.content_sha256` (text, nullable), added by the new + `0005_version_content_digest` migration and mirrored onto + `artifact.content_sha256` the same way `metadata` already mirrors. Computed + at write time — sha256 (hex) over the UTF-8 bytes of `content` for text and + URL artifacts, or over the uploaded bytes for a blob-backed file artifact's + version 1, since its `content` column is a store-specific pointer, not the + bytes. A metadata/title-only revise (content omitted, so it carries + forward) carries the digest forward unchanged. Returned as `contentSha256` + on `GET /api/artifacts/:id`, `GET /api/artifacts/:id/versions/:version`, + `GET /api/artifacts/:id/versions`, and the `POST /api/artifacts/:id/versions` + response. Existing rows serialize `contentSha256: null` — written before + digests existed, and never backfilled. +- `expectedVersion` (positive integer, optional) on + `POST /api/artifacts/:id/versions`. Checked under the same `SELECT ... FOR + UPDATE` that guards the version bump: a mismatch answers `409 + {"error":"Version conflict","currentVersion":N}` and writes nothing. + Omitting it is today's unconditional-write behavior. Lets a caller bind a + revise to the exact version it last read — for example, a human approval on + specific content — instead of silently overwriting a change it never saw. - `ARTIFACT_UPLOAD_POLICY` accepts packaged archives — `application/gzip` / `application/x-gzip` (`.tar.gz`, `.tgz`, `.gz`) and `application/x-tar` (`.tar`) — so consumers storing packaged builds are no longer refused with diff --git a/README.md b/README.md index d5fe075..a09aa45 100644 --- a/README.md +++ b/README.md @@ -189,7 +189,7 @@ happen in major versions. | `POST /api/artifacts` | Human import — link a URL or paste text | | `POST /api/artifacts/upload` | multipart import. An optional `generatedBy` form field is stored as `source.generatedBy`, a free-form display label nothing here reads back | | `GET /api/artifacts/:id` | Deep link (archived artifacts still load) | -| `GET`/`POST /api/artifacts/:id/versions` | Version history (paginated, no content bodies) and revision | +| `GET`/`POST /api/artifacts/:id/versions` | Version history (paginated, no content bodies) and revision. `POST` accepts an optional `expectedVersion` precondition — see below | | `GET /api/artifacts/:id/versions/:version` | One version, including content | | `POST /api/artifacts/:id/(un)archive` | Idempotent soft-hide | | `GET /api/artifacts/:id/download` | One path over three storage conventions | @@ -231,6 +231,23 @@ the same way an omitted `title`/`content` does; `parentVersionIds` is never carr forward — a version with no explicit parents simply has none. `artifact.metadata` mirrors the current version's value, same as `title`/`content`/`version` already do. +**Content digest:** every version carries `contentSha256` — sha256 (hex) over the UTF-8 +bytes of `content` for text and URL artifacts, or over the uploaded bytes for a +blob-backed file artifact's version 1 (its `content` column is a store-specific +pointer, not the bytes themselves). Returned on the detail route, a pinned version +read, version history, and the revise response; `artifact.contentSha256` mirrors the +current version's value the same way `metadata` does. A metadata/title-only revise +(content omitted, so it carries forward) carries the digest forward unchanged too. Rows +written before this field existed serialize `contentSha256: null` — there is no +backfill. + +**Optimistic revise (`expectedVersion`):** `POST /api/artifacts/:id/versions` accepts an +optional `expectedVersion` (a positive integer), checked under the same `FOR UPDATE` lock +that guards the write. A mismatch answers `409 {"error":"Version conflict","currentVersion":N}` +and writes nothing; omitting it is today's unconditional-write behavior. Use it to bind a +revise to the exact version a caller last read — a human review approving specific +content, for example — rather than silently overwriting whatever changed underneath it. + **Write size limits:** create and revise reject titles longer than 512 characters and content larger than 15 MiB UTF-8 (`ArtifactSizeError` / HTTP 400). JSON mutators also refuse a declared `Content-Length` over that same 15 MiB ceiling with HTTP 413 before diff --git a/examples/reference-host/test/acceptance.test.ts b/examples/reference-host/test/acceptance.test.ts index 8c6b6a9..176e239 100644 --- a/examples/reference-host/test/acceptance.test.ts +++ b/examples/reference-host/test/acceptance.test.ts @@ -78,6 +78,23 @@ describe("import a URL, read it back, revise it, read the history", () => { expect(history.versions[1]!.title).toBe("Launch plan"); }); + test("revise with a stale expectedVersion is refused 409 and leaves the version count unchanged", async () => { + const conflict = await host.request( + `/api/artifacts/${artifactId}/versions`, + postJson({ content: "https://example.com/plan-3", expectedVersion: 1 }), + ); + expect(conflict.status).toBe(409); + expect(await json<{ error: string; currentVersion: number }>(conflict)).toEqual({ + error: "Version conflict", + currentVersion: 2, + }); + + const history = await json<{ versions: { version: number }[] }>( + await host.request(`/api/artifacts/${artifactId}/versions`), + ); + expect(history.versions.map((v) => v.version)).toEqual([2, 1]); + }); + test("GET /versions/:version serves version 1's own content, unaffected by the revision", async () => { const v1 = await json<{ artifact: { version: number; title: string; content: string } }>( await host.request(`/api/artifacts/${artifactId}/versions/1`), diff --git a/src/artifacts.test.ts b/src/artifacts.test.ts index 7a1f517..60f2c8b 100644 --- a/src/artifacts.test.ts +++ b/src/artifacts.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { and, eq } from "drizzle-orm"; +import { and, eq, sql } from "drizzle-orm"; import { ArtifactNotFoundError, ArtifactSizeError, @@ -7,6 +7,7 @@ import { createArtifact, findArtifactByTitle, findOrVersionArtifact, + getArtifact, getArtifactVersion, listArtifactVersions, MAX_ARTIFACT_CONTENT_BYTES, @@ -14,6 +15,8 @@ import { normalizeSource, serializeArtifact, setArtifactArchived, + sha256Hex, + VersionConflictError, writeArtifactVersion, } from "./artifacts.js"; import { artifact, artifactVersion } from "./schema.js"; @@ -32,6 +35,7 @@ describe("create", () => { version: 1, metadata: null, parentVersionIds: null, + contentSha256: sha256Hex("first"), }); }); @@ -674,3 +678,101 @@ describe("version metadata and lineage", () => { }); }); +describe("content digest", () => { + test("create computes sha256 over the UTF-8 bytes of content", async () => { + const db = await testDb(); + const row = await seedArtifact(db, { title: "Brief", content: "first" }); + + expect(row.contentSha256).toBe(sha256Hex("first")); + const pinned = await getArtifactVersion(db, row.id, 1); + expect(pinned?.contentSha256).toBe(sha256Hex("first")); + }); + + test("changes when content is revised", async () => { + const db = await testDb(); + const row = await seedArtifact(db, { content: "v1" }); + + const second = await writeArtifactVersion(db, { + scope: SCOPE, + artifactId: row.id, + content: "v2", + }); + expect(second.contentSha256).toBe(sha256Hex("v2")); + expect(second.contentSha256).not.toBe(sha256Hex("v1")); + + const v2 = await getArtifactVersion(db, row.id, 2); + expect(v2?.contentSha256).toBe(sha256Hex("v2")); + }); + + test("carries the prior digest forward unchanged on a metadata/title-only revise", async () => { + const db = await testDb(); + const row = await seedArtifact(db, { content: "unchanged" }); + const originalDigest = sha256Hex("unchanged"); + expect(row.contentSha256).toBe(originalDigest); + + const revised = await writeArtifactVersion(db, { + scope: SCOPE, + artifactId: row.id, + title: "New Title", + metadata: { tag: "reviewed" }, + }); + expect(revised.contentSha256).toBe(originalDigest); + + const v2 = await getArtifactVersion(db, row.id, 2); + expect(v2?.contentSha256).toBe(originalDigest); + }); + + test("a legacy row with a null digest serializes as null", async () => { + const db = await testDb(); + const row = await seedArtifact(db, { content: "legacy" }); + // Simulate a row written before digests existed — never backfilled. + await db.execute( + sql`UPDATE "artifacts"."artifact" SET "content_sha256" = NULL WHERE "id" = ${row.id}`, + ); + await db.execute( + sql`UPDATE "artifacts"."artifact_version" SET "content_sha256" = NULL WHERE "artifact_id" = ${row.id}`, + ); + + const fetched = await getArtifact(db, row.id); + expect(fetched).not.toBeNull(); + expect(serializeArtifact(fetched!).contentSha256).toBeNull(); + + const pinned = await getArtifactVersion(db, row.id, 1); + expect(pinned?.contentSha256).toBeNull(); + }); +}); + +describe("expectedVersion precondition", () => { + test("a matching expectedVersion succeeds", async () => { + const db = await testDb(); + const row = await seedArtifact(db, { content: "v1" }); + + const revised = await writeArtifactVersion(db, { + scope: SCOPE, + artifactId: row.id, + content: "v2", + expectedVersion: 1, + }); + expect(revised.version).toBe(2); + }); + + test("a mismatched expectedVersion 409s (VersionConflictError) and writes nothing", async () => { + const db = await testDb(); + const row = await seedArtifact(db, { content: "v1" }); + + await expect( + writeArtifactVersion(db, { + scope: SCOPE, + artifactId: row.id, + content: "v2", + expectedVersion: 5, + }), + ).rejects.toBeInstanceOf(VersionConflictError); + + const history = await listArtifactVersions(db, row.id); + expect(history.versions.length).toBe(1); + const current = await getArtifact(db, row.id); + expect(current?.version).toBe(1); + }); +}); + diff --git a/src/artifacts.ts b/src/artifacts.ts index 8f6ef0e..8dbb77d 100644 --- a/src/artifacts.ts +++ b/src/artifacts.ts @@ -1,4 +1,5 @@ import "./arktype.js"; +import { createHash } from "node:crypto"; import { type } from "arktype"; import { and, @@ -153,6 +154,11 @@ export type SerializedArtifactBase = { ownerPrincipalId: string | null; /** Mirrors the current version's `artifact_version.metadata`, opaque to the package. */ metadata: Record | null; + /** + * sha256 (hex) of the current version's content. Null on a row written + * before digests existed — never backfilled. + */ + contentSha256: string | null; archivedAt: string | null; createdAt: string; updatedAt: string; @@ -183,6 +189,7 @@ function serializeArtifactBase( version: row.version, ownerPrincipalId: row.ownerPrincipalId, metadata: (row.metadata as Record | null) ?? null, + contentSha256: row.contentSha256, archivedAt: row.archivedAt?.toISOString() ?? null, createdAt: row.createdAt.toISOString(), updatedAt: row.updatedAt.toISOString(), @@ -211,6 +218,17 @@ function normalizeContentForKind(kind: string, content: string): string { return content; } +/** + * sha256 (hex) over the UTF-8 bytes of `content`. Used for every text/URL + * artifact write; a blob-backed file artifact's first version instead passes + * an explicit digest of the uploaded bytes (see `createFileArtifact` in + * `uploads.ts`), since its `content` column is a store-specific pointer, not + * the bytes themselves. + */ +export function sha256Hex(content: string): string { + return createHash("sha256").update(content, "utf8").digest("hex"); +} + export type CreateArtifactArgs = { scope: ResolvedPrincipal; /** The human who owns this artifact; null for agents with no owning member. */ @@ -223,6 +241,14 @@ export type CreateArtifactArgs = { metadata?: Record | null; /** Explicit lineage for version 1 — never inferred from order. */ parentVersionIds?: string[] | null; + /** + * Digest override for a blob-backed file artifact, whose `content` column + * is a store-specific pointer rather than the bytes themselves — the caller + * (`createFileArtifact` in `uploads.ts`) digests the uploaded bytes and + * passes the result here. Omitted for every other kind, which digests + * `content` itself. + */ + contentSha256?: string; }; /** @@ -254,6 +280,7 @@ export async function createArtifact( }); const metadata = args.metadata ?? null; const parentVersionIds = args.parentVersionIds ?? null; + const contentSha256 = args.contentSha256 ?? sha256Hex(content); const now = new Date(); const [row] = await tx @@ -268,6 +295,7 @@ export async function createArtifact( source: args.source, version: 1, metadata, + contentSha256, createdAt: now, updatedAt: now, }) @@ -282,6 +310,7 @@ export async function createArtifact( authorId: args.scope.principalId, metadata, parentVersionIds, + contentSha256, createdAt: now, }); @@ -295,6 +324,23 @@ export class ArtifactNotFoundError extends Error { } } +/** + * Thrown by `reviseArtifactVersion` when the caller's `expectedVersion` + * precondition does not match the current version, observed under the same + * `FOR UPDATE` lock that guards the write. Nothing is written. + */ +export class VersionConflictError extends Error { + constructor( + artifactId: string, + readonly currentVersion: number, + ) { + super( + `Version conflict on artifact ${artifactId}: current version is ${currentVersion}`, + ); + this.name = "VersionConflictError"; + } +} + /** * The lock-and-write core of a revision, sharing a caller-supplied `tx` so it * composes with a lock already held on that transaction (see @@ -317,6 +363,12 @@ async function reviseArtifactVersion( metadata?: Record | null; /** Explicit lineage for this version — never inferred, never carried forward. */ parentVersionIds?: string[] | null; + /** + * Precondition checked under the `FOR UPDATE` lock below: when set and it + * does not match the current version, {@link VersionConflictError} is + * thrown and nothing is written. Omitted preserves today's behavior. + */ + expectedVersion?: number; }, now: Date, ): Promise { @@ -344,6 +396,13 @@ async function reviseArtifactVersion( throw new ArtifactNotFoundError(args.artifactId); } + if ( + args.expectedVersion !== undefined && + args.expectedVersion !== existing.version + ) { + throw new VersionConflictError(args.artifactId, existing.version); + } + const version = existing.version + 1; const title = args.title ?? existing.title; const content = @@ -358,10 +417,14 @@ async function reviseArtifactVersion( ? (existing.metadata as Record | null) : args.metadata; const parentVersionIds = args.parentVersionIds ?? null; + // Content omitted: the previous content carries forward, so its digest + // carries forward unchanged rather than being recomputed. + const contentSha256 = + args.content === undefined ? existing.contentSha256 : sha256Hex(content); const [updated] = await tx .update(artifact) - .set({ title, content, version, metadata, updatedAt: now }) + .set({ title, content, version, metadata, contentSha256, updatedAt: now }) .where(eq(artifact.id, args.artifactId)) .returning(); if (!updated) throw new ArtifactNotFoundError(args.artifactId); @@ -374,6 +437,7 @@ async function reviseArtifactVersion( authorId: args.scope.principalId, metadata, parentVersionIds, + contentSha256, createdAt: now, }); @@ -395,12 +459,15 @@ export async function writeArtifactVersion( metadata?: Record | null; /** Explicit lineage for this version — never inferred from order. */ parentVersionIds?: string[] | null; + /** See `VersionConflictError` — omitted preserves today's behavior. */ + expectedVersion?: number; }, ): Promise<{ artifactId: string; version: number; title: string; metadata: Record | null; + contentSha256: string | null; }> { if ( args.title === undefined && @@ -421,6 +488,7 @@ export async function writeArtifactVersion( version: row.version, title: row.title, metadata: (row.metadata as Record | null) ?? null, + contentSha256: row.contentSha256, }; }); } @@ -448,6 +516,7 @@ export async function getArtifactVersion( version: number; metadata: Record | null; parentVersionIds: string[] | null; + contentSha256: string | null; } | null> { const [row] = await db .select({ @@ -456,6 +525,7 @@ export async function getArtifactVersion( version: artifactVersion.version, metadata: artifactVersion.metadata, parentVersionIds: artifactVersion.parentVersionIds, + contentSha256: artifactVersion.contentSha256, }) .from(artifactVersion) .where( @@ -480,6 +550,7 @@ export type ArtifactVersionListItem = { createdAt: string; metadata: Record | null; parentVersionIds: string[] | null; + contentSha256: string | null; }; export type ListArtifactVersionsFilters = { @@ -510,6 +581,7 @@ export async function listArtifactVersions( createdAt: artifactVersion.createdAt, metadata: artifactVersion.metadata, parentVersionIds: artifactVersion.parentVersionIds, + contentSha256: artifactVersion.contentSha256, }) .from(artifactVersion) .where(and(...conditions)) diff --git a/src/migrations.test.ts b/src/migrations.test.ts index ec50ce7..a8c07e0 100644 --- a/src/migrations.test.ts +++ b/src/migrations.test.ts @@ -724,4 +724,87 @@ describe("migrations", () => { await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`); await runArtifactMigrations(db); }); + + test("0005_version_content_digest adds a nullable content_sha256 column to both tables", async () => { + assertDestructiveArtifactTestsAllowed(DATABASE_URL); + await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`); + await runArtifactMigrations(db); + + const columns = await db.execute<{ + table_name: string; + column_name: string; + udt_name: string; + is_nullable: string; + }>(sql` + SELECT table_name, column_name, udt_name, is_nullable + FROM information_schema.columns + WHERE table_schema = ${SCHEMA} + AND table_name IN ('artifact', 'artifact_version') + AND column_name = 'content_sha256' + ORDER BY table_name + `); + expect([...columns]).toEqual([ + { + table_name: "artifact", + column_name: "content_sha256", + udt_name: "text", + is_nullable: "YES", + }, + { + table_name: "artifact_version", + column_name: "content_sha256", + udt_name: "text", + is_nullable: "YES", + }, + ]); + + const ledger = await db.execute<{ id: string }>( + sql`SELECT "id" FROM ${sql.identifier(SCHEMA)}.${sql.identifier(LEDGER)} ORDER BY "id"`, + ); + expect(ledger.map((r) => r.id)).toContain("0005_version_content_digest"); + }); + + test("adopting a 0004-only database applies 0005's new column forward", async () => { + assertDestructiveArtifactTestsAllowed(DATABASE_URL); + await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`); + + const upTo0004 = MIGRATIONS.filter((m) => m.id !== "0005_version_content_digest"); + await db.transaction(async (tx) => { + await tx.execute(sql`CREATE SCHEMA IF NOT EXISTS ${sql.identifier(SCHEMA)}`); + await tx.execute(sql` + CREATE TABLE IF NOT EXISTS ${sql.identifier(SCHEMA)}.${sql.identifier(LEDGER)} ( + "id" text PRIMARY KEY, + "checksum" text NOT NULL, + "applied_at" timestamptz NOT NULL DEFAULT now() + ) + `); + for (const migration of upTo0004) { + for (const statement of migration.statements) { + await tx.execute(statement); + } + await tx.execute(sql` + INSERT INTO ${sql.identifier(SCHEMA)}.${sql.identifier(LEDGER)} + ("id", "checksum") + VALUES (${migration.id}, ${migrationChecksum(migration)}) + `); + } + }); + + await runArtifactMigrations(db); + + const ledger = await db.execute<{ id: string }>( + sql`SELECT "id" FROM ${sql.identifier(SCHEMA)}.${sql.identifier(LEDGER)} ORDER BY "id"`, + ); + expect(ledger.map((r) => r.id)).toEqual(MIGRATIONS.map((m) => m.id)); + + const columns = await db.execute<{ column_name: string }>(sql` + SELECT column_name FROM information_schema.columns + WHERE table_schema = ${SCHEMA} AND table_name = 'artifact_version' + AND column_name = 'content_sha256' + `); + expect(columns.map((c) => c.column_name)).toEqual(["content_sha256"]); + + await db.execute(sql`DROP SCHEMA IF EXISTS ${sql.identifier(SCHEMA)} CASCADE`); + await runArtifactMigrations(db); + }); }); diff --git a/src/migrations.ts b/src/migrations.ts index f39eb10..795035c 100644 --- a/src/migrations.ts +++ b/src/migrations.ts @@ -219,6 +219,26 @@ export const MIGRATIONS: Migration[] = [ `, ], }, + { + // `content_sha256` is computed at write time (sha256 hex over the UTF-8 + // bytes of `content` for text/URL artifacts, or over the uploaded bytes + // for a blob-backed file artifact's first version) and mirrored onto + // `artifact` the same way `metadata` already is. Both columns are + // nullable and additive: `ADD COLUMN IF NOT EXISTS` is safe to re-run, and + // existing rows stay null — "written before digests existed" — with no + // backfill. + id: "0005_version_content_digest", + statements: [ + sql` + ALTER TABLE "artifacts"."artifact" + ADD COLUMN IF NOT EXISTS "content_sha256" text + `, + sql` + ALTER TABLE "artifacts"."artifact_version" + ADD COLUMN IF NOT EXISTS "content_sha256" text + `, + ], + }, ]; // Advisory locks are namespaced by this integer alone; deliberately arbitrary @@ -307,6 +327,7 @@ const EXPECTED_OWNED_SHAPE: Readonly> { name: "created_at", udt: "timestamptz" }, { name: "updated_at", udt: "timestamptz" }, { name: "metadata", udt: "jsonb" }, + { name: "content_sha256", udt: "text" }, ], artifact_version: [ { name: "id", udt: "text" }, @@ -318,6 +339,7 @@ const EXPECTED_OWNED_SHAPE: Readonly> { name: "created_at", udt: "timestamptz" }, { name: "metadata", udt: "jsonb" }, { name: "parent_version_ids", udt: "_text" }, + { name: "content_sha256", udt: "text" }, ], upload: [ { name: "id", udt: "text" }, diff --git a/src/mount.test.ts b/src/mount.test.ts index f3a6f89..b751896 100644 --- a/src/mount.test.ts +++ b/src/mount.test.ts @@ -10,6 +10,7 @@ import { listArtifacts, MAX_ARTIFACT_CONTENT_BYTES, setArtifactArchived, + sha256Hex, } from "./artifacts.js"; import { MAX_UPLOAD_BYTES, @@ -659,6 +660,59 @@ describe("versions", () => { expect(res.status).toBe(400); }); + test("revise with a matching expectedVersion succeeds", async () => { + const db = await testDb(); + const app = host(db); + const row = await seedArtifact(db, { title: "Draft", content: "v1" }); + + const res = await app.request( + `/artifacts/${row.id}/versions`, + json({ content: "v2", expectedVersion: 1 }), + ); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ version: 2 }); + }); + + test("revise with a mismatched expectedVersion is 409 and leaves the version count unchanged", async () => { + const db = await testDb(); + const app = host(db); + const row = await seedArtifact(db, { title: "Draft", content: "v1" }); + + const res = await app.request( + `/artifacts/${row.id}/versions`, + json({ content: "v2", expectedVersion: 5 }), + ); + expect(res.status).toBe(409); + expect(await res.json()).toEqual({ error: "Version conflict", currentVersion: 1 }); + + const history = (await ( + await app.request(`/artifacts/${row.id}/versions`) + ).json()) as { versions: { version: number }[] }; + expect(history.versions.map((v) => v.version)).toEqual([1]); + }); + + test("omitting expectedVersion is today's unconditional-write behavior", async () => { + const db = await testDb(); + const app = host(db); + const row = await seedArtifact(db, { title: "Draft", content: "v1" }); + + const res = await app.request(`/artifacts/${row.id}/versions`, json({ content: "v2" })); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ version: 2 }); + }); + + test("revise rejects a non-positive-integer expectedVersion with 400", async () => { + const db = await testDb(); + const row = await seedArtifact(db); + for (const expectedVersion of [0, -1, 1.5]) { + const res = await host(db).request( + `/artifacts/${row.id}/versions`, + json({ content: "x", expectedVersion }), + ); + expect(res.status).toBe(400); + } + }); + test("revising an archived artifact is 404", async () => { const db = await testDb(); const row = await seedArtifact(db); @@ -735,13 +789,18 @@ describe("GET /artifacts/:id/versions/:version", () => { const res = await app.request(`/artifacts/${row.id}/versions/1`); expect(res.status).toBe(200); - const body = (await res.json()) as { artifact: { version: number; title: string; content: string } }; + const body = (await res.json()) as { + artifact: { version: number; title: string; content: string; contentSha256: string | null }; + }; expect(body.artifact).toMatchObject({ version: 1, title: "Draft", content: "v1" }); + expect(body.artifact.contentSha256).toBe(sha256Hex("v1")); const current = await app.request(`/artifacts/${row.id}/versions/2`); - expect(await current.json()).toMatchObject({ - artifact: { version: 2, title: "Final", content: "v2" }, - }); + const currentBody = (await current.json()) as { + artifact: { version: number; title: string; content: string; contentSha256: string | null }; + }; + expect(currentBody.artifact).toMatchObject({ version: 2, title: "Final", content: "v2" }); + expect(currentBody.artifact.contentSha256).toBe(sha256Hex("v2")); }); test("an unknown version is 404", async () => { diff --git a/src/mount.ts b/src/mount.ts index b15173b..23374ea 100644 --- a/src/mount.ts +++ b/src/mount.ts @@ -22,6 +22,7 @@ import { serializeArtifactListItem, setArtifactArchived, SKILL_DRAFT_KIND, + VersionConflictError, writeArtifactVersion, type ArtifactListRow, type SerializedArtifact, @@ -157,10 +158,17 @@ const GeneratedByField = type("unknown") }) .to("string <= 200 | undefined"); +// A JSON-body positive integer, distinct from `VersionRef` below (a path/query +// string). Omitted entirely preserves today's unconditional-write behavior. +const ExpectedVersion = type("number").narrow( + (n, ctx) => (Number.isInteger(n) && n >= 1) || ctx.mustBe("a positive integer"), +); + const ReviseArtifactRequest = type({ "title?": TrimmedNonEmpty, "content?": TrimmedNonEmpty, "metadata?": NullableMetadata, + "expectedVersion?": ExpectedVersion, }).narrow( (body, ctx) => body.title !== undefined || @@ -720,6 +728,7 @@ export function mountArtifacts( content: versionRow.content, version: versionRow.version, metadata: versionRow.metadata, + contentSha256: versionRow.contentSha256, }, ]); return c.json({ artifact: artifactJson }); @@ -732,13 +741,14 @@ export function mountArtifacts( tags: ["Artifacts"], summary: "Revise an artifact, creating a new version", description: - "Locks the row FOR UPDATE and bumps version by one; a unique (artifactId, version) index backstops a racing writer. Archived and skill-draft artifacts present as not found. `metadata` is optional and opaque; when omitted, the prior version's metadata carries forward, and an explicit `null` clears it.", + "Locks the row FOR UPDATE and bumps version by one; a unique (artifactId, version) index backstops a racing writer. Archived and skill-draft artifacts present as not found. `metadata` is optional and opaque; when omitted, the prior version's metadata carries forward, and an explicit `null` clears it. An optional `expectedVersion` is checked under the same lock: a mismatch answers 409 and writes nothing.", parameters: [idParam], responses: { 200: { description: "New version created" }, 400: { description: "Invalid request body or content" }, 403: { description: "No resolvable principal, or not permitted" }, 404: { description: "Artifact not found" }, + 409: { description: "expectedVersion did not match the current version" }, 413: { description: "Declared Content-Length over the content ceiling" }, }, }), @@ -770,12 +780,21 @@ export function mountArtifacts( ...(body.title !== undefined ? { title: body.title } : {}), ...(body.content !== undefined ? { content: body.content } : {}), ...(body.metadata !== undefined ? { metadata: body.metadata } : {}), + ...(body.expectedVersion !== undefined + ? { expectedVersion: body.expectedVersion } + : {}), }), ); } catch (error) { if (error instanceof ArtifactNotFoundError) { return c.json({ error: "Artifact not found" }, 404); } + if (error instanceof VersionConflictError) { + return c.json( + { error: "Version conflict", currentVersion: error.currentVersion }, + 409, + ); + } if (error instanceof ArtifactSizeError) { return c.json({ error: error.message }, 400); } diff --git a/src/schema.ts b/src/schema.ts index 183270e..a27879b 100644 --- a/src/schema.ts +++ b/src/schema.ts @@ -80,6 +80,12 @@ export const artifact = artifactsSchema.table( * interpreted here. */ metadata: jsonb("metadata"), + /** + * Mirrors the current version's `artifact_version.content_sha256` the same + * way `metadata` mirrors the current version's metadata. Null on rows + * written before digests existed — never backfilled. + */ + contentSha256: text("content_sha256"), /** Soft-archive: null = visible, a timestamp = hidden from discovery. */ archivedAt: timestamp("archived_at", { withTimezone: true }), createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), @@ -119,6 +125,14 @@ export const artifactVersion = artifactsSchema.table( metadata: jsonb("metadata"), /** Explicit lineage set by the writer — never inferred from version order. */ parentVersionIds: text("parent_version_ids").array(), + /** + * sha256 (hex) over the UTF-8 bytes of `content` for text and URL + * artifacts, or over the uploaded bytes for a blob-backed file artifact's + * first version. A metadata/title-only revise (content carried forward) + * carries this forward unchanged. Null means "written before digests + * existed" — there is no backfill. + */ + contentSha256: text("content_sha256"), createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), }, (t) => [ diff --git a/src/uploads.test.ts b/src/uploads.test.ts index 50adf4b..9f955aa 100644 --- a/src/uploads.test.ts +++ b/src/uploads.test.ts @@ -1,3 +1,4 @@ +import { createHash } from "node:crypto"; import { describe, expect, test } from "bun:test"; import { ARTIFACT_UPLOAD_POLICY, @@ -252,4 +253,24 @@ describe("createFileArtifact", () => { expect((await db.select().from(upload)).length).toBe(0); expect((await db.select().from(artifact)).length).toBe(0); }); + + test("records a digest of the uploaded bytes, not of the ContentStore's `content` pointer", async () => { + const db = await testDb(); + const bytes = new Uint8Array([9, 8, 7, 6, 5]); + const row = await db.transaction((tx) => + createFileArtifact(tx, InlineContentStore, { + scope: SCOPE, + ownerPrincipalId: SCOPE.principalId, + filename: "report.pdf", + mimeType: "application/pdf", + bytes, + policy: ARTIFACT_UPLOAD_POLICY, + }), + ); + + const expected = createHash("sha256").update(bytes).digest("hex"); + // InlineContentStore's `content` is empty — the digest must not be over it. + expect(row.content).toBe(""); + expect(row.contentSha256).toBe(expected); + }); }); diff --git a/src/uploads.ts b/src/uploads.ts index b0d9e03..20fc7cd 100644 --- a/src/uploads.ts +++ b/src/uploads.ts @@ -1,3 +1,4 @@ +import { createHash } from "node:crypto"; import { isAllowedMimeType } from "@intx/types"; import type { ArtifactTx } from "./db.js"; import { createArtifact } from "./artifacts.js"; @@ -231,12 +232,17 @@ export async function createFileArtifact( mimeType: args.mimeType, bytes: args.bytes, }); + // Digest the uploaded bytes, not `stored.content` — for a blob-backed store + // that column is a pointer (empty, or a data: URL), not the bytes + // themselves, and this is version 1's authoritative digest either way. + const contentSha256 = createHash("sha256").update(args.bytes).digest("hex"); return await createArtifact(tx, { scope: args.scope, ownerPrincipalId: args.ownerPrincipalId, kind: uploadArtifactKind(args.mimeType), title: args.filename, content: stored.content, + contentSha256, source: { origin: args.origin ?? "imported", ...(args.generatedBy !== undefined ? { generatedBy: args.generatedBy } : {}),