feat(personal-kits): answer kit.api_disabled while off and add an enablement smoke

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

@@ -1,6 +1,6 @@
 /**
  * HTTP boundary of the personal kit API (`/api/v1/kits/@handle/name/...`).
- * 404 while `SKILLX_PERSONAL_KITS_API` is off. The caller is the release
+ * 404 `kit.api_disabled` while `SKILLX_PERSONAL_KITS_API` is off. The caller is the release
  * viewer (unscoped account API key or web session; scoped publish keys are
  * refused); a kit is visible only to its owner, everyone else gets the same
  * 404 as an unknown kit. Reads count against `RELEASE_READ_LIMITER`, writes
@@ -9,7 +9,7 @@
  */
 import { PersonalKitError } from "@skillx/contracts";
 import { IDENTITY_PATTERN, validateIdentity } from "@skillx/skill-tree";
-import { ReleaseHttpError, errorResponse, notFoundResponse, releaseError } from "~/lib/releases/release-http";
+import { ReleaseHttpError, errorResponse, releaseError } from "~/lib/releases/release-http";
 import { checkReleaseRateLimit, rateLimitCaller } from "~/lib/releases/release-rate-limit";
 import { resolveViewer } from "~/lib/releases/release-read";
 import { NO_STORE, failureHeaders, readJsonObject, resolveLimitedActor } from "~/lib/releases/release-state-http";
