Fix/name collision on import

.yarn/changelogs/service.0cbd66e9.md

@@ -0,0 +1,46 @@
+<!-- version-type: patch -->
+# service
+
+## 🐛 Bug Fixes
+
+### Reject duplicate IDs with `409 Conflict` on create endpoints
+
+Creating an entity with a client-supplied `id`/`name` that already exists now fails fast with a `409` response and a descriptive message instead of surfacing a generic store-level error (or, worse, silently overwriting state during a stack import). Affected endpoints:
+
+- `POST /github-repositories` — rejects when `body.id` matches an existing GitHub repository.
+- `POST /prerequisites` — rejects when `body.id` matches an existing prerequisite.
+- `POST /services` — rejects when `body.id` matches an existing service definition.
+- `POST /stacks` — rejects when `body.name` matches an existing stack.
+
+### Pre-flight conflict check on stack import
+
+`POST /stacks/import` now collects all id collisions (`stack.name`, `services[].id`, `repositories[].id`, `prerequisites[].id`) before touching any store, and returns a single `409` listing every conflicting id. Previously, the importer could partially apply changes and then enter the rollback path, which would delete the pre-existing records that shared the colliding ids.
+
+### Atomic rollback on partial-create failures
+
+Multi-step create flows now undo the records they have already inserted when a later step fails, so a failed create no longer leaves orphaned rows in the data store:
+
+- `POST /prerequisites` — removes the prerequisite if seeding the initial `PrerequisiteCheckResult` throws.
+- `POST /services` — removes the partially-inserted `ServiceDefinition`, `ServiceConfig`, `ServiceStatus`, and link rows if any later insert throws.
+- `POST /stacks` — removes the `StackDefinition` if inserting the `StackConfig` throws.
+
+Errors that are not already a `RequestError` are wrapped into a `500` with the original message preserved.
+
+## 🔧 Refactor
+
+The duplicate-id check and the multi-step create + rollback logic for `POST /services`, `POST /stacks`, and `POST /github-repositories` were moved out of the inline `setup-*-rest-api.ts` handlers into named, testable `RequestAction` modules — matching the existing `ImportStackAction` / `CreatePrerequisiteAction` shape:
+
+- `service/src/app-models/services/actions/create-service-action.ts` — exports `CreateServiceAction`, with `assertNoServiceIdCollision`, `buildServiceArtifacts`, and `createServiceWithRollback` as module-local helpers.
+- `service/src/app-models/stacks/actions/create-stack-action.ts` — exports `CreateStackAction` with an `assertNoStackNameCollision` helper.
+- `service/src/app-models/github-repositories/actions/create-github-repo-action.ts` — exports `CreateGitHubRepoAction`.
+
+The three `setup-*-rest-api.ts` files now wire the action through `Validate(...)` with no inline logic, which removes the High/Medium complexity findings introduced by the original fix on the services and stacks endpoints. No behavioral change.
+
+## 🧪 Tests
+
+- Added prerequisite-creation specs covering the duplicate-id `409` path and the check-result rollback path.
+- Added stack-import specs covering duplicate `stack.name` and cross-stack duplicate `service.id` rejection, asserting the pre-existing records remain untouched.
+- Added `create-github-repo-action.spec.ts` covering happy-path persistence (`201`) and duplicate-id `409`.
+- Added `create-stack-action.spec.ts` covering duplicate-name `409` and the `StackDefinition` rollback when `StackConfig.add` throws.
+- Added `create-service-action.spec.ts` covering duplicate-id `409`, full rollback when the `ServiceStatus` insert throws, and link-row rollback when a later `ServiceDependencyLink` insert throws.
+- Extended `import-export-actions.spec.ts` with duplicate-`repository.id` and combined service+repo+prerequisite collision tests, asserting `assertNoImportConflicts` aggregates every conflicting id into a single `409` message.

.yarn/changelogs/stack-craft.0cbd66e9.md

@@ -0,0 +1,12 @@
+<!-- version-type: patch -->
+# stack-craft
+
+## 🐛 Bug Fixes
+
+### Prevent silent overwrites when creating or importing entities with duplicate IDs
+
+Creating a GitHub repository, prerequisite, service, or stack with an `id`/`name` that already exists now fails with a clear `409 Conflict` error instead of overwriting the existing record or returning an opaque store error. Importing a stack performs the same collision check up-front across the stack name and all nested service, repository, and prerequisite ids, so a conflicting import is rejected before any data is mutated.
+
+Partially-created entities are also rolled back when a later step in the create flow fails (e.g. service definition + config + status + links), so failed creates no longer leave orphan rows behind.
+
+See the `service` changelog for the affected endpoints and response details.

.yarn/versions/0cbd66e9.yml

@@ -0,0 +1,3 @@
+releases:
+  service: patch
+  stack-craft: patch

service/src/app-models/github-repositories/actions/create-github-repo-action.spec.ts

