From 67ccfc116016d9d57349e987ed44ef0a8889a130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 23 Apr 2026 22:44:41 -0400 Subject: [PATCH] fix: enforce work product taxonomy --- src/main/api-server/routes-files.ts | 22 +-- src/main/database/documents-work-products.ts | 77 ++++++++--- src/main/prompt-builder/api-docs/files.en.md | 6 +- src/shared/contracts.ts | 1 + src/shared/contracts/schemas-work-products.ts | 45 +++++++ src/shared/types/enums.ts | 7 + src/shared/types/records-extended.ts | 12 +- .../work-product-primary-invariant.spec.ts | 3 +- .../work-product-url-policy.spec.ts | 126 ++++++++++++++++++ 9 files changed, 261 insertions(+), 38 deletions(-) create mode 100644 src/shared/contracts/schemas-work-products.ts diff --git a/src/main/api-server/routes-files.ts b/src/main/api-server/routes-files.ts index 6efe2e12..6f5b4dcf 100644 --- a/src/main/api-server/routes-files.ts +++ b/src/main/api-server/routes-files.ts @@ -1,5 +1,11 @@ import log from "electron-log/main.js"; import { z } from "zod"; +import { + workProductProviderEnum, + workProductReviewStateEnum, + workProductStatusEnum, + workProductTypeEnum, +} from "@shared/contracts"; import { requireParam, type AgentApiServer } from "./server-core"; import { normalizeExternalHttpUrl } from "../external-url-policy"; @@ -37,27 +43,27 @@ const updateDocumentSchema = z.object({ }); const createWorkProductSchema = z.object({ - type: z.string(), - provider: z.string(), + type: workProductTypeEnum, + provider: workProductProviderEnum, title: z.string(), projectId: z.string().optional(), externalId: z.string().optional(), url: workProductUrlSchema.optional(), - status: z.string().optional(), - reviewState: z.string().optional(), + status: workProductStatusEnum.optional(), + reviewState: workProductReviewStateEnum.optional(), isPrimary: z.boolean().optional(), summary: z.string().optional(), metadata: z.unknown().optional(), }); const updateWorkProductSchema = z.object({ - type: z.string().optional(), - provider: z.string().optional(), + type: workProductTypeEnum.optional(), + provider: workProductProviderEnum.optional(), externalId: z.string().optional(), title: z.string().optional(), url: workProductUrlSchema.optional(), - status: z.string().optional(), - reviewState: z.string().optional(), + status: workProductStatusEnum.optional(), + reviewState: workProductReviewStateEnum.optional(), isPrimary: z.boolean().optional(), summary: z.string().optional(), metadataJson: z.string().optional(), diff --git a/src/main/database/documents-work-products.ts b/src/main/database/documents-work-products.ts index ecadd97e..88881989 100644 --- a/src/main/database/documents-work-products.ts +++ b/src/main/database/documents-work-products.ts @@ -2,6 +2,16 @@ import { randomUUID } from "node:crypto"; import { statSync } from "node:fs"; import { isAbsolute, resolve as resolvePath, relative as relativePath } from "node:path"; import type { DatabaseSync, SQLInputValue } from "node:sqlite"; +import { + workProductProviderEnum, + workProductReviewStateEnum, + workProductStatusEnum, + workProductTypeEnum, + type WorkProductProvider, + type WorkProductReviewState, + type WorkProductStatus, + type WorkProductType, +} from "@shared/contracts"; import type { ReadAll, Transaction } from "./helpers"; import { nowIso } from "./helpers"; @@ -73,6 +83,22 @@ function joinWorkspaceRelative(root: string, rel: string): string | null { return joined; } +function parseWorkProductType(value: string): WorkProductType { + return workProductTypeEnum.parse(value); +} + +function parseWorkProductProvider(value: string): WorkProductProvider { + return workProductProviderEnum.parse(value); +} + +function parseWorkProductStatus(value: string): WorkProductStatus { + return workProductStatusEnum.parse(value); +} + +function parseWorkProductReviewState(value: string): WorkProductReviewState { + return workProductReviewStateEnum.parse(value); +} + export function buildWorkProductMethods( getDb: () => DatabaseSync, readAll: ReadAll, @@ -82,19 +108,22 @@ export function buildWorkProductMethods( createWorkProduct(input: { companyId: string; taskId: string | null; projectId?: string | null; workspaceId?: string | null; - type: string; provider: string; externalId?: string | null; - title: string; url?: string | null; status?: string; reviewState?: string; + type: WorkProductType; provider: WorkProductProvider; externalId?: string | null; + title: string; url?: string | null; status?: WorkProductStatus; reviewState?: WorkProductReviewState; isPrimary?: boolean; summary?: string | null; metadataJson?: string | null; createdByRunId?: string | null; }): string { return transaction(() => { const id = randomUUID(); const ts = nowIso(); - const status = input.status ?? "active"; + const type = parseWorkProductType(input.type); + const provider = parseWorkProductProvider(input.provider); + const status = input.status !== undefined ? parseWorkProductStatus(input.status) : "active"; + const reviewState = input.reviewState !== undefined ? parseWorkProductReviewState(input.reviewState) : "none"; if (input.isPrimary && input.taskId && status !== "archived") { getDb().prepare( "UPDATE work_products SET is_primary = 0, updated_at = ? WHERE company_id = ? AND task_id = ? AND type = ? AND status <> 'archived'", - ).run(ts, input.companyId, input.taskId, input.type); + ).run(ts, input.companyId, input.taskId, type); } getDb().prepare( `INSERT INTO work_products (id, company_id, project_id, task_id, workspace_id, type, provider, external_id, title, url, status, review_state, is_primary, summary, metadata_json, created_by_run_id, last_touched_by_run_id, created_at, updated_at) @@ -102,9 +131,9 @@ export function buildWorkProductMethods( ).run( id, input.companyId, input.projectId ?? null, input.taskId ?? null, input.workspaceId ?? null, - input.type, input.provider, input.externalId ?? null, + type, provider, input.externalId ?? null, input.title, input.url ?? null, status, - input.reviewState ?? "none", input.isPrimary ? 1 : 0, + reviewState, input.isPrimary ? 1 : 0, input.summary ?? null, input.metadataJson ?? null, input.createdByRunId ?? null, input.createdByRunId ?? null, ts, ts, ); @@ -114,16 +143,17 @@ export function buildWorkProductMethods( findWorkProductByExternalId( companyId: string, - provider: string, + provider: WorkProductProvider, externalId: string, workspaceId: string | null, ): { id: string; taskId: string | null } | null { + const parsedProvider = parseWorkProductProvider(provider); if (workspaceId) { const row = getDb() .prepare( "SELECT id, task_id as taskId FROM work_products WHERE company_id = ? AND workspace_id = ? AND provider = ? AND external_id = ? LIMIT 1", ) - .get(companyId, workspaceId, provider, externalId) as + .get(companyId, workspaceId, parsedProvider, externalId) as | { id: string; taskId: string | null } | undefined; return row ?? null; @@ -132,7 +162,7 @@ export function buildWorkProductMethods( .prepare( "SELECT id, task_id as taskId FROM work_products WHERE company_id = ? AND workspace_id IS NULL AND provider = ? AND external_id = ? LIMIT 1", ) - .get(companyId, provider, externalId) as + .get(companyId, parsedProvider, externalId) as | { id: string; taskId: string | null } | undefined; return row ?? null; @@ -142,14 +172,14 @@ export function buildWorkProductMethods( id: string, companyId: string, patch: { - type?: string; - provider?: string; + type?: WorkProductType; + provider?: WorkProductProvider; externalId?: string | null; workspaceId?: string | null; title?: string; url?: string | null; - status?: string; - reviewState?: string; + status?: WorkProductStatus; + reviewState?: WorkProductReviewState; isPrimary?: boolean; summary?: string | null; metadataJson?: string | null; @@ -165,8 +195,10 @@ export function buildWorkProductMethods( if (!current) return 0; const ts = nowIso(); - const nextType = patch.type ?? current.type; - const nextStatus = patch.status ?? current.status; + const nextType = patch.type !== undefined ? parseWorkProductType(patch.type) : parseWorkProductType(current.type); + const nextStatus = patch.status !== undefined ? parseWorkProductStatus(patch.status) : parseWorkProductStatus(current.status); + const parsedProvider = patch.provider !== undefined ? parseWorkProductProvider(patch.provider) : undefined; + const parsedReviewState = patch.reviewState !== undefined ? parseWorkProductReviewState(patch.reviewState) : undefined; const nextIsPrimary = patch.isPrimary ?? Boolean(current.isPrimary); if (nextIsPrimary && current.taskId && nextStatus !== "archived") { getDb().prepare( @@ -177,21 +209,21 @@ export function buildWorkProductMethods( const sets: string[] = ["updated_at = ?"]; const values: SQLInputValue[] = [ts]; const map: Array<[keyof typeof patch, string, (v: unknown) => SQLInputValue]> = [ - ["type", "type", (v) => (v as string) ?? null], - ["provider", "provider", (v) => (v as string) ?? null], + ["type", "type", () => nextType], + ["provider", "provider", () => parsedProvider ?? null], ["externalId", "external_id", (v) => (v as string | null) ?? null], ["workspaceId", "workspace_id", (v) => (v as string | null) ?? null], ["title", "title", (v) => (v as string) ?? null], ["url", "url", (v) => (v as string | null) ?? null], - ["status", "status", (v) => (v as string) ?? null], - ["reviewState", "review_state", (v) => (v as string) ?? null], + ["status", "status", () => nextStatus], + ["reviewState", "review_state", () => parsedReviewState ?? null], ["isPrimary", "is_primary", (v) => (v ? 1 : 0)], ["summary", "summary", (v) => (v as string | null) ?? null], ["metadataJson", "metadata_json", (v) => (v as string | null) ?? null], ["lastTouchedByRunId", "last_touched_by_run_id", (v) => (v as string | null) ?? null], ]; for (const [key, col, coerce] of map) { - if (key in patch) { + if (patch[key] !== undefined) { sets.push(`${col} = ?`); values.push(coerce(patch[key])); } @@ -318,11 +350,12 @@ export function buildWorkProductMethods( archiveWorkProductsByExternalPaths(input: { companyId: string; workspaceId: string | null; - provider: string; + provider: WorkProductProvider; paths: string[]; lastTouchedByRunId: string | null; }): Array<{ id: string; externalId: string }> { if (input.paths.length === 0) return []; + const provider = parseWorkProductProvider(input.provider); const ts = nowIso(); const CHUNK = 500; const workspaceClause = input.workspaceId @@ -336,7 +369,7 @@ export function buildWorkProductMethods( input.lastTouchedByRunId ?? null, ts, input.companyId, - input.provider, + provider, ]; if (input.workspaceId) params.push(input.workspaceId); params.push(...chunk); diff --git a/src/main/prompt-builder/api-docs/files.en.md b/src/main/prompt-builder/api-docs/files.en.md index 672fbe0d..dd1ea2c7 100644 --- a/src/main/prompt-builder/api-docs/files.en.md +++ b/src/main/prompt-builder/api-docs/files.en.md @@ -41,11 +41,11 @@ GET /api/companies/{{companyId}}/documents/{documentId}/revisions ### Work Products (Structured Deliverables) POST /api/issues/{issueId}/work-products Body: { - "type": "pull_request|branch|commit|artifact|document|preview_url|runtime_service", - "provider": "agentcompany|github|custom", + "type": "file|link|pull_request|branch|commit|artifact|document|preview_url|runtime_service", + "provider": "filesystem|external|agentcompany|github|custom", "title": "Work product title", "url": "https://... (optional)", - "status": "active|ready_for_review|approved|merged|closed|failed|draft", + "status": "active|ready_for_review|approved|merged|closed|failed|draft|archived", "reviewState": "none|needs_board_review|approved|changes_requested", "isPrimary": true, "summary": "Brief description of the deliverable" diff --git a/src/shared/contracts.ts b/src/shared/contracts.ts index 5420f87e..ce168962 100644 --- a/src/shared/contracts.ts +++ b/src/shared/contracts.ts @@ -7,5 +7,6 @@ export * from "./contracts/schemas-approvals"; export * from "./contracts/schemas-messages"; export * from "./contracts/schemas-execution"; export * from "./contracts/schemas-misc"; +export * from "./contracts/schemas-work-products"; export * from "./contracts/desktop-api"; export * from "./contracts/preload-forwarders"; diff --git a/src/shared/contracts/schemas-work-products.ts b/src/shared/contracts/schemas-work-products.ts new file mode 100644 index 00000000..d26a9e98 --- /dev/null +++ b/src/shared/contracts/schemas-work-products.ts @@ -0,0 +1,45 @@ +import { z } from "zod"; + +export const workProductTypeEnum = z.enum([ + "file", + "link", + "pull_request", + "branch", + "commit", + "artifact", + "document", + "preview_url", + "runtime_service", +]); + +export const workProductProviderEnum = z.enum([ + "filesystem", + "external", + "agentcompany", + "github", + "custom", +]); + +export const workProductStatusEnum = z.enum([ + "active", + "ready_for_review", + "approved", + "merged", + "closed", + "failed", + "draft", + "archived", +]); + +export const workProductReviewStateEnum = z.enum([ + "none", + "needs_board_review", + "approved", + "changes_requested", +]); + +export type WorkProductType = z.infer; +export type WorkProductProvider = z.infer; +export type WorkProductStatus = z.infer; +export type WorkProductReviewState = z.infer; + diff --git a/src/shared/types/enums.ts b/src/shared/types/enums.ts index 63077e7b..95a0c1dd 100644 --- a/src/shared/types/enums.ts +++ b/src/shared/types/enums.ts @@ -235,6 +235,13 @@ export type DocumentType = export type DocumentStatus = "draft" | "in_review" | "approved" | "archived" | "superseded"; +export type { + WorkProductType, + WorkProductProvider, + WorkProductStatus, + WorkProductReviewState, +} from "../contracts/schemas-work-products"; + export type KnowledgeCategory = | "lesson_learned" | "best_practice" diff --git a/src/shared/types/records-extended.ts b/src/shared/types/records-extended.ts index 3cef438d..b30d7846 100644 --- a/src/shared/types/records-extended.ts +++ b/src/shared/types/records-extended.ts @@ -23,6 +23,10 @@ import type { ThreadWaitingFor, ThreadNotifyOn, DepartmentId, + WorkProductType, + WorkProductProvider, + WorkProductStatus, + WorkProductReviewState, } from "./enums"; export interface SocialAccountRecord { @@ -103,13 +107,13 @@ export interface WorkProductRecord { projectId: string | null; taskId: string | null; workspaceId: string | null; - type: string; - provider: string; + type: WorkProductType; + provider: WorkProductProvider; externalId: string | null; title: string; url: string | null; - status: string; - reviewState: string; + status: WorkProductStatus; + reviewState: WorkProductReviewState; isPrimary: boolean; summary: string | null; metadataJson: string | null; diff --git a/tests/integration/work-product-primary-invariant.spec.ts b/tests/integration/work-product-primary-invariant.spec.ts index 681459d5..133c3ad6 100644 --- a/tests/integration/work-product-primary-invariant.spec.ts +++ b/tests/integration/work-product-primary-invariant.spec.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { DatabaseSync } from "node:sqlite"; import { afterEach, describe, expect, it } from "vitest"; import { AppDatabase } from "../../src/main/database"; +import type { WorkProductType } from "../../src/shared/types"; const tempDirs: string[] = []; @@ -49,7 +50,7 @@ async function createHarness() { return { db, companyId, taskId }; } -function createWorkProduct(db: AppDatabase, companyId: string, taskId: string, title: string, isPrimary = false, type = "file") { +function createWorkProduct(db: AppDatabase, companyId: string, taskId: string, title: string, isPrimary = false, type: WorkProductType = "file") { return db.core.documents.createWorkProduct({ companyId, taskId, diff --git a/tests/integration/work-product-url-policy.spec.ts b/tests/integration/work-product-url-policy.spec.ts index 282bf130..e092bfdf 100644 --- a/tests/integration/work-product-url-policy.spec.ts +++ b/tests/integration/work-product-url-policy.spec.ts @@ -93,6 +93,132 @@ async function createHarness() { } describe("work product URL API policy", () => { + it("rejects unknown work product taxonomy during creation", async () => { + const harness = await createHarness(); + try { + const cases = [ + { type: "spreadsheet", provider: "external", title: "Bad type" }, + { type: "link", provider: "slack", title: "Bad provider" }, + { type: "link", provider: "external", title: "Bad status", status: "ship_it" }, + { type: "link", provider: "external", title: "Bad review", reviewState: "rubber_stamp" }, + ]; + + for (const body of cases) { + const response = await fetch(`${harness.baseUrl}/api/issues/${harness.taskId}/work-products`, { + method: "POST", + headers: harness.headers, + body: JSON.stringify(body), + }); + + expect(response.status).toBe(400); + } + expect(harness.db.queryAll("select id from work_products")).toHaveLength(0); + } finally { + harness.server.stop(); + harness.db.close(); + } + }); + + it("rejects unknown work product taxonomy during updates", async () => { + const harness = await createHarness(); + try { + const id = harness.db.core.documents.createWorkProduct({ + companyId: harness.companyId, + taskId: harness.taskId, + type: "link", + provider: "external", + title: "External report", + status: "active", + reviewState: "none", + }); + const cases = [ + { type: "spreadsheet" }, + { provider: "slack" }, + { status: "ship_it" }, + { reviewState: "rubber_stamp" }, + ]; + + for (const body of cases) { + const response = await fetch(`${harness.baseUrl}/api/work-products/${id}`, { + method: "PATCH", + headers: harness.headers, + body: JSON.stringify(body), + }); + + expect(response.status).toBe(400); + } + const row = harness.db.queryAll<{ + type: string; + provider: string; + status: string; + review_state: string; + }>("select type, provider, status, review_state from work_products where id = ? limit 1", id)[0]; + expect(row).toEqual({ + type: "link", + provider: "external", + status: "active", + review_state: "none", + }); + } finally { + harness.server.stop(); + harness.db.close(); + } + }); + + it("rejects unknown work product taxonomy at the database boundary", async () => { + const harness = await createHarness(); + try { + expect(() => harness.db.core.documents.createWorkProduct({ + companyId: harness.companyId, + taskId: harness.taskId, + type: "link", + // @ts-expect-error verifies runtime validation for direct DB callers. + provider: "slack", + title: "Bad provider", + })).toThrow(); + expect(harness.db.queryAll("select id from work_products")).toHaveLength(0); + + expect(() => harness.db.core.documents.createWorkProduct({ + companyId: harness.companyId, + taskId: harness.taskId, + type: "link", + provider: "external", + title: "Empty status", + // @ts-expect-error verifies runtime validation for direct DB callers. + status: "", + })).toThrow(); + expect(harness.db.queryAll("select id from work_products")).toHaveLength(0); + + expect(() => harness.db.core.documents.createWorkProduct({ + companyId: harness.companyId, + taskId: harness.taskId, + type: "link", + provider: "external", + title: "Empty review state", + // @ts-expect-error verifies runtime validation for direct DB callers. + reviewState: "", + })).toThrow(); + expect(harness.db.queryAll("select id from work_products")).toHaveLength(0); + + const id = harness.db.core.documents.createWorkProduct({ + companyId: harness.companyId, + taskId: harness.taskId, + type: "link", + provider: "external", + title: "External report", + }); + expect(() => harness.db.core.documents.updateWorkProduct(id, harness.companyId, { + // @ts-expect-error verifies runtime validation for direct DB callers. + status: "ship_it", + })).toThrow(); + const row = harness.db.queryAll<{ status: string }>("select status from work_products where id = ? limit 1", id)[0]; + expect(row?.status).toBe("active"); + } finally { + harness.server.stop(); + harness.db.close(); + } + }); + it("rejects blank URLs during work product creation", async () => { const harness = await createHarness(); try {