diff --git a/api/knowledge-fs-contract.lock.json b/api/knowledge-fs-contract.lock.json index 3b9a154826c..75e22bb79c5 100644 --- a/api/knowledge-fs-contract.lock.json +++ b/api/knowledge-fs-contract.lock.json @@ -1,6 +1,6 @@ { "schemaVersion": 5, - "subtreeTree": "af4a3d87a3ac4dcfd91a0ea6247de6609add0aae", + "subtreeTree": "222470dcafc7a985cfb78896c31180f21cf88d75", "openapiSha256": "3a712231fa850c4f5151bc283205da9062086a0b693f0d1ab01c2c526323f018", "capabilityV2AuthManifestSha256": "fc0a47e23cce12544882f0298522b4933002e892b84ce1815df7e81d36a7a0c7", "capabilityV2AuthTestVectorSha256": "ae0de37b1ff05c40f905cf17a7b410d8971acacf64db07d5ee3d6fecfa559ce3", diff --git a/knowledge-fs/.harness/changes/2026-08-17-metadata-write-deletion-admission.md b/knowledge-fs/.harness/changes/2026-08-17-metadata-write-deletion-admission.md new file mode 100644 index 00000000000..39df33c6a34 --- /dev/null +++ b/knowledge-fs/.harness/changes/2026-08-17-metadata-write-deletion-admission.md @@ -0,0 +1,42 @@ +# Metadata write deletion admission + +Date: 2026-08-17 + +## What changed + +- Metadata field creation, rename, and deletion now use the shared knowledge-space deletion + admission lock instead of querying a non-existent `knowledge_spaces.state` column. +- The shared admission verifies the current `lifecycle_state`, the space's `deletion_job_id`, and + the durable `deletion_jobs.active_slot` before any metadata mutation runs in the same transaction. +- Added PostgreSQL and TiDB regressions for the canonical admission SQL and for rejecting a write + while an active durable deletion job exists. + +## Why + +Creating a metadata field failed in the test environment with PostgreSQL error `42703` because the +repository queried `knowledge_spaces.state`. The current schema stores the lifecycle in +`lifecycle_state`; it has no `state` column. The database error escaped the domain layer and was +reported to the Console as `KNOWLEDGE_FS_INTERNAL_ERROR` with HTTP 503. + +Using the shared deletion admission fixes the schema mismatch and preserves the intended race-safe +write fence. A direct column substitution would still miss deletion jobs that have acquired their +durable active slot. + +## Verification + +- The new PostgreSQL and TiDB tests were first run against the previous implementation and failed + on the `state` SQL and missing deletion-job admission. +- `pnpm --dir knowledge-fs --filter @knowledge/api exec vitest run + src/knowledge-space-metadata-repository.test.ts` passed all 16 tests. +- The complete `@knowledge/api` suite passed: 410 files passed, 1 skipped; 4,494 tests passed, 3 + skipped. +- `pnpm --dir knowledge-fs --filter @knowledge/api typecheck` passed. +- `pnpm --dir knowledge-fs build` passed all 12 workspace builds. +- Focused Biome checks for both changed TypeScript files and `git diff --check` passed. + +## Risks and follow-up + +- A knowledge space with an active deletion job continues to return the existing metadata + not-found domain result; this change does not alter the public error contract. +- Reads and document metadata reconciliation are unchanged. Only metadata field catalog mutations + acquire the canonical deletion fence. diff --git a/knowledge-fs/packages/api/src/knowledge-space-metadata-repository.test.ts b/knowledge-fs/packages/api/src/knowledge-space-metadata-repository.test.ts index 09d2b06e965..b440992daf1 100644 --- a/knowledge-fs/packages/api/src/knowledge-space-metadata-repository.test.ts +++ b/knowledge-fs/packages/api/src/knowledge-space-metadata-repository.test.ts @@ -8,6 +8,7 @@ import type { import { describe, expect, it } from "vitest"; import { + KnowledgeSpaceMetadataNotFoundError, KnowledgeSpaceMetadataValidationError, createDatabaseKnowledgeSpaceMetadataRepository, } from "./knowledge-space-metadata-repository"; @@ -21,6 +22,105 @@ const now = "2026-08-10T12:00:00.000Z"; describe.each(["postgres", "tidb"] as const)( "database knowledge-space metadata repository (%s)", (dialect) => { + it("admits metadata writes through the canonical knowledge-space deletion gate", async () => { + const calls: DatabaseExecuteInput[] = []; + let inserted = false; + const database = testDatabase(dialect, async (input) => { + calls.push(input); + if (input.tableName === "knowledge_spaces") { + return { + rows: [ + { + deletion_job_id: null, + id: knowledgeSpaceId, + lifecycle_state: "active", + }, + ], + rowsAffected: 1, + }; + } + if (input.tableName === "deletion_jobs") { + return { rows: [], rowsAffected: 0 }; + } + if (input.operation === "insert") { + inserted = true; + return { rows: [], rowsAffected: 1 }; + } + if (input.sql.includes("COUNT(*)")) { + return { rows: [{ field_count: 0 }], rowsAffected: 1 }; + } + if (input.sql.includes("name") && input.sql.includes("LIMIT 1")) { + return { rows: [], rowsAffected: 0 }; + } + return { + rows: inserted ? [fieldRow({ binding_count: 0 })] : [], + rowsAffected: inserted ? 1 : 0, + }; + }); + const repository = createDatabaseKnowledgeSpaceMetadataRepository({ + database, + generateFieldId: () => fieldId, + maxListLimit: 100, + }); + + await expect( + repository.create({ + knowledgeSpaceId, + name: "priority", + now, + subjectId: "account:1", + tenantId, + type: "string", + }), + ).resolves.toMatchObject({ id: fieldId }); + + const spaceAdmission = calls.find((call) => call.tableName === "knowledge_spaces"); + expect(spaceAdmission?.sql).toContain("lifecycle_state"); + expect(spaceAdmission?.sql).toContain("deletion_job_id"); + expect(spaceAdmission?.sql).not.toMatch(/["`]state["`]/u); + expect(calls.some((call) => call.tableName === "deletion_jobs")).toBe(true); + }); + + it("rejects metadata writes while a durable deletion job is active", async () => { + const calls: DatabaseExecuteInput[] = []; + const database = testDatabase(dialect, async (input) => { + calls.push(input); + if (input.tableName === "knowledge_spaces") { + return { + rows: [ + { + deletion_job_id: null, + id: knowledgeSpaceId, + lifecycle_state: "active", + }, + ], + rowsAffected: 1, + }; + } + if (input.tableName === "deletion_jobs") { + return { rows: [{ id: "active-deletion-job" }], rowsAffected: 1 }; + } + return { rows: [], rowsAffected: 0 }; + }); + const repository = createDatabaseKnowledgeSpaceMetadataRepository({ + database, + generateFieldId: () => fieldId, + maxListLimit: 100, + }); + + await expect( + repository.create({ + knowledgeSpaceId, + name: "priority", + now, + subjectId: "account:1", + tenantId, + type: "string", + }), + ).rejects.toBeInstanceOf(KnowledgeSpaceMetadataNotFoundError); + expect(calls.some((call) => call.operation === "insert")).toBe(false); + }); + it("lists a bounded field catalog with binding counts and tenant-space keyset scope", async () => { let select: DatabaseExecuteInput | undefined; const database = testDatabase(dialect, async (input) => { @@ -60,7 +160,10 @@ describe.each(["postgres", "tidb"] as const)( const database = testDatabase(dialect, async (input) => { calls.push(input); if (input.tableName === "knowledge_spaces") { - return { rows: [{ id: knowledgeSpaceId }], rowsAffected: 1 }; + return { rows: [activeSpaceRow()], rowsAffected: 1 }; + } + if (input.tableName === "deletion_jobs") { + return { rows: [], rowsAffected: 0 }; } if (input.operation === "insert") { inserted = true; @@ -200,7 +303,10 @@ describe.each(["postgres", "tidb"] as const)( const database = testDatabase(dialect, async (input) => { calls.push(input); if (input.tableName === "knowledge_spaces") { - return { rows: [{ id: knowledgeSpaceId }], rowsAffected: 1 }; + return { rows: [activeSpaceRow()], rowsAffected: 1 }; + } + if (input.tableName === "deletion_jobs") { + return { rows: [], rowsAffected: 0 }; } if (input.tableName === "knowledge_space_metadata_fields" && input.operation === "select") { if (input.sql.includes("LIMIT 1")) return { rows: [], rowsAffected: 0 }; @@ -289,6 +395,14 @@ function fieldRow(overrides: DatabaseRow = {}): DatabaseRow { }; } +function activeSpaceRow(): DatabaseRow { + return { + deletion_job_id: null, + id: knowledgeSpaceId, + lifecycle_state: "active", + }; +} + function testDatabase( dialect: DatabaseAdapter["dialect"], execute: (input: DatabaseExecuteInput) => Promise, diff --git a/knowledge-fs/packages/api/src/knowledge-space-metadata-repository.ts b/knowledge-fs/packages/api/src/knowledge-space-metadata-repository.ts index 1a6464847b8..9063ce3b3a4 100644 --- a/knowledge-fs/packages/api/src/knowledge-space-metadata-repository.ts +++ b/knowledge-fs/packages/api/src/knowledge-space-metadata-repository.ts @@ -9,6 +9,7 @@ import type { import { nonnegativeSafeIntegerColumn, stringColumn } from "./database-row-utils"; import { databasePlaceholder, quoteDatabaseIdentifier } from "./database-sql-utils"; +import { lockKnowledgeSpaceForDeletionAdmission } from "./knowledge-space-deletion-admission"; export type KnowledgeSpaceMetadataFieldType = "number" | "string" | "time"; @@ -443,14 +444,9 @@ async function requireWritableSpace( executor: DatabaseExecutor, input: MetadataScope, ): Promise { - const result = await executor.execute({ - maxRows: 1, - operation: "select", - params: [input.tenantId, input.knowledgeSpaceId], - sql: `SELECT ${q(database, "id")} FROM ${q(database, "knowledge_spaces")} WHERE ${q(database, "tenant_id")} = ${p(database, 1)} AND ${q(database, "id")} = ${p(database, 2)} AND ${q(database, "state")} <> 'deleting' FOR UPDATE;`, - tableName: "knowledge_spaces", - }); - if (!result.rows[0]) throw new KnowledgeSpaceMetadataNotFoundError("Knowledge space not found"); + if (!(await lockKnowledgeSpaceForDeletionAdmission(database, executor, input))) { + throw new KnowledgeSpaceMetadataNotFoundError("Knowledge space not found"); + } } function mapField(row: DatabaseRow): KnowledgeSpaceMetadataField {