@@ -0,0 +1,65 @@
+import { getDataSetFor } from '@furystack/repository'
+import { describe, expect, it } from 'vitest'
+
+import { createMockActionContext, withTestInjector } from '../../../test-helpers.js'
+import { GitHubRepositoryDataSet } from '../../data-store/tokens.js'
+import { CreateGitHubRepoAction } from './create-github-repo-action.js'
+
+type CreateRepoBody = Parameters<typeof CreateGitHubRepoAction>[0] extends { getBody: () => Promise<infer B> }
+  ? B
+  : never
+
+const baseRepo = {
+  id: 'repo-1',
+  stackName: 'my-stack',
+  url: 'https://github.com/example/repo',
+  displayName: 'Example Repo',
+  description: '',
+}
+
+describe('CreateGitHubRepoAction', () => {
+  it('should persist the repository and return it with statusCode 201', async () => {
+    await withTestInjector(async ({ elevated }) => {
+      const result = await CreateGitHubRepoAction({
+        ...createMockActionContext<CreateRepoBody>({ injector: elevated, body: baseRepo }),
+        request: {} as never,
+      })
+
+      expect(result.statusCode).toBe(201)
+      expect(result.chunk.id).toBe('repo-1')
+
+      const stored = await getDataSetFor(elevated, GitHubRepositoryDataSet).find(elevated, {})
+      expect(stored).toHaveLength(1)
+      expect(stored[0]?.url).toBe('https://github.com/example/repo')
+    })
+  })
+
+  it('should reject with 409 when a repository with the same id already exists', async () => {
+    await withTestInjector(async ({ elevated }) => {
+      const ts = new Date().toISOString()
+      await getDataSetFor(elevated, GitHubRepositoryDataSet).add(elevated, {
+        ...baseRepo,
+        displayName: 'Original',
+        createdAt: ts,
+        updatedAt: ts,
+      })
+
+      await expect(
+        CreateGitHubRepoAction({
+          ...createMockActionContext<CreateRepoBody>({
+            injector: elevated,
+            body: { ...baseRepo, displayName: 'Duplicate' },
+          }),
+          request: {} as never,
+        }),
+      ).rejects.toMatchObject({
+        message: expect.stringContaining('"repo-1" already exists'),
+        responseCode: 409,
+      })
+
+      const stored = await getDataSetFor(elevated, GitHubRepositoryDataSet).find(elevated, {})
+      expect(stored).toHaveLength(1)
+      expect(stored[0]?.displayName).toBe('Original')
+    })
+  })
+})

service/src/app-models/github-repositories/actions/create-github-repo-action.ts

@@ -0,0 +1,35 @@
+import type { WithOptionalId } from '@furystack/core'
+import { RequestError } from '@furystack/rest'
+import { JsonResult, type RequestAction } from '@furystack/rest-service'
+import type { GitHubRepoWritableFields, PostGitHubRepoEndpoint } from 'common'
+import { GitHubRepository } from 'common'
+
+import { legacyRepository as getRepository } from '../../../utils/legacy-repository.js'
+
+/**
+ * POST action that registers a new {@link GitHubRepository}.
+ *
+ * Rejects with `409 Conflict` when the client-supplied `body.id` matches an
+ * existing record, so the store-level `add` cannot silently overwrite it.
+ */
+export const CreateGitHubRepoAction: RequestAction<PostGitHubRepoEndpoint> = async ({ injector, getBody }) => {
+  const body = await getBody()
+  const ds = getRepository(injector).getDataSetFor<
+    GitHubRepository,
+    'id',
+    WithOptionalId<GitHubRepoWritableFields, 'id'>
+  >(GitHubRepository, 'id')
+
+  if (body.id !== undefined) {
+    const existing = await ds.get(injector, body.id)
+    if (existing) {
+      throw new RequestError(`A GitHub repository with id "${body.id}" already exists. Choose a different id.`, 409)
+    }
+  }
+
+  const { created } = await ds.add(injector, body)
+  if (!created?.length) {
+    throw new RequestError('Repository not created', 500)
+  }
+  return JsonResult(created[0], 201)
+}

service/src/app-models/github-repositories/setup-github-repos-rest-api.ts

@@ -5,7 +5,6 @@ import {
   createGetCollectionEndpoint,
   createGetEntityEndpoint,
   createPatchEndpoint,
-  createPostEndpoint,
   useRestService,
   Validate,
 } from '@furystack/rest-service'
@@ -16,6 +15,7 @@ import { GitHubRepositoryDataSet } from '../data-store/tokens.js'
 import { getCorsOptions } from '../../get-cors-options.js'
 import { getHost } from '../../get-host.js'
 import { getPort } from '../../get-port.js'