@@ -27,6 +27,20 @@ export function personalKitsApiEnabled(env: { SKILLX_PERSONAL_KITS_API?: string
 }
 
 export const kitNotFound = () => releaseError(404, "not_found", "Not found");
+
+/** Code of the flag-off answer. */
+export const KIT_API_DISABLED = "kit.api_disabled";
+
+/**
+ * The flag-off answer. It stays a 404, so a client that only reads the
+ * status keeps treating the API as absent, and its code tells a client the
+ * deployment does not offer personal kits, so it can say so instead of
+ * "no such kit". It is answered before any authentication or lookup and
+ * names no kit, owner or state: only the deployment's flag, which the routes
+ * do not hide either (other methods still get the framework's 405).
+ */
+export const kitApiDisabledResponse = () =>
+  errorResponse(releaseError(404, KIT_API_DISABLED, "Personal kits are not enabled on this SkillX deployment"));
 export const badRequest = (message: string) => releaseError(400, "invalid_request", message);
 export const conflict = (code: string, message: string) => releaseError(409, code, message);
 
@@ -160,7 +174,7 @@ export async function serveKit(
   handler: (caller: KitCaller) => Promise<Response>,
   now: () => Date = () => new Date(),
 ): Promise<Response> {
-  if (!personalKitsApiEnabled(env)) return notFoundResponse();
+  if (!personalKitsApiEnabled(env)) return kitApiDisabledResponse();
   try {
     let userId: string;
     if (access === "read") {

apps/web/tests/personal-kits/kit-enablement-smoke.test.ts

@@ -0,0 +1,120 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { main, parseBaseSkill, runPersonalKitSmoke } from "../../../../scripts/smoke-personal-kits.mjs";
+import { BODY, createKitHarness, skillText } from "./kit-harness";
+import type { KitHarness } from "./kit-harness";
+
+vi.mock("~/lib/auth/session-helpers", () => ({ getSession: async () => null }));
+
+/**
+ * The enablement smoke run against the real kit and release route modules
+ * over the in-memory D1/R2: the local receipt that the smoke itself works.
+ * It says nothing about a deployed SkillX.
+ */
+let k: KitHarness;
+
+beforeEach(async () => {
+  k = await createKitHarness();
+  await k.publish("@alpha/demo", "1.0.0", { "SKILL.md": skillText(BODY) });
+});
+
+const run = (extra: Record<string, unknown> = {}) =>
+  runPersonalKitSmoke({
+    fetchImpl: k.serve,
+    base: "https://skillx.test",
+    ownerKey: k.ownerKey,
+    otherKey: k.otherKey,
+    handle: "alpha",
+    baseSkill: "@alpha/demo@1.0.0",
+    ...extra,
+  });
+
+const failed = (receipt: { checks: Array<{ ok: boolean }> }) => receipt.checks.filter((c) => !c.ok);
+
+describe("personal kit enablement smoke", () => {
+  it("passes every check against an enabled API and leaves one private kit", async () => {
+    const receipt = await run();
+    expect(receipt.enabled).toBe(true);
+    expect(receipt.kit).toBe("@alpha/agentkit-smoke");
+    expect(failed(receipt)).toEqual([]);
+    expect(receipt.checks.map((c: { label: string }) => c.label)).toEqual([
+      "unknown kit is 404 not_found",
+      "anonymous is 401",
+      "base release readable",
+      "smoke kit generation readable",
+      "store revision is 201",
+      "idempotent replay is 200",
+      "stale If-Match is 412",
+      "activate is 200 with a lock",
+      "lock reads back",
+      "other account read is 404",
+      "other account write is 404",
+      "other account activate is 404",
+      "other account cannot create under the handle",
+      "owner kit unchanged by the other account",
+    ]);
+    expect(receipt.ok).toBe(true);
+    expect(receipt.crossAccount).toBe(true);
+    expect(JSON.stringify(receipt)).not.toContain(k.ownerKey);
+    expect(JSON.stringify(receipt)).not.toContain(k.otherKey);
+    // The foreign writes stored nothing: one kit, one revision, all the owner's.
+    expect(k.count("personal_kits")).toBe(1);
+    expect(k.count("personal_kit_revisions")).toBe(1);
+  });
+
+  it("reuses the one smoke kit on a later run instead of creating another", async () => {
+    expect((await run()).ok).toBe(true);
+    const second = await run();
+    expect(failed(second)).toEqual([]);
+    expect(k.count("personal_kits")).toBe(1);
+    expect(k.count("personal_kit_revisions")).toBe(2);
+  });
+
+  it("treats a redirect as a failed check and never follows it", async () => {
+    const seen: Array<RequestInit | undefined> = [];
+    const fetchImpl = async (_input: string | URL | Request, init?: RequestInit) => {
+      seen.push(init);
+      return new Response(null, { status: 302, headers: { Location: "https://elsewhere.test/" } });
+    };
+    const receipt = await run({ fetchImpl });
+    expect(receipt.ok).toBe(false);
+    expect(receipt.checks).toEqual([{ label: "unknown kit is 404 not_found", ok: false, status: 302, code: "redirect" }]);
+    expect(seen.every((init) => init?.redirect === "manual")).toBe(true);
+  });
+
+  it("refuses a remote run without a second account key, before any request", async () => {
+    const fetchImpl = vi.fn();
+    const errors: string[] = [];
+    const io = { fetchImpl, log: () => {}, error: (m: string) => errors.push(m) };
+    const args = ["--base", "https://staging.skillx.test", "--handle", "alpha", "--base-skill", "@alpha/demo@1.0.0"];
+    expect(await main(args, { SKILLX_SMOKE_OWNER_KEY: "k" }, io)).toBe(1);
+    expect(await main([...args, "--allow-remote"], { SKILLX_SMOKE_OWNER_KEY: "k" }, io)).toBe(1);
+    expect(errors.join("\n")).toMatch(/SKILLX_SMOKE_OTHER_KEY/);
+    expect(fetchImpl).not.toHaveBeenCalled();
+  });
+
+  it("refuses a remote run over plain http even with both keys, before any request", async () => {
+    const fetchImpl = vi.fn();
+    const errors: string[] = [];
+    const io = { fetchImpl, log: () => {}, error: (m: string) => errors.push(m) };
+    const args = ["--base", "http://staging.skillx.test", "--allow-remote", "--handle", "alpha", "--base-skill", "@alpha/demo@1.0.0"];
+    expect(await main(args, { SKILLX_SMOKE_OWNER_KEY: "k", SKILLX_SMOKE_OTHER_KEY: "o" }, io)).toBe(1);
+    expect(errors.join("\n")).toMatch(/must be https/);
+    expect(fetchImpl).not.toHaveBeenCalled();
+  });
+
+  it("reports a flag-off deployment as disabled and writes nothing", async () => {
+    const receipt = await run({ fetchImpl: k.serveWith({ SKILLX_PERSONAL_KITS_API: "0" }) });
+    expect(receipt).toMatchObject({ enabled: false, ok: false, checks: [{ label: "api enabled", ok: false, status: 404, code: "kit.api_disabled" }] });
+    expect(k.count("personal_kits")).toBe(0);
+  });
+
+  it("refuses bad input before any request", async () => {
+    const fetchImpl = vi.fn();
+    await expect(run({ fetchImpl, ownerKey: "" })).rejects.toThrow(/owner API key/);
+    await expect(run({ fetchImpl, handle: "Alpha" })).rejects.toThrow(/--handle/);
+    await expect(run({ fetchImpl, baseSkill: "@alpha/demo" })).rejects.toThrow(/--base-skill/);
+    await expect(run({ fetchImpl, name: "Smoke_Kit" })).rejects.toThrow(/--kit-name/);
+    expect(fetchImpl).not.toHaveBeenCalled();
+    expect(parseBaseSkill("@alpha/demo@1.0.0-beta.1")).toEqual({ pkg: "@alpha/demo", version: "1.0.0-beta.1" });
+  });
+});

apps/web/tests/personal-kits/kit-harness.ts

@@ -8,6 +8,7 @@ import { compareUtf8, memoryTree, overlayBetween, PERSONAL_KIT_SCHEMA } from "@s
 import type { KitTree, SkillOverlay } from "@skillx/contracts";
 import { releasePublishScope } from "~/lib/auth/api-key-scopes";
 import { loader, action } from "~/routes/api.v1.kits";
+import { loader as manifestLoader } from "~/routes/api.v1.release-manifest";
 import { FakeRateLimiter } from "../releases/fake-rate-limiter";
 import { createHarness, file, tarOf } from "../releases/release-harness";
 
@@ -93,11 +94,34 @@ export async function createKitHarness() {
     return { status: res.status, headers: res.headers, body: (await res.json()) as Record<string, any> };
   }
 
+  /**
+   * A `fetch` over the real kit and release-manifest route modules, for
+   * clients (the enablement smoke) that speak HTTP, with `extra` over the
+   * harness env. Other paths are 599.
+   */
+  const serveWith = (extra: Record<string, unknown> = {}) => async (input: string | URL | Request, init?: RequestInit): Promise<Response> => {
+    const request = new Request(input, init);
+    const path = new URL(request.url).pathname;
+    const context = { cloudflare: { env: env(extra) } };
+    const kitPath = /^\/api\/v1\/kits\/(@[^/]+)\/([^/]+)(?:\/(.+))?$/.exec(path);
+    if (kitPath !== null) {
+      const params = { scope: kitPath[1], name: kitPath[2], ...(kitPath[3] === undefined ? {} : { "*": kitPath[3] }) };
+      const args = { request, params, context } as never;
+      return request.method === "GET" || request.method === "HEAD" ? loader(args) : action(args);
+    }
+    const manifest = /^\/api\/v1\/packages\/(@[^/]+)\/([^/]+)\/releases\/([^/]+)$/.exec(path);
+    if (manifest !== null) {
+      return manifestLoader({ request, params: { scope: manifest[1], slug: manifest[2], version: manifest[3] }, context } as never);
+    }
+    return new Response("no route in the kit harness", { status: 599 });
+  };
+  const serve = serveWith();
+
   const count = (table: string) => (h.d1.raw.prepare(`SELECT count(*) AS n FROM ${table}`).get() as { n: number }).n;
   const sql = (statement: string, ...params: Array<string | number | null>) => h.d1.raw.prepare(statement).run(...params);
 
   let keyCounter = 0;
   const idem = () => `idem-key-${String(++keyCounter).padStart(4, "0")}`;
 
-  return { h, ownerKey, otherKey, publish, treeOf, overlayTo, entry, doc, call, count, sql, idem };
+  return { h, ownerKey, otherKey, publish, treeOf, overlayTo, entry, doc, call, serve, serveWith, count, sql, idem };
 }

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

@@ -1,4 +1,5 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
+import { personalKitsApiEnabled } from "~/lib/personal-kits/kit-http";
 import { BODY, createKitHarness, skillText } from "./kit-harness";
 import type { KitHarness } from "./kit-harness";
 
@@ -24,10 +25,31 @@ async function createKit(files = MINE) {
 }
 
 describe("personal kit revisions", () => {
-  it("is 404 everywhere while the flag is off", async () => {
-    for (const res of [await k.call("", undefined, { env: { SKILLX_PERSONAL_KITS_API: "0" } }), await k.call("revisions", {}, { env: { SKILLX_PERSONAL_KITS_API: "0" } })]) {
+  it("is 404 kit.api_disabled everywhere while the flag is off, before any auth or lookup", async () => {
+    const off = { env: { SKILLX_PERSONAL_KITS_API: "0" } };
+    for (const res of [
+      await k.call("", undefined, off),
+      await k.call("revisions", {}, off),
+      await k.call("lock", undefined, { ...off, key: null }),
+      await k.call("", undefined, { ...off, kit: "@nobody/missing" }),
+    ]) {
       expect(res.status).toBe(404);
-      expect(res.body.error.code).toBe("not_found");
+      expect(res.body.error.code).toBe("kit.api_disabled");
+      expect(res.headers.get("Cache-Control")).toBe("no-store");
+    }
+    expect(k.count("personal_kits") + k.count("personal_kit_audit")).toBe(0);
+  });
+
+  it('enables on "1" or "true", trimmed and in any case, and nothing else', async () => {
+    for (const on of ["1", "true", "TRUE", " 1 ", "\ttrue\n"]) {
+      expect(personalKitsApiEnabled({ SKILLX_PERSONAL_KITS_API: on }), JSON.stringify(on)).toBe(true);
+      const res = await k.call("", undefined, { env: { SKILLX_PERSONAL_KITS_API: on } });
+      expect(res.body.error?.code, JSON.stringify(on)).toBe("not_found");
+    }
+    for (const off of [undefined, "", " ", "0", "false", "yes", "on", "1 1", "truee"]) {
+      expect(personalKitsApiEnabled({ SKILLX_PERSONAL_KITS_API: off }), JSON.stringify(off)).toBe(false);
+      const res = await k.call("", undefined, { env: { SKILLX_PERSONAL_KITS_API: off } });
+      expect(res.body.error?.code, JSON.stringify(off)).toBe("kit.api_disabled");
     }
   });
 

apps/web/wrangler.jsonc

@@ -68,7 +68,8 @@
     // Subrequests (D1 + R2 calls) one release publish may spend; above it the upload is refused with 413
     // before any R2 write. Workers Paid allows 10,000 per request; on a free account (1,000) set ~600.
     "SKILLX_RELEASE_SUBREQUEST_BUDGET": "5000",
-    // Personal kit API (/api/v1/kits/@handle/name/...). "0" = off (404 everywhere).
+    // Personal kit API (/api/v1/kits/@handle/name/...). On only for "1" or "true" (trimmed, any
+    // case); any other value is off and every kit path answers 404 kit.api_disabled.
     "SKILLX_PERSONAL_KITS_API": "0"
   }
 }

docs/personal-kits-api.md

@@ -10,9 +10,14 @@ without an explicit user approval.
 
 - Mount: `/api/v1/kits/@{handle}/{name}[/...]`. Source:
   `apps/web/app/routes/api.v1.kits.ts` and `apps/web/app/lib/personal-kits/`.
-- Feature flag: `SKILLX_PERSONAL_KITS_API` (`"1"` enables it). While it is off,
-  every path returns the JSON 404. Reading base releases also needs the release
-  stack (`SKILLX_RELEASES_API`, D1, R2).
+- Feature flag: `SKILLX_PERSONAL_KITS_API` (`"1"` or `"true"`, trimmed and in
+  any case, enables it, as for `SKILLX_RELEASES_API`; any other value or no
+  value is off). While it is off,
+  every path returns the JSON 404 `kit.api_disabled`, before any
+  authentication or lookup, so a client can say "this deployment does not offer
+  personal kits" instead of "no such kit". Reading base releases also needs the
+  release stack (`SKILLX_RELEASES_API`, D1, R2). Enabling and rolling back are
+  in [Enablement and rollback](#enablement-and-rollback).
 - Wire formats: `packages/contracts/src/personal-kit/`. The golden vector
   `packages/contracts/src/fixtures/personal-kit-golden.json` must re-derive byte for byte on both the
   SkillX (TypeScript) and AgentKit (Go) sides.
@@ -527,6 +532,44 @@ quarantined), `reviewed` moves only to `quarantined`, nothing returns to
 with the owner's consent proof in `approval_json`, which is never cleared. It never holds transcripts, prompts, environment
 or file content. Its rollback SQL is in its header.
 
+## Enablement and rollback
+
+The flag is the only switch; enabling it runs no migration (`0015` and `0016`
+are applied with the rest) and touches no existing data. The production value
+in `apps/web/wrangler.jsonc` stays `"0"` until an operator turns it on.
+
+1. Turn it on for one environment (`SKILLX_PERSONAL_KITS_API` = `"1"`) and
+   deploy that environment.
+2. Run the enablement smoke against it with a dedicated smoke owner account
+   whose publisher holds a published base skill, and a second account:
+   `SKILLX_SMOKE_OWNER_KEY=... SKILLX_SMOKE_OTHER_KEY=... node scripts/smoke-personal-kits.mjs --base <url> --allow-remote --handle <owner handle> --base-skill @pub/slug@version`.
+   A remote run refuses to start without `SKILLX_SMOKE_OTHER_KEY` or with a
+   base URL that is not `https`. The smoke
+   checks 401 for anonymous, idempotent replay, 412 on a stale `If-Match`,
+   activation and the lock read-back, then that the second account gets 404 on
+   reading the kit, storing a revision and activating it, gets 403
+   `kit.publisher_not_owned` creating a kit under the owner's handle, and leaves
+   the owner's kit unchanged. It never follows a redirect (a 3xx fails the
+   check) and prints a JSON receipt with no keys. Exit 2 means the deployment
+   still answers `kit.api_disabled`.
+3. Clean up: the API cannot delete a kit (revisions and audit rows are
+   immutable, and kits are `ON DELETE RESTRICT`), so the smoke reuses one
+   private kit, `@handle/agentkit-smoke` (`--kit-name` to choose another), and
+   each run only appends and activates one revision there; the receipt names
+   the kit and the run. Keep that kit on the dedicated smoke account; do not
+   point the smoke at a real user's handle. The second account's attempt to
+   create `@handle/smoke-squat-<random>` leaves no kit behind: it is refused
+   with 403 `kit.publisher_not_owned` before anything is written.
+4. Roll back by setting the flag to `"0"` and redeploying. Every kit path then
+   answers 404 `kit.api_disabled` again; stored revisions, locks and audit rows
+   stay in D1 untouched and come back unchanged when the flag returns. Clients
+   keep their local kits: AgentKit maps `kit.api_disabled` to "unsupported"
+   (exit 2) and never treats it as a deleted kit.
+
+`apps/web/tests/personal-kits/kit-enablement-smoke.test.ts` runs the same
+smoke against the real route modules over the in-memory test D1/R2; it proves
+the smoke and the flag-off answer, not a deployment.
+
 ## Error codes
 
 | Code | Status |
@@ -538,4 +581,5 @@ or file content. Its rollback SQL is in its header.
 | `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, updates or `If-Match`) | 400 |
-| `not_found` (flag off, not the owner, unknown id) | 404 |
+| `kit.api_disabled` (flag off, on every path) | 404 |
+| `not_found` (not the owner, unknown id) | 404 |

packages/cli/src/lib/api-client.ts

@@ -1,10 +1,20 @@
 import { getApiKey, getBaseUrl } from '../utils/config-store.js';
 
 export class ApiError extends Error {
-  constructor(public status: number, message: string) {
+  constructor(public status: number, message: string, public body = '') {
     super(message);
     this.name = 'ApiError';
   }
+
+  /** The `error.code` of a JSON error body, or null. */
+  get code(): string | null {
+    try {
+      const code = (JSON.parse(this.body) as { error?: { code?: unknown } })?.error?.code;
+      return typeof code === 'string' ? code : null;
+    } catch {
+      return null;
+    }
+  }
 }
 
 export async function apiRequest<T = unknown>(
@@ -28,7 +38,7 @@ export async function apiRequest<T = unknown>(
 
   if (!res.ok) {
     const body = await res.text();
-    throw new ApiError(res.status, `API error ${res.status}: ${body}`);
+    throw new ApiError(res.status, `API error ${res.status}: ${body}`, body);
   }
 
   return res.json() as Promise<T>;