From 46b713338a594369f90eb788b062db41b1aeb3bf Mon Sep 17 00:00:00 2001 From: Duy Nguyen Date: Fri, 16 Oct 2026 09:55:09 +0700 Subject: [PATCH 1/2] feat(kits): honor If-Match generation preconditions on kit writes A write may send the expected generation as If-Match (the kit ETag "g" or a bare ) instead of, or agreeing with, the body expectedGeneration. A stale If-Match answers 412 kit.generation_conflict; the body field keeps its 409. Also proves a pin on an older or yanked release still stores. --- apps/web/app/lib/personal-kits/kit-http.ts | 22 +++++ apps/web/app/lib/personal-kits/kit-router.ts | 8 +- .../personal-kits/kit-preconditions.test.ts | 89 +++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 apps/web/tests/personal-kits/kit-preconditions.test.ts diff --git a/apps/web/app/lib/personal-kits/kit-http.ts b/apps/web/app/lib/personal-kits/kit-http.ts index e8afa3e..545b36c 100644 --- a/apps/web/app/lib/personal-kits/kit-http.ts +++ b/apps/web/app/lib/personal-kits/kit-http.ts @@ -86,6 +86,25 @@ export function requireGeneration(raw: unknown): number { return raw; } +/** `If-Match: "g"` (the kit's ETag), `""` or ``. */ +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): Record { + 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" or '); + 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) }); diff --git a/apps/web/app/lib/personal-kits/kit-router.ts b/apps/web/app/lib/personal-kits/kit-router.ts index b9dd564..8168217 100644 --- a/apps/web/app/lib/personal-kits/kit-router.ts +++ b/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[] = [ diff --git a/apps/web/tests/personal-kits/kit-preconditions.test.ts b/apps/web/tests/personal-kits/kit-preconditions.test.ts new file mode 100644 index 0000000..ed083dc --- /dev/null +++ b/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, headers?: Record) => + 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"); + }); +}); From c366d6188a68113a09933e930c71e6a904e90415 Mon Sep 17 00:00:00 2001 From: Duy Nguyen Date: Fri, 16 Oct 2026 09:55:09 +0700 Subject: [PATCH 2/2] feat(kits): optional composition metadata on revision documents Revision documents may carry AgentKit's extends, include and resolution records under composition. It is strictly validated, never normalized and part of the revision digest; documents without it keep their digests. Rebase and conflict resolution revisions drop it because they move the pins the records describe. --- .../app/lib/personal-kits/kit-conflicts.ts | 4 +- apps/web/app/lib/personal-kits/kit-rebase.ts | 4 +- .../app/lib/personal-kits/kit-revisions.ts | 9 +- .../personal-kits/kit-composition.test.ts | 64 ++++++++ docs/personal-kits-api.md | 54 ++++++- .../personal-kit-composition-golden.json | 83 ++++++++++ packages/contracts/src/personal-kit/index.ts | 1 + .../src/personal-kit/kit-composition.test.ts | 65 ++++++++ .../src/personal-kit/kit-composition.ts | 150 ++++++++++++++++++ .../src/personal-kit/kit-document.ts | 7 +- 10 files changed, 432 insertions(+), 9 deletions(-) create mode 100644 apps/web/tests/personal-kits/kit-composition.test.ts create mode 100644 packages/contracts/src/fixtures/personal-kit-composition-golden.json create mode 100644 packages/contracts/src/personal-kit/kit-composition.test.ts create mode 100644 packages/contracts/src/personal-kit/kit-composition.ts diff --git a/apps/web/app/lib/personal-kits/kit-conflicts.ts b/apps/web/app/lib/personal-kits/kit-conflicts.ts index e887b63..87b45df 100644 --- a/apps/web/app/lib/personal-kits/kit-conflicts.ts +++ b/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, diff --git a/apps/web/app/lib/personal-kits/kit-rebase.ts b/apps/web/app/lib/personal-kits/kit-rebase.ts index 414a045..84f277a 100644 --- a/apps/web/app/lib/personal-kits/kit-rebase.ts +++ b/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 }; } diff --git a/apps/web/app/lib/personal-kits/kit-revisions.ts b/apps/web/app/lib/personal-kits/kit-revisions.ts index 7be2d22..272930f 100644 --- a/apps/web/app/lib/personal-kits/kit-revisions.ts +++ b/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, diff --git a/apps/web/tests/personal-kits/kit-composition.test.ts b/apps/web/tests/personal-kits/kit-composition.test.ts new file mode 100644 index 0000000..1038471 --- /dev/null +++ b/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(); + }); +}); diff --git a/docs/personal-kits-api.md b/docs/personal-kits-api.md index 5f79a70..9f748ef 100644 --- a/docs/personal-kits-api.md +++ b/docs/personal-kits-api.md @@ -105,9 +105,48 @@ without an explicit user approval. - `compositionDigest` is the tree digest of the base with the overlay applied. The server recomputes it from the pinned release bytes and refuses a mismatch with 409 `kit.composition_digest_mismatch`. +- `base` may pin any release of the package the owner can read, not only the + newest one. A yanked pin still resolves and adds a warning; an unknown + version, or a digest that is not that version's, is `kit.base_unavailable`. - The revision digest is the SHA-256 of the canonical JSON bytes of the validated document. +#### Optional `composition` (AgentKit metadata) + +```json +"composition": { + "extends": [ { "kit": "engineer", "version": "3.1.0" }, + { "skillx": "@alpha/base", "version": "1.0.0", "releaseDigest": "sha256:…" } ], + "include": [ "@alpha/demo" ], + "resolutions": [ { "identity": "@alpha/demo", "path": "SKILL.md", "category": "conflict.same_hunk", + "baseFrom": "sha256:…", "baseTo": "absent", "baseTreeFrom": "sha256:…", "baseTreeTo": "sha256:…", + "choice": "keep-old", "overlayDigestAfter": "sha256:…", "kit": "engineer", + "fromVersion": "3.0.0", "toVersion": "3.1.0", "planToken": "…", "approvedBy": "user" } ] +} +``` + +It carries the AgentKit manifest's `extends` and `include` and the user's +resolution records (`agentkit.skill-overlay-resolution/v1` records), so a kit +synced from SkillX can be rebased or edited from that copy. + +- It is optional. A document without it is unchanged and keeps its revision + digest; with it, the composition is part of the canonical JSON and so of + the digest (vector: `packages/contracts/src/fixtures/personal-kit-composition-golden.json`). +- Validation is strict (`kit.invalid` at the offending path) and nothing is + normalized or reordered: `extends` has 1 to 16 unique entries, each `kit` + (an AgentKit kit ID) or `skillx` (an identity, with `releaseDigest`), with + an exact `version`; `include` has at most 256 unique identities; + `resolutions` has at most 1,000 records, unique by identity, path and + category, with `choice` one of `keep-old`, `use-upstream`, `edit-overlay` + or `fork` (a fork record names `fork: {identity, alias}` with the record's + identity) and `approvedBy: "user"`. +- SkillX stores it verbatim and never resolves `extends` or applies a record: + the entries' pins, overlays and composition digests stay the only truth. +- A server-built revision (rebase, conflict resolution) drops it, because it + moves the pins the records describe; the client attaches fresh records in + its next revision. A session-derived proposal keeps the from revision's + composition (it never moves a pin). + ### Lock: `skillx.personal-kit-lock/v1` The lock is derived on read from immutable documents, so equal revisions @@ -130,8 +169,16 @@ ancestor appears as `{kit, revisionDigest, foreign: true}`. Every kit has a `generation`. `GET` returns it, with an `ETag` of `"g"`. -- Every write carries `expectedGeneration`. A stale value fails with 409 - `kit.generation_conflict` and writes nothing. The kit row, the revision or +- Every write carries the expected generation, as `If-Match: "g"` (the + ETag; a bare `` is also accepted) or as `expectedGeneration` in the body + (kept for compatibility). Both may be sent when they agree; a disagreement, + a list, `*` or a weak tag is 400 `invalid_request`. A stale generation + writes nothing and fails with 412 `kit.generation_conflict` when it came + with `If-Match`, or 409 `kit.generation_conflict` when it came in the body. + A first write (a kit that does not exist yet) expects generation 0. + `proposals/validate` takes `If-Match` too and reports `generationConflict` + instead of failing. +- The kit row, the revision or The kit row, the revision or conflict, and the audit row commit in one D1 batch under the generation guard. - Creating writes (`revisions`, `rebase`, `conflicts/:id/resolve`) also carry an @@ -485,9 +532,10 @@ or file content. Its rollback SQL is in its header. | Code | Status | |---|---| | `kit.generation_conflict`, `kit.idempotency_conflict`, `kit.revision_exists`, `kit.composition_digest_mismatch`, `overlay.conflict`, `kit.approval_required`, `kit.review_required`, `kit.revision_unusable`, `kit.revision_effective`, `kit.discard_unconfirmed`, `kit.conflict_closed`, `kit.locked_incompatible`, `kit.not_a_proposal`, `proposal.private_kit_required` | 409 | +| `kit.generation_conflict` when the stale generation came with `If-Match` | 412 | | `proposal.invalid`, `proposal.forbidden_field`, `proposal.private_only`, `proposal.evidence_too_large`, `proposal.unknown_skill`, `proposal.base_mismatch`, `proposal.content_refused` (`findings`), `proposal.undeclared_permission` (`required`, `missing`), `proposal.excess_permission` (`required`, `extra`) | 422 | | `kit.admin_only`, `kit.interactive_approval_required` | 403 | | `kit.invalid`, `overlay.invalid`, `schema.unsupported_version`, `kit.duplicate_identity`, `kit.mode_violation`, `kit.alias_collision`, `overlay.shared_alias`, `identity.reserved_alias`, `kit.request_too_large`, `kit.cycle`, `kit.multi_parent`, `kit.public_derivative_forbidden`, `kit.base_unavailable` (409 when resolving a conflict) | 422 | | `kit.publisher_not_owned` | 403 | -| `invalid_request` (malformed body, ids or updates) | 400 | +| `invalid_request` (malformed body, ids, updates or `If-Match`) | 400 | | `not_found` (flag off, not the owner, unknown id) | 404 | diff --git a/packages/contracts/src/fixtures/personal-kit-composition-golden.json b/packages/contracts/src/fixtures/personal-kit-composition-golden.json new file mode 100644 index 0000000..58a108e --- /dev/null +++ b/packages/contracts/src/fixtures/personal-kit-composition-golden.json @@ -0,0 +1,83 @@ +{ + "description": "skillx.personal-kit/v1 document with the optional composition (AgentKit extends, include and resolution records). The composition is part of the canonical JSON, so it is part of revisionDigest; the same document without it keeps the digest of personal-kit-golden.json.", + "document": { + "schema": "skillx.personal-kit/v1", + "kit": "@alpha/my-kit", + "localId": "local/my-kit", + "visibility": "private", + "skills": [ + { + "identity": "@alpha/demo", + "mode": "override", + "base": { + "package": "@alpha/demo", + "version": "1.0.0", + "releaseDigest": "sha256:ae50cd6e75cafa22f9159941ca54df9e367f8b78120776af31eb32ca54d4891e" + }, + "overlay": { + "schema": "agentkit.skill-overlay/v1", + "baseTreeDigest": "sha256:ae50cd6e75cafa22f9159941ca54df9e367f8b78120776af31eb32ca54d4891e", + "ops": [ + { + "op": "patch", + "path": "SKILL.md", + "expectSha256": "9217ecd6d4a97f5905dfdfc65f9adb76ede2edbf81b4e3d032dfb82924c88fb8", + "patch": "@@ -5,5 +5,5 @@\n ---\n # Demo\n one\n-two\n+two (mine)\n three\n" + }, + { + "op": "add", + "path": "notes.md", + "contentBase64": "bXkgbm90ZXMK" + }, + { + "op": "delete", + "path": "references/guide.md", + "expectSha256": "03f7c4651afebc005736e55d23fd22574ed332a34e54f74a4ad8aeef384aeef5" + } + ] + }, + "compositionDigest": "sha256:69d62891c257dee43830c4ea37e826d232e439dfe856473b3be55bc38537056b" + } + ], + "exclude": [], + "runtimes": {}, + "composition": { + "extends": [ + { + "kit": "engineer", + "version": "3.1.0" + }, + { + "skillx": "@alpha/foundation", + "version": "1.0.0", + "releaseDigest": "sha256:1111111111111111111111111111111111111111111111111111111111111111" + } + ], + "include": [ + "@alpha/demo" + ], + "resolutions": [ + { + "identity": "@alpha/demo", + "path": "SKILL.md", + "category": "conflict.same_hunk", + "baseFrom": "sha256:2222222222222222222222222222222222222222222222222222222222222222", + "baseTo": "absent", + "baseTreeFrom": "sha256:ae50cd6e75cafa22f9159941ca54df9e367f8b78120776af31eb32ca54d4891e", + "baseTreeTo": "sha256:3333333333333333333333333333333333333333333333333333333333333333", + "choice": "keep-old", + "overlayDigestAfter": "sha256:4444444444444444444444444444444444444444444444444444444444444444", + "kit": "engineer", + "fromVersion": "3.0.0", + "toVersion": "3.1.0", + "planToken": "plan-0123456789abcdef", + "approvedBy": "user" + } + ] + } + }, + "expected": { + "revisionDigest": "sha256:212249165bebd186d44ae930bbaaed2a2c0d607f7362084d942ba540488bb4a4", + "withoutCompositionDigest": "sha256:c1411cc295cd403a18c46a238bd74706caccf50c445722bb7f65009bb30cfd1a" + } +} diff --git a/packages/contracts/src/personal-kit/index.ts b/packages/contracts/src/personal-kit/index.ts index 00ddd7a..8d9fc65 100644 --- a/packages/contracts/src/personal-kit/index.ts +++ b/packages/contracts/src/personal-kit/index.ts @@ -12,6 +12,7 @@ export type { TreeFacts } from "./frontmatter-facts"; export { overlayBetween, rebaseOverlay } from "./rebase"; export type { RebaseFinding, RebaseResult } from "./rebase"; export * from "./kit-document"; +export * from "./kit-composition"; export * from "./kit-lock"; export * from "./proposal-policy"; diff --git a/packages/contracts/src/personal-kit/kit-composition.test.ts b/packages/contracts/src/personal-kit/kit-composition.test.ts new file mode 100644 index 0000000..92f9e28 --- /dev/null +++ b/packages/contracts/src/personal-kit/kit-composition.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { PersonalKitError } from "./codes"; +import { revisionDigest, validateKitDocument } from "./kit-document"; +import golden from "../fixtures/personal-kit-composition-golden.json"; +import plain from "../fixtures/personal-kit-golden.json"; + +const D = (c: string) => `sha256:${c.repeat(64)}`; +const record = golden.document.composition.resolutions[0]!; +const withComposition = (composition: unknown): Record => ({ ...structuredClone(plain.document), composition }); +const base = golden.document.composition; +const code = (composition: unknown) => { + try { + validateKitDocument(withComposition(composition)); + return "ok"; + } catch (error) { + return error instanceof PersonalKitError ? `${error.code} ${error.path}` : String(error); + } +}; + +describe("revision document composition", () => { + it("is part of the revision digest, and leaves a document without it on its old digest", async () => { + const doc = validateKitDocument(structuredClone(golden.document)); + expect(doc).toEqual(golden.document); + expect(await revisionDigest(doc)).toBe(golden.expected.revisionDigest); + const { composition, ...without } = structuredClone(golden.document); + expect(composition).toBeDefined(); + expect(await revisionDigest(validateKitDocument(without))).toBe(golden.expected.withoutCompositionDigest); + expect(golden.expected.withoutCompositionDigest).toBe(plain.expected.revisionDigest); + }); + + it("accepts extends alone, either source kind, and every choice", () => { + expect(code({ extends: [{ kit: "engineer", version: "3.1.0" }] })).toBe("ok"); + expect(code({ extends: [{ skillx: "@alpha/base", version: "1.0.0", releaseDigest: D("1") }], include: [] , resolutions: [] })).toBe("ok"); + for (const choice of ["keep-old", "use-upstream", "edit-overlay"]) expect(code({ ...base, resolutions: [{ ...record, choice }] })).toBe("ok"); + const fork = { ...record, identity: "@local/my-plan", choice: "fork", fork: { identity: "@local/my-plan", alias: "my-plan" } }; + expect(code({ ...base, resolutions: [fork] })).toBe("ok"); + }); + + it("refuses malformed composition at the offending path, never normalizing it", () => { + const at = (path: string) => `kit.invalid composition${path}`; + expect(code([])).toBe(at("")); + expect(code({ ...base, extra: 1 })).toBe(at(".extra")); + expect(code({ include: [] })).toBe(at(".extends")); + expect(code({ extends: [] })).toBe(at(".extends")); + expect(code({ extends: Array.from({ length: 17 }, (_, i) => ({ kit: `k${i}`, version: "1.0.0" })) })).toBe(at(".extends")); + expect(code({ extends: [{ kit: "engineer", version: "^3.1.0" }] })).toBe(at(".extends[0].version")); + expect(code({ extends: [{ kit: "Engineer", version: "3.1.0" }] })).toBe(at(".extends[0].kit")); + expect(code({ extends: [{ kit: "engineer", skillx: "@a/b", version: "3.1.0" }] })).toBe(at(".extends[0]")); + expect(code({ extends: [{ skillx: "@a/b", version: "3.1.0" }] })).toBe(at(".extends[0].releaseDigest")); + expect(code({ extends: [{ kit: "engineer", version: "3.1.0" }, { kit: "engineer", version: "3.2.0" }] })).toBe(at(".extends[1]")); + expect(code({ ...base, include: ["@alpha/demo", "@alpha/demo"] })).toBe(at(".include[1]")); + expect(code({ ...base, include: ["demo"] })).toBe(at(".include[0]")); + expect(code({ ...base, resolutions: [record, record] })).toBe(at(".resolutions[1]")); + expect(code({ ...base, resolutions: [{ ...record, choice: "auto" }] })).toBe(at(".resolutions[0].choice")); + expect(code({ ...base, resolutions: [{ ...record, approvedBy: "agent" }] })).toBe(at(".resolutions[0].approvedBy")); + expect(code({ ...base, resolutions: [{ ...record, category: "same_hunk" }] })).toBe(at(".resolutions[0].category")); + expect(code({ ...base, resolutions: [{ ...record, baseTo: "missing" }] })).toBe(at(".resolutions[0].baseTo")); + expect(code({ ...base, resolutions: [{ ...record, path: "a\nb" }] })).toBe(at(".resolutions[0].path")); + expect(code({ ...base, resolutions: [{ ...record, transcript: "x" }] })).toBe(at(".resolutions[0].transcript")); + expect(code({ ...base, resolutions: [{ ...record, choice: "fork" }] })).toBe(at(".resolutions[0].fork")); + expect(code({ ...base, resolutions: [{ ...record, fork: { identity: record.identity, alias: "x" } }] })).toBe(at(".resolutions[0].fork")); + const otherFork = { ...record, choice: "fork", fork: { identity: "@local/other", alias: "other" } }; + expect(code({ ...base, resolutions: [otherFork] })).toBe(at(".resolutions[0].fork.identity")); + }); +}); diff --git a/packages/contracts/src/personal-kit/kit-composition.ts b/packages/contracts/src/personal-kit/kit-composition.ts new file mode 100644 index 0000000..7e9c192 --- /dev/null +++ b/packages/contracts/src/personal-kit/kit-composition.ts @@ -0,0 +1,150 @@ +/** + * Optional `composition` of a revision document: the AgentKit manifest's + * `extends` and `include` plus the user's resolution records (keep-old, + * use-upstream, edit-overlay and fork decisions), so a kit synced from + * SkillX can be rebased or edited from that copy. SkillX validates the shape + * and stores it under the revision digest; it never resolves `extends` or + * applies a record (the entries' pins and overlays stay the only truth). + * Field names mirror AgentKit `agentkit.personal-kit/v1` and + * `agentkit.skill-overlay-resolution/v1`. Nothing is normalized: lists are + * refused, not reordered, so the client's digest and the server's agree. + */ +import { validateIdentity } from "@skillx/skill-tree/rules"; +import { RELEASE_VERSION_PATTERN } from "../release-manifest"; +import { KIT_CODES, kitError } from "./codes"; +import { DIGEST_PATTERN } from "./overlay-envelope"; + +export type KitExtend = { kit: string; version: string } | { skillx: string; version: string; releaseDigest: string }; + +export const RESOLUTION_CHOICES = ["keep-old", "use-upstream", "edit-overlay", "fork"] as const; + +export interface KitResolutionRecord { + identity: string; + path: string; + category: string; + baseFrom: string; + baseTo: string; + baseTreeFrom: string; + baseTreeTo: string; + choice: (typeof RESOLUTION_CHOICES)[number]; + fork?: { identity: string; alias: string }; + overlayDigestAfter: string; + kit: string; + fromVersion: string; + toVersion: string; + planToken: string; + approvedBy: "user"; +} + +export interface KitComposition { + extends: KitExtend[]; + include?: string[]; + resolutions?: KitResolutionRecord[]; +} + +export const KIT_COMPOSITION_LIMITS = { maxExtends: 16, maxInclude: 256, maxResolutions: 1000, maxText: 256, maxPath: 4096 } as const; + +/** An AgentKit kit ID: lowercase [a-z0-9] words joined by single hyphens. */ +const KIT_ID = /^[a-z0-9]+(-[a-z0-9]+)*$/; +const CATEGORY = /^conflict\.[a-z0-9_.-]{1,64}$/; +const TOKEN = /^[\x21-\x7e]{1,256}$/; +const isRecord = (v: unknown): v is Record => typeof v === "object" && v !== null && !Array.isArray(v); +const invalid = (path: string, detail: string) => kitError(KIT_CODES.documentInvalid, path, detail); + +function only(raw: Record, at: string, fields: string[]): void { + for (const key of Object.keys(raw)) if (!fields.includes(key)) throw invalid(`${at}.${key}`, "is not a known field"); +} + +function identity(value: unknown, at: string): void { + try { + if (typeof value !== "string") throw new Error("not a string"); + validateIdentity(value); + } catch (error) { + throw invalid(at, `must be @publisher/name (${error instanceof Error ? error.message : String(error)})`); + } +} + +function text(value: unknown, at: string, pattern: RegExp, what: string): void { + if (typeof value !== "string" || !pattern.test(value)) throw invalid(at, `must be ${what}`); +} + +const digest = (value: unknown, at: string) => text(value, at, DIGEST_PATTERN, "sha256:"); +const version = (value: unknown, at: string) => text(value, at, RELEASE_VERSION_PATTERN, "an exact release version"); +const kitId = (value: unknown, at: string) => { + text(value, at, KIT_ID, "a kit id (lowercase [a-z0-9] words joined by single hyphens)"); + if ((value as string).length > 64) throw invalid(at, "must be at most 64 bytes"); +}; + +function list(raw: unknown, at: string, min: number, max: number): unknown[] { + if (!Array.isArray(raw) || raw.length < min || raw.length > max) throw invalid(at, `must be an array of ${min} to ${max} entries`); + return raw; +} + +function unique(keys: string[], at: string): void { + const seen = new Set(); + keys.forEach((key, i) => { + if (seen.has(key)) throw invalid(`${at}[${i}]`, "duplicates an earlier entry"); + seen.add(key); + }); +} + +function checkExtend(raw: unknown, at: string): string { + if (!isRecord(raw)) throw invalid(at, "must be an object"); + version(raw.version, `${at}.version`); + if (raw.kit !== undefined && raw.skillx !== undefined) throw invalid(at, "names either kit or skillx, not both"); + if (raw.kit !== undefined) { + only(raw, at, ["kit", "version"]); + kitId(raw.kit, `${at}.kit`); + return `kit:${raw.kit as string}`; + } + only(raw, at, ["skillx", "version", "releaseDigest"]); + if (raw.skillx === undefined) throw invalid(at, "must name kit or skillx"); + identity(raw.skillx, `${at}.skillx`); + digest(raw.releaseDigest, `${at}.releaseDigest`); + return `skillx:${raw.skillx as string}`; +} + +function checkRecord(raw: unknown, at: string): string { + if (!isRecord(raw)) throw invalid(at, "must be an object"); + only(raw, at, ["identity", "path", "category", "baseFrom", "baseTo", "baseTreeFrom", "baseTreeTo", "choice", "fork", "overlayDigestAfter", "kit", "fromVersion", "toVersion", "planToken", "approvedBy"]); + identity(raw.identity, `${at}.identity`); + if (typeof raw.path !== "string" || raw.path === "" || raw.path.length > KIT_COMPOSITION_LIMITS.maxPath || /[\0-\x1f]/.test(raw.path)) { + throw invalid(`${at}.path`, "must be a non-empty path without control characters"); + } + text(raw.category, `${at}.category`, CATEGORY, "conflict."); + for (const key of ["baseFrom", "baseTo"]) if (raw[key] !== "absent") digest(raw[key], `${at}.${key}`); + for (const key of ["baseTreeFrom", "baseTreeTo", "overlayDigestAfter"]) digest(raw[key], `${at}.${key}`); + if (!(RESOLUTION_CHOICES as readonly unknown[]).includes(raw.choice)) throw invalid(`${at}.choice`, `must be one of: ${RESOLUTION_CHOICES.join(", ")}`); + if (raw.choice === "fork") { + if (!isRecord(raw.fork)) throw invalid(`${at}.fork`, "a fork record names the fork identity and alias"); + only(raw.fork, `${at}.fork`, ["identity", "alias"]); + identity(raw.fork.identity, `${at}.fork.identity`); + if (raw.fork.identity !== raw.identity) throw invalid(`${at}.fork.identity`, "must equal the record identity"); + text(raw.fork.alias, `${at}.fork.alias`, /^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/, "a directory alias"); + } else if (raw.fork !== undefined) throw invalid(`${at}.fork`, "is only allowed on a fork record"); + kitId(raw.kit, `${at}.kit`); + version(raw.fromVersion, `${at}.fromVersion`); + version(raw.toVersion, `${at}.toVersion`); + text(raw.planToken, `${at}.planToken`, TOKEN, "1 to 256 printable ASCII characters"); + if (raw.approvedBy !== "user") throw invalid(`${at}.approvedBy`, "must be user"); + return `${raw.identity as string}\0${raw.path}\0${raw.category as string}`; +} + +/** Validates `composition`; throws `PersonalKitError` (`kit.document_invalid`) at the offending path. */ +export function validateComposition(raw: unknown): KitComposition { + const at = "composition"; + if (!isRecord(raw)) throw invalid(at, "must be an object"); + only(raw, at, ["extends", "include", "resolutions"]); + const ext = list(raw.extends, `${at}.extends`, 1, KIT_COMPOSITION_LIMITS.maxExtends); + unique(ext.map((e, i) => checkExtend(e, `${at}.extends[${i}]`)), `${at}.extends`); + if (raw.include !== undefined) { + const include = list(raw.include, `${at}.include`, 0, KIT_COMPOSITION_LIMITS.maxInclude); + include.forEach((id, i) => identity(id, `${at}.include[${i}]`)); + unique(include as string[], `${at}.include`); + } + if (raw.resolutions !== undefined) { + const records = list(raw.resolutions, `${at}.resolutions`, 0, KIT_COMPOSITION_LIMITS.maxResolutions); + unique(records.map((r, i) => checkRecord(r, `${at}.resolutions[${i}]`)), `${at}.resolutions`); + } + return raw as unknown as KitComposition; +} diff --git a/packages/contracts/src/personal-kit/kit-document.ts b/packages/contracts/src/personal-kit/kit-document.ts index 60fcf92..1c54fc3 100644 --- a/packages/contracts/src/personal-kit/kit-document.ts +++ b/packages/contracts/src/personal-kit/kit-document.ts @@ -18,6 +18,8 @@ import { RELEASE_VERSION_PATTERN } from "../release-manifest"; import { satisfiesRange } from "../semver-range"; import { KIT_CODES, PERSONAL_KIT_SCHEMA, PersonalKitError, kitError } from "./codes"; import { sha256Hex } from "./bytes"; +import { validateComposition } from "./kit-composition"; +import type { KitComposition } from "./kit-composition"; import { DIGEST_PATTERN, validateOverlay } from "./overlay-envelope"; import type { SkillOverlay } from "./overlay-envelope"; @@ -45,6 +47,8 @@ export interface PersonalKitDocument { exclude: string[]; profile?: string; runtimes: Record; + /** AgentKit composition metadata; absent on older documents, whose digests are unchanged. */ + composition?: KitComposition; } export const KIT_DOCUMENT_LIMITS = { maxBytes: 1024 * 1024, maxSkills: 128, maxExclude: 256, maxRuntimes: 16 } as const; @@ -149,7 +153,7 @@ export function validateKitDocument(input: unknown): PersonalKitDocument { throw kitError(KIT_CODES.unsupportedVersion, "schema", `unsupported document schema ${input.schema}`); } if (input.schema !== PERSONAL_KIT_SCHEMA) throw invalid("schema", `must be ${PERSONAL_KIT_SCHEMA}`); - only(input, "", ["schema", "kit", "localId", "visibility", "parent", "skills", "exclude", "profile", "runtimes"]); + only(input, "", ["schema", "kit", "localId", "visibility", "parent", "skills", "exclude", "profile", "runtimes", "composition"]); const kit = identity(input.kit, "kit"); if (input.localId !== undefined) { if (input.localId !== `local/${nameOf(kit)}`) throw invalid("localId", `must be local/${nameOf(kit)}`); @@ -172,6 +176,7 @@ export function validateKitDocument(input: unknown): PersonalKitDocument { // Sorted and unique, so equivalent documents have one digest. const excluded = [...new Set(exclude as string[])].sort(compareUtf8); if (input.profile !== undefined && (typeof input.profile !== "string" || !PROFILE.test(input.profile))) throw invalid("profile", "must be lowercase [a-z0-9-], at most 64 characters"); + if (input.composition !== undefined) validateComposition(input.composition); const doc = { ...(input as unknown as PersonalKitDocument), skills, exclude: excluded, runtimes: checkRuntimes(input.runtimes) }; if (canonicalJsonBytes(doc).length > KIT_DOCUMENT_LIMITS.maxBytes) throw invalid("", "document exceeds 1 MiB"); return doc;