+import { CreateGitHubRepoAction } from './actions/create-github-repo-action.js'
 import { ValidateRepoAction } from './actions/validate-repo-action.js'
 
 export const setupGitHubReposRestApi = async (injector: Injector) => {
@@ -40,7 +40,7 @@ export const setupGitHubReposRestApi = async (injector: Injector) => {
         '/github-repositories': Validate({
           schema: githubReposApiSchema,
           schemaName: 'PostGitHubRepoEndpoint',
-        })(createPostEndpoint(GitHubRepositoryDataSet)),
+        })(CreateGitHubRepoAction),
         '/github-repositories/:id/validate': Validate({
           schema: githubReposApiSchema,
           schemaName: 'ValidateRepoEndpoint',

service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.spec.ts

@@ -46,6 +46,74 @@ describe('CreatePrerequisiteAction', () => {
     })
   })
 
+  it('should reject with 409 when a prerequisite with the same id already exists', async () => {
+    await withTestInjector(async ({ elevated }) => {
+      const ts = new Date().toISOString()
+      await getDataSetFor(elevated, PrerequisiteDataSet).add(elevated, {
+        id: 'existing-prereq',
+        stackName: 'stack',
+        name: 'Existing',
+        type: 'custom-script',
+        config: { script: 'echo existing' },
+        installationHelp: '',
+        createdAt: ts,
+        updatedAt: ts,
+      })
+
+      mockedReadPostBody.mockResolvedValue({
+        id: 'existing-prereq',
+        stackName: 'stack',
+        name: 'Duplicate',
+        type: 'custom-script' as const,
+        config: { script: 'echo dup' },
+        installationHelp: '',
+      })
+
+      await expect(
+        CreatePrerequisiteAction({
+          ...createMockActionContext({ injector: elevated }),
+          request: {} as never,
+        }),
+      ).rejects.toMatchObject({
+        message: expect.stringContaining('"existing-prereq" already exists'),
+        responseCode: 409,
+      })
+
+      const prereqs = await getDataSetFor(elevated, PrerequisiteDataSet).find(elevated, {})
+      expect(prereqs).toHaveLength(1)
+      expect(prereqs[0].name).toBe('Existing')
+    })
+  })
+
+  it('should roll back the prerequisite when seeding the check result fails', async () => {
+    await withTestInjector(async ({ elevated }) => {
+      mockedReadPostBody.mockResolvedValue({
+        id: 'rollback-prereq',
+        stackName: 'stack',
+        name: 'Rollback',
+        type: 'custom-script' as const,
+        config: { script: 'echo rollback' },
+        installationHelp: '',
+      })
+
+      const checkResultDs = getDataSetFor(elevated, PrerequisiteCheckResultDataSet)
+      vi.spyOn(checkResultDs, 'add').mockRejectedValueOnce(new Error('check-result store down'))
+
+      await expect(
+        CreatePrerequisiteAction({
+          ...createMockActionContext({ injector: elevated }),
+          request: {} as never,
+        }),
+      ).rejects.toMatchObject({
+        message: expect.stringContaining('check-result store down'),
+        responseCode: 500,
+      })
+
+      const prereqs = await getDataSetFor(elevated, PrerequisiteDataSet).find(elevated, {})
+      expect(prereqs).toHaveLength(0)
+    })
+  })
+
   it('should throw 500 when prerequisite creation returns empty', async () => {
     await withTestInjector(async ({ elevated }) => {
       mockedReadPostBody.mockResolvedValue({

service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.ts

@@ -19,18 +19,36 @@ export const CreatePrerequisiteAction: RequestAction<PostPrerequisiteEndpoint> =
   const checkResultDs = repository.getDataSetFor(PrerequisiteCheckResult, 'prerequisiteId')
 
   const body = await readPostBody<WithOptionalId<PrerequisiteWritableFields, 'id'>>(request)
+
+  if (body.id !== undefined) {
+    const existing = await prereqDs.get(injector, body.id)
+    if (existing) {
+      throw new RequestError(`A prerequisite with id "${body.id}" already exists. Choose a different id.`, 409)
+    }
+  }
+
   const { created } = await prereqDs.add(injector, body)
   if (!created?.length) {
     throw new RequestError('Prerequisite not created', 500)
   }
 
   const newPrereq = created[0]
-  await checkResultDs.add(injector, {
-    prerequisiteId: newPrereq.id,
-    status: 'unchecked',
-    output: '',
-    checkedAt: '',
-  })
+  try {
+    await checkResultDs.add(injector, {
+      prerequisiteId: newPrereq.id,
+      status: 'unchecked',
+      output: '',
+      checkedAt: '',
+    })
+  } catch (error) {
+    await prereqDs.remove(injector, newPrereq.id).catch(() => undefined)
+    throw error instanceof RequestError
+      ? error
+      : new RequestError(
+          `Failed to create prerequisite: ${error instanceof Error ? error.message : 'unknown error'}`,
+          500,
+        )
+  }
 
   return JsonResult(newPrereq, 201)
 }