From c7ed66e77ec310206020a107a6f440a7b1a3f89b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 16:58:59 -0700 Subject: [PATCH 1/4] Add test proving a skill name conflict on create is skipped, not fatal Reproduces the live incident: a half-seeded tenant re-running `workbench seed` hit "409 already exists" on skill creation and aborted the whole run, even though the hub's own error advice was to re-run the same command. A step that already succeeded must never kill a re-run. --- packages/hub-client/test/seed.test.ts | 41 +++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/packages/hub-client/test/seed.test.ts b/packages/hub-client/test/seed.test.ts index e65e9251..683af656 100644 --- a/packages/hub-client/test/seed.test.ts +++ b/packages/hub-client/test/seed.test.ts @@ -1281,6 +1281,47 @@ describe("default skills seeding", () => { expect(lines.some((l) => l.includes("already exists"))).toBe(true); }); + + test("a skill the by-name GET misses but the create route rejects as a conflict is skipped, not fatal", async () => { + // Reproduces the live incident: a half-seeded tenant re-running + // `workbench seed` hit `409 already exists` on skill creation and + // aborted the whole run, even though the hub's own error advice was + // "re-run: workbench seed". A step that already succeeded must never + // kill a re-run. + const { lines, log } = collector(); + const { push } = recordingPusher(); + const handler: FakeHandler = (method, path) => { + if ( + method === "GET" && + path.startsWith(`/api/tenants/${TENANT_ID}/skills/`) + ) + return { status: 404, data: {} }; + if (method === "POST" && path === `/api/tenants/${TENANT_ID}/skills`) + return { + status: 409, + data: { + code: "conflict", + message: `a skill named "${DEFAULT_SKILLS[0]!.name}" already exists in this workbench`, + }, + }; + const base = baseRoutes(method, path); + if (base) return base; + return workflowRoutes(method, path); + }; + + const echoOnly = DEFAULT_WORKFLOWS.filter((w) => w.assetName === "echo"); + await seedTenant( + args({ + api: fakeAPI(handler), + pushWorkflow: push, + log, + workflows: echoOnly, + confirmDeployments: false, + }), + ); + + expect(lines.some((l) => l.includes("already exists"))).toBe(true); + }); }); describe("seedCatalog", () => { From a30298cf02daf62c4c4d23a1ea4944893985ada6 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 16:59:00 -0700 Subject: [PATCH 2/4] Seed: treat a skill name conflict on create as a skip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit plantDefaultSkills checked for an existing skill by name before creating one, but never handled a 409 from the create call itself — so a row the lookup missed (an inherited/other-scope row, or a race with a concurrent seed pass) turned "already exists" into a fatal CliError instead of a skip. Every other seed step (grants, workflow assets, catalog rows) already treats a name-conflict 409 as already-done; skills now match. --- packages/hub-client/src/seed.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/hub-client/src/seed.ts b/packages/hub-client/src/seed.ts index e71cc897..f67804d2 100644 --- a/packages/hub-client/src/seed.ts +++ b/packages/hub-client/src/seed.ts @@ -521,6 +521,14 @@ async function plantDefaultSkills( }, cookies, ); + if (created.status === 409) { + // The by-name GET above missed a row that the create route still + // considers a conflict (an inherited/other-scope row, or a race + // with a concurrent seed pass) — "already exists" is a skip, not + // a fatal error, exactly like every other seed step's 409. + log(`skill ${skill.name} already exists (skipped)`); + continue; + } if (created.status !== 201) { throw new CliError( `the hub rejected the default skill "${skill.name}" with status ${created.status}: ${JSON.stringify(created.data)}`, From 591439219478fe8eb8b6f8569137118e847cdfec Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 16:59:51 -0700 Subject: [PATCH 3/4] Fix lint: avoid non-null assertion in the new skill-conflict test --- packages/hub-client/test/seed.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/hub-client/test/seed.test.ts b/packages/hub-client/test/seed.test.ts index 683af656..77bdf66f 100644 --- a/packages/hub-client/test/seed.test.ts +++ b/packages/hub-client/test/seed.test.ts @@ -1290,6 +1290,10 @@ describe("default skills seeding", () => { // kill a re-run. const { lines, log } = collector(); const { push } = recordingPusher(); + const conflictingSkillName = DEFAULT_SKILLS[0]?.name; + if (conflictingSkillName === undefined) { + throw new Error("DEFAULT_SKILLS must not be empty for this test"); + } const handler: FakeHandler = (method, path) => { if ( method === "GET" && @@ -1301,7 +1305,7 @@ describe("default skills seeding", () => { status: 409, data: { code: "conflict", - message: `a skill named "${DEFAULT_SKILLS[0]!.name}" already exists in this workbench`, + message: `a skill named "${conflictingSkillName}" already exists in this workbench`, }, }; const base = baseRoutes(method, path); From 004ebeaaad7ac02746e3fae9447467eda8bced28 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 17:00:08 -0700 Subject: [PATCH 4/4] Update docs: default skills and tool-registry-publish reconciliation Document the skill-conflict skip alongside the existing seed reconciliation properties, and explain why a failed publish can no longer strand a dangling package-registry asset, plus how re-seeding repairs a tenant whose asset survives from before that ordering shipped. --- docs/seed-reconciliation.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/docs/seed-reconciliation.md b/docs/seed-reconciliation.md index b8ca03c3..9414426e 100644 --- a/docs/seed-reconciliation.md +++ b/docs/seed-reconciliation.md @@ -66,6 +66,39 @@ plants `DEFAULT_ROUTINE_PRESETS` through `POST /routines` with a - A routine whose preset no longer ships is deleted only while pristine (`updatedAt` still equals `createdAt`); a member-touched row is kept. +## Default skills (`workbench seed`) + +`plantDefaultSkills` (`packages/hub-client/src/seed.ts`) plants each +`DEFAULT_SKILLS` entry through `POST /api/tenants/:id/skills`, after +first checking `GET /api/tenants/:id/skills/:name`. + +- An existing row the by-name GET finds is skipped outright. +- A `409` from the create call itself — a row the GET missed (an + inherited/other-scope row, or a race with a concurrent seed pass) — + is also a skip, never a fatal error. Every seed step treats + "already exists" as done, not as a reason to abort the run the + hub's own error advice told the operator to re-run. + +## Tool registry publish (`workbench seed`, ahead of every workflow deploy) + +`publishCorbitsToolsRegistry` (`packages/tool-registry-publish/src/publish.ts`) +finds-or-creates the tenant's `corbits-tools` package-registry asset, +then PUTs whatever tarball is missing. Two properties keep a failed +publish from stranding a usable-looking-but-empty asset: + +- `checkToolPackageFreshness` runs **before** the asset is ever + created — a version-bump violation aborts the publish with no HTTP + call made and no asset row planted, so this exact failure can never + leave a dangling registry asset behind on a fresh tenant again. +- Listing tarballs on an asset whose repo has no commits yet (a + never-published asset, or one whose row survived from before the + point above shipped) answers an empty list rather than throwing — + so a re-run of `publishCorbitsToolsRegistry` treats it exactly like + a brand-new registry and pushes every package, which is what + actually creates the repo's first commit. Repairing a tenant with + this history is the same operation as seeding one for the first + time: re-run `workbench seed`. + ## Env provider credentials (hub boot) `apps/hub/src/env-credential-plant.ts` delegates to