feat(kits): If-Match preconditions and composition metadata for personal kits

apps/web/app/lib/personal-kits/kit-conflicts.ts

@@ -17,7 +17,7 @@ import type { KitSkillEntry, PersonalKitDocument } from "@skillx/contracts";
 import { badRequest, conflict, json, kitNotFound } from "./kit-http";
 import type { KitCaller } from "./kit-http";
 import { assertSeedRevision, commitKitMutation, documentOf, getRevision, ownedKit, revisionView, writableKit } from "./kit-store";
-import { generationConflict, storeRevision, storedResponse } from "./kit-revisions";
+import { generationConflict, storeRevision, storedResponse, withoutComposition } from "./kit-revisions";
 import type { RebaseReport } from "./kit-rebase";
 
 const CHOICES = ["keep-old", "use-upstream", "edit-overlay", "fork"] as const;
@@ -130,7 +130,7 @@ export async function resolveConflict(caller: KitCaller, kitName: string, id: st
     if (to === null && d.choice !== "keep-old") throw conflict("kit.base_unavailable", `${entry.identity}: the upstream release is not available; keep-old is the only choice`);
     return applyDecision(entry, d, to ?? entry.base);
   });
-  const doc: PersonalKitDocument = { ...from, skills: skills.sort((a, b) => compareUtf8(a.identity, b.identity)) };
+  const doc: PersonalKitDocument = { ...withoutComposition(from), skills: skills.sort((a, b) => compareUtf8(a.identity, b.identity)) };
   const reeval = decisions.some((d) => d.choice !== "keep-old");
   const resolution = {
     schema: RESOLUTION_SCHEMA,

apps/web/app/lib/personal-kits/kit-http.ts

@@ -86,6 +86,25 @@ export function requireGeneration(raw: unknown): number {
   return raw;
 }
 
+/** `If-Match: "g<n>"` (the kit's ETag), `"<n>"` or `<n>`. */
+const IF_MATCH = /^(?:"g?(\d{1,15})"|(\d{1,15}))$/;
+
+/**
+ * The expected generation of a write, from `If-Match` or the body's
+ * `expectedGeneration` (kept for compatibility). Both may be sent only when
+ * they agree. A stale generation is 412 when it came with `If-Match` (see
+ * `serveKit`), 409 `kit.generation_conflict` otherwise.
+ */
+export function withIfMatch(request: Request, body: Record<string, unknown>): Record<string, unknown> {
+  const raw = request.headers.get("If-Match");
+  if (raw === null) return body;
+  const m = IF_MATCH.exec(raw.trim());
+  if (m === null) throw badRequest('If-Match must be one kit generation, as its ETag "g<n>" or <n>');
+  const generation = Number(m[1] ?? m[2]);
+  if (body.expectedGeneration !== undefined && body.expectedGeneration !== generation) throw badRequest("If-Match and expectedGeneration name different generations");
+  return { ...body, expectedGeneration: generation };
+}
+
 const IDEMPOTENCY = /^[A-Za-z0-9._:-]{8,128}$/;
 export function requireIdempotencyKey(raw: unknown): string {
   if (typeof raw !== "string" || !IDEMPOTENCY.test(raw)) throw badRequest("idempotencyKey must be 8 to 128 characters of [A-Za-z0-9._:-]");
@@ -161,6 +180,9 @@ export async function serveKit(
     if (error instanceof DetailedKitError) {
       return json({ error: { code: error.code, message: error.message, ...error.details } }, error.status);
     }
+    if (error instanceof ReleaseHttpError && error.code === "kit.generation_conflict" && request.headers.has("If-Match")) {
+      return errorResponse(releaseError(412, error.code, error.message));
+    }
     if (error instanceof ReleaseHttpError) {
       const retry = (error as { retryAfter?: number }).retryAfter;
       return errorResponse(error, retry === undefined ? failureHeaders(error) : { "Retry-After": String(retry) });

apps/web/app/lib/personal-kits/kit-rebase.ts

@@ -26,7 +26,7 @@ import {
   writableKit,
 } from "./kit-store";
 import type { KitRow, RevisionRow } from "./kit-store";
-import { generationConflict, storeRevision, storedResponse } from "./kit-revisions";
+import { generationConflict, storeRevision, storedResponse, withoutComposition } from "./kit-revisions";
 
 export const REBASE_REPORT_SCHEMA = "skillx.kit-rebase-report/v1";
 const MAX_UPDATES = 128;
@@ -108,7 +108,7 @@ export async function computeRebase(caller: KitCaller, from: RevisionRow, update
   }
   if (entries.length === 0) throw badRequest("no kit entry uses a package named in updates");
   const report: RebaseReport = { schema: REBASE_REPORT_SCHEMA, fromRevisionId: from.id, status: entries.some((e) => e.status === "needs-review") ? "needs-review" : "clean", entries };
-  const candidate: PersonalKitDocument = { ...doc, skills: doc.skills.map((s) => candidates[s.identity] ?? s) };
+  const candidate: PersonalKitDocument = { ...withoutComposition(doc), skills: doc.skills.map((s) => candidates[s.identity] ?? s) };
   return { report, candidates, candidate };
 }
 

apps/web/app/lib/personal-kits/kit-revisions.ts

@@ -45,6 +45,13 @@ export interface StoreRequest {
   action?: string;
 }
 
+/**
+ * A server-built revision (rebase, conflict resolution) moves pins the
+ * client's composition records describe, so it never carries them over; the
+ * client re-attaches fresh records in its next manual revision.
+ */
+export const withoutComposition = ({ composition, ...doc }: PersonalKitDocument): PersonalKitDocument => doc;
+
 export const generationConflict = () => conflict("kit.generation_conflict", "the kit changed; reload it and retry with its current generation");
 
 export function kitView(kit: KitRow, effective: RevisionRow | null, openConflicts: number) {
@@ -153,7 +160,7 @@ export async function readDiff(caller: KitCaller, kitName: string, fromId: strin
   const b = JSON.parse(to.document_json) as PersonalKitDocument;
   const byId = (d: PersonalKitDocument) => new Map(d.skills.map((s) => [s.identity, s]));
   const [x, y] = [byId(a), byId(b)];
-  const top = (["visibility", "parent", "exclude", "profile", "runtimes"] as const).filter((k) => canonicalJson(a[k] ?? null) !== canonicalJson(b[k] ?? null));
+  const top = (["visibility", "parent", "exclude", "profile", "runtimes", "composition"] as const).filter((k) => canonicalJson(a[k] ?? null) !== canonicalJson(b[k] ?? null));
   return json({
     from: from.id,
     to: to.id,

apps/web/app/lib/personal-kits/kit-router.ts

@@ -3,7 +3,7 @@
  * GETs are reads; POSTs are writes (write limiter, fail closed). An unknown
  * sub-path is the JSON 404; a known path with the wrong method is 405.
  */
-import { json, parseKitName, readKitBody, requireGeneration, requireId, requireIdempotencyKey, serveKit } from "./kit-http";
+import { json, parseKitName, readKitBody, requireGeneration, requireId, requireIdempotencyKey, serveKit, withIfMatch } from "./kit-http";
 import type { KitCaller, KitEnv } from "./kit-http";
 import { readConflict, readConflicts, resolveConflict } from "./kit-conflicts";
 import { activateRevision, readLock, rejectRevision } from "./kit-lifecycle";
@@ -23,7 +23,11 @@ interface RouteDef {
   admin?: true;
 }
 
-const body = (c: KitCaller, fields: string[]) => readKitBody(c.request, fields);
+/** The request body; on a generation-guarded write, `If-Match` supplies or confirms `expectedGeneration`. */
+const body = async (c: KitCaller, fields: string[]) => {
+  const raw = await readKitBody(c.request, fields);
+  return fields.includes("expectedGeneration") ? withIfMatch(c.request, raw) : raw;
+};
 const query = (c: KitCaller, name: string) => new URL(c.request.url).searchParams.get(name);
 
 const ROUTES: RouteDef[] = [

apps/web/tests/personal-kits/kit-composition.test.ts

@@ -0,0 +1,64 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { BODY, createKitHarness, skillText } from "./kit-harness";
+import type { KitHarness } from "./kit-harness";
+
+vi.mock("~/lib/auth/session-helpers", () => ({ getSession: async () => null }));
+
+let k: KitHarness;
+const V1 = { "SKILL.md": skillText(BODY) };
+const V2 = { "SKILL.md": skillText(BODY.replace("eight\n", "eight (v2)\n")) };
+const MINE = { "SKILL.md": skillText(BODY.replace("two\n", "two (mine)\n")) };
+const D = (c: string) => `sha256:${c.repeat(64)}`;
+
+const COMPOSITION = {
+  extends: [{ kit: "engineer", version: "3.1.0" }],
+  include: ["@alpha/demo"],
+  resolutions: [
+    {
+      identity: "@alpha/demo", path: "SKILL.md", category: "conflict.same_hunk", baseFrom: D("1"), baseTo: D("2"), baseTreeFrom: D("3"), baseTreeTo: D("4"),
+      choice: "keep-old", overlayDigestAfter: D("5"), kit: "engineer", fromVersion: "3.0.0", toVersion: "3.1.0", planToken: "plan-0001", approvedBy: "user",
+    },
+  ],
+};
+
+beforeEach(async () => {
+  k = await createKitHarness();
+  await k.publish("@alpha/demo", "1.0.0", V1);
+});
+
+describe("revision composition", () => {
+  it("stores composition verbatim under the revision digest, returns it on read and diffs it", async () => {
+    const plain = k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]);
+    const r1 = (await k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document: plain })).body.revision;
+    const composed = { ...plain, composition: COMPOSITION };
+    const res = await k.call("revisions", { expectedGeneration: 1, idempotencyKey: k.idem(), document: composed });
+    expect(res.status).toBe(201);
+    expect(res.body.revision.digest).not.toBe(r1.digest);
+    const read = await k.call(`revisions/${res.body.revision.id}`);
+    expect(read.body.document.composition).toEqual(COMPOSITION);
+    const diff = await k.call(`diff?from=${r1.id}&to=${res.body.revision.id}`);
+    expect(diff.body.document).toEqual(["composition"]);
+    expect(diff.body.changed).toEqual([]);
+  });
+
+  it("refuses an invalid composition with kit.invalid and writes nothing", async () => {
+    const document = { ...k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]), composition: { ...COMPOSITION, extends: [] } };
+    const res = await k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document });
+    expect(res.status).toBe(422);
+    expect(res.body.error.code).toBe("kit.invalid");
+    expect(res.body.error.message).toMatch(/^composition\.extends/);
+    expect(k.count("personal_kits") + k.count("personal_kit_revisions")).toBe(0);
+  });
+
+  it("drops composition from a server-built rebase revision, whose pins its records no longer describe", async () => {
+    const document = { ...k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]), composition: COMPOSITION };
+    const rev = (await k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document })).body.revision;
+    await k.call("activate", { expectedGeneration: 1, revisionId: rev.id, approval: { revisionDigest: rev.digest } });
+    await k.publish("@alpha/demo", "2.0.0", V2);
+    const rebased = await k.call("rebase", { expectedGeneration: 2, idempotencyKey: k.idem(), updates: [{ package: "@alpha/demo", version: "2.0.0" }] });
+    expect(rebased.status).toBe(201);
+    const doc = (await k.call(`revisions/${rebased.body.revision.id}`)).body.document;
+    expect(doc.skills[0].base.version).toBe("2.0.0");
+    expect(doc.composition).toBeUndefined();
+  });
+});

apps/web/tests/personal-kits/kit-preconditions.test.ts

@@ -0,0 +1,89 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { BODY, createKitHarness, skillText } from "./kit-harness";
+import type { KitHarness } from "./kit-harness";
+
+vi.mock("~/lib/auth/session-helpers", () => ({ getSession: async () => null }));
+
+let k: KitHarness;
+const V1 = { "SKILL.md": skillText(BODY) };
+const V2 = { "SKILL.md": skillText(BODY.replace("eight\n", "eight (v2)\n")) };
+const MINE = { "SKILL.md": skillText(BODY.replace("two\n", "two (mine)\n")) };
+
+beforeEach(async () => {
+  k = await createKitHarness();
+  await k.publish("@alpha/demo", "1.0.0", V1);
+});
+
+const store = async (document: unknown, body: Record<string, unknown>, headers?: Record<string, string>) =>
+  k.call("revisions", { idempotencyKey: k.idem(), document, ...body }, { headers });
+
+describe("If-Match on kit writes", () => {
+  it("takes the generation from If-Match, as the kit ETag or a bare number, without a body field", async () => {
+    const first = await store(k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]), {}, { "If-Match": '"g0"' });
+    expect(first.status).toBe(201);
+    const etag = (await k.call("")).headers.get("ETag")!;
+    expect(etag).toBe('"g1"');
+    const second = await store(k.doc([await k.entry("@alpha/demo", "1.0.0")]), {}, { "If-Match": etag });
+    expect(second.status).toBe(201);
+    const activate = await k.call("activate", { revisionId: second.body.revision.id, approval: { revisionDigest: second.body.revision.digest } }, { headers: { "If-Match": "2" } });
+    expect(activate.status).toBe(200);
+  });
+
+  it("answers a stale If-Match with 412 and writes nothing, while a stale body field stays 409", async () => {
+    await store(k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]), { expectedGeneration: 0 });
+    const before = k.count("personal_kit_revisions") + k.count("personal_kit_audit");
+    const document = k.doc([await k.entry("@alpha/demo", "1.0.0")]);
+    const stale = await store(document, {}, { "If-Match": '"g0"' });
+    expect(stale.status).toBe(412);
+    expect(stale.body.error.code).toBe("kit.generation_conflict");
+    const legacy = await store(document, { expectedGeneration: 0 });
+    expect(legacy.status).toBe(409);
+    expect(legacy.body.error.code).toBe("kit.generation_conflict");
+    expect(k.count("personal_kit_revisions") + k.count("personal_kit_audit")).toBe(before);
+  });
+
+  it("guards every generation-checked write, not only revision stores", async () => {
+    const rev = (await store(k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]), { expectedGeneration: 0 })).body.revision;
+    const activate = await k.call("activate", { revisionId: rev.id, approval: { revisionDigest: rev.digest } }, { headers: { "If-Match": '"g7"' } });
+    expect(activate.status).toBe(412);
+    await k.publish("@alpha/demo", "2.0.0", V2);
+    const rebase = await k.call("rebase", { idempotencyKey: k.idem(), updates: [{ package: "@alpha/demo", version: "2.0.0" }] }, { headers: { "If-Match": '"g5"' } });
+    expect(rebase.status).toBe(412);
+  });
+
+  it("refuses a malformed If-Match or one that disagrees with the body, before any write", async () => {
+    const document = k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]);
+    for (const value of ["*", 'W/"g0"', '"g0", "g1"', "-1", "g"]) {
+      const res = await store(document, {}, { "If-Match": value });
+      expect(res.status, value).toBe(400);
+      expect(res.body.error.code).toBe("invalid_request");
+    }
+    const split = await store(document, { expectedGeneration: 1 }, { "If-Match": '"g0"' });
+    expect(split.status).toBe(400);
+    const agree = await store(document, { expectedGeneration: 0 }, { "If-Match": '"g0"' });
+    expect(agree.status).toBe(201);
+  });
+});
+
+describe("base pins", () => {
+  it("accepts any published release of a package as a pin, not only the newest", async () => {
+    await k.publish("@alpha/demo", "2.0.0", V2);
+    const old = await store(k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]), { expectedGeneration: 0 });
+    expect(old.status).toBe(201);
+    expect(old.body.warnings).toEqual([]);
+    const activate = await k.call("activate", { expectedGeneration: 1, revisionId: old.body.revision.id, approval: { revisionDigest: old.body.revision.digest } });
+    expect(activate.status).toBe(200);
+    expect(activate.body.lock.entries[0]).toMatchObject({ identity: "@alpha/demo", version: "1.0.0" });
+  });
+
+  it("keeps resolving a yanked pin with a warning, and refuses an unknown version", async () => {
+    await k.publish("@alpha/demo", "2.0.0", V2);
+    k.sql("UPDATE package_releases SET release_state = 'yanked' WHERE version = '1.0.0'");
+    const yanked = await store(k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]), { expectedGeneration: 0 });
+    expect(yanked.status).toBe(201);
+    expect(yanked.body.warnings).toEqual(["@alpha/demo@1.0.0 is yanked; the pin still resolves"]);
+    const entry = await k.entry("@alpha/demo", "1.0.0", MINE);
+    const unknown = await store(k.doc([{ ...entry, base: { ...entry.base, version: "1.5.0" } }]), { expectedGeneration: 1 });
+    expect(unknown.body.error.code).toBe("kit.base_unavailable");
+  });
+});