update

src/shared/approval-documents.ts

@@ -4,6 +4,38 @@ export interface ApprovalDocumentReference {
   type?: string
 }
 
+export type ApprovalPayloadInspection =
+  | { kind: "empty" }
+  | { kind: "ok"; payload: Record<string, unknown> }
+  | { kind: "malformed"; reason: string }
+
+/**
+ * Inspect an approval payloadJson and classify it. Unlike the tolerant
+ * extractors (which collapse malformed rows to "no attachments"), this
+ * returns an explicit "malformed" signal so the inspector can surface a
+ * dignified error card instead of rendering as if nothing was attached.
+ *
+ * The HTTP route + file_approval tool both reject malformed payloads at
+ * write time via mergeApprovalAttachments. This detector exists for the
+ * read path, where a legacy row with a broken payload could otherwise
+ * silently render as empty.
+ */
+export function inspectApprovalPayload(payloadJson: string | null | undefined): ApprovalPayloadInspection {
+  if (payloadJson == null || payloadJson.length === 0) return { kind: "empty" }
+  let parsed: unknown
+  try {
+    parsed = JSON.parse(payloadJson)
+  } catch (error) {
+    const detail = error instanceof Error ? error.message : String(error)
+    return { kind: "malformed", reason: `payloadJson is not valid JSON (${detail})` }
+  }
+  if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
+    const actual = Array.isArray(parsed) ? "array" : parsed === null ? "null" : typeof parsed
+    return { kind: "malformed", reason: `payloadJson must be a JSON object (got ${actual})` }
+  }
+  return { kind: "ok", payload: parsed as Record<string, unknown> }
+}
+
 function parsePayload(payloadJson: string | null): Record<string, unknown> | null {
   if (!payloadJson) return null
   try {
@@ -16,33 +48,55 @@ function parsePayload(payloadJson: string | null): Record<string, unknown> | nul
   }
 }
 
+/**
+ * Extract document references from an approval's persisted payloadJson.
+ *
+ * Priority order (canonical → legacy), with later sources providing
+ * enrichment only for ids introduced by earlier sources:
+ *   1. payload.documentIds — canonical schema top-level field (written by
+ *      mergeApprovalAttachments). This is the authoritative id set.
+ *   2. payload.documentId / documentTitle / documentType — singular legacy
+ *      shape; kept for back-compat with old rows.
+ *   3. payload.documents[] — legacy rich shape; used to enrich title/type
+ *      metadata on ids already present from the canonical list, and to
+ *      contribute ids that pre-date the canonical field.
+ */
 export function extractApprovalDocumentReferences(payloadJson: string | null): ApprovalDocumentReference[] {
   const payload = parsePayload(payloadJson)
   if (!payload) return []
 
   const refs = new Map<string, ApprovalDocumentReference>()
-  const addRef = (id: unknown, title?: unknown, type?: unknown) => {
+  const addId = (id: unknown) => {
     if (typeof id !== "string" || id.length === 0) return
+    if (!refs.has(id)) refs.set(id, { id })
+  }
+  const enrich = (id: unknown, title?: unknown, type?: unknown) => {
+    if (typeof id !== "string" || id.length === 0) return
+    const prior = refs.get(id) ?? { id }
     refs.set(id, {
       id,
-      title: typeof title === "string" && title.length > 0 ? title : undefined,
-      type: typeof type === "string" && type.length > 0 ? type : undefined,
+      title: prior.title ?? (typeof title === "string" && title.length > 0 ? title : undefined),
+      type: prior.type ?? (typeof type === "string" && type.length > 0 ? type : undefined),
     })
   }
 
-  addRef(payload.documentId, payload.documentTitle, payload.documentType)
-
+  // 1. Canonical top-level list first — establishes the authoritative ids.
   const documentIds = payload.documentIds
   if (Array.isArray(documentIds)) {
-    for (const entry of documentIds) addRef(entry)
+    for (const entry of documentIds) addId(entry)
   }
 
+  // 2. Legacy singular shape.
+  enrich(payload.documentId, payload.documentTitle, payload.documentType)
+
+  // 3. Legacy rich shape — enriches metadata on canonical ids and adds
+  // ids that only ever lived in the legacy form.
   const documents = payload.documents
   if (Array.isArray(documents)) {
     for (const entry of documents) {
       if (!entry || typeof entry !== "object" || Array.isArray(entry)) continue
       const record = entry as Record<string, unknown>
-      addRef(record.id, record.title, record.type)
+      enrich(record.id, record.title, record.type)
     }
   }
 
@@ -52,3 +106,35 @@ export function extractApprovalDocumentReferences(payloadJson: string | null): A
 export function extractApprovalDocumentIds(payloadJson: string | null): string[] {
   return extractApprovalDocumentReferences(payloadJson).map((entry) => entry.id)
 }
+
+/**
+ * Mirror of extractApprovalDocumentReferences for work products. Work products
+ * are filesystem-backed (or URL-backed) artifacts — the board inspector opens
+ * them via the OS default handler rather than editing them inline.
+ *
+ * Canonical source is payload.workProductIds (schema top-level, folded in by
+ * mergeApprovalAttachments). Legacy payload.workProducts[] is still read for
+ * back-compat but new callers must not write it.
+ */
+export function extractApprovalWorkProductIds(payloadJson: string | null): string[] {
+  const payload = parsePayload(payloadJson)
+  if (!payload) return []
+  const out = new Set<string>()
+  // Canonical first.
+  const workProductIds = payload.workProductIds
+  if (Array.isArray(workProductIds)) {
+    for (const entry of workProductIds) {
+      if (typeof entry === "string" && entry.length > 0) out.add(entry)
+    }
+  }
+  // Legacy rich shape; back-compat only.
+  const workProducts = payload.workProducts
+  if (Array.isArray(workProducts)) {
+    for (const entry of workProducts) {
+      if (!entry || typeof entry !== "object" || Array.isArray(entry)) continue
+      const record = entry as Record<string, unknown>
+      if (typeof record.id === "string" && record.id.length > 0) out.add(record.id)
+    }
+  }
+  return [...out]
+}

src/shared/contracts/schemas-approvals.ts

@@ -1,6 +1,14 @@
 import { z } from "zod";
 import { optionalId } from "./helpers";
 
+// Schema-typed attachment pointers. Agents used to jam everything into the
+// free-form `payloadJson` string, which meant the board UI had to parse JSON
+// blind and hope the keys matched. documentIds / workProductIds make the
+// attachment contract schema-visible: validators see them, the HTTP route
+// folds them back into payloadJson for persistence, and the inspector renders
+// them deterministically in the "linked documents" panel on the first screen.
+// payloadJson remains as the escape hatch for type-specific structured
+// fields (social post drafts, hire proposals, etc.).
 export const approvalInputSchema = z.object({
   companyId: z.string().uuid(),
   relatedTaskId: optionalId,
@@ -9,9 +17,66 @@ export const approvalInputSchema = z.object({
   type: z.enum(["hire_agent", "approve_ceo_strategy", "dangerous_command", "secret_access", "budget_request", "promotion", "termination", "vendor_approval", "document_review", "production_deploy", "incident_escalation", "social_post", "deliverable_review", "budget_override", "plan_review", "scope_review", "draft_review", "strategy_review", "exception_review"]),
   payloadSummary: z.string().min(2).max(280),
   impactSummary: z.string().min(2).max(400),
+  // payloadJson: type-specific structured fields only (social draft, hire
+  // proposal, etc). DEPRECATED: embedding `documentIds` / `workProductIds` /
+  // `documents[]` / `workProducts[]` here. Use the top-level schema fields
+  // below — they are the canonical source of truth for attachment pointers.
+  // Legacy embedded mirrors are still read for back-compat but new callers
+  // must not write them.
   payloadJson: z.string().nullable().optional(),
+  // CANONICAL attachment pointers. mergeApprovalAttachments folds these into
+  // payloadJson at persist time so on-disk rows have a single readable shape,
+  // but the schema top-level is the authoritative contract.
+  documentIds: z.array(z.string().uuid()).optional(),
+  workProductIds: z.array(z.string().uuid()).optional(),
 });
 
+/**
+ * Merge canonical documentIds / workProductIds (schema top-level) into
+ * payloadJson so downstream readers (ApprovalDocumentsPanel,
+ * extractApprovalDocumentReferences) see a single source of truth.
+ *
+ * Throws if payloadJson is non-empty but not valid JSON-object: the previous
+ * behavior stuffed the opaque string under `rawPayload` which hid producer
+ * bugs (an agent passing a stringified-twice JSON, or a literal "undefined")
+ * and silently broke attachment rendering. Let it crash at the boundary.
+ */
+export function mergeApprovalAttachments(
+  payloadJson: string | null | undefined,
+  documentIds: string[] | undefined,
+  workProductIds: string[] | undefined,
+): string | null {
+  const hasDocs = Array.isArray(documentIds) && documentIds.length > 0;
+  const hasProducts = Array.isArray(workProductIds) && workProductIds.length > 0;
+  if (!hasDocs && !hasProducts) return payloadJson ?? null;
+
+  let base: Record<string, unknown> = {};
+  if (payloadJson) {
+    let parsed: unknown;
+    try {
+      parsed = JSON.parse(payloadJson);
+    } catch (error) {
+      const detail = error instanceof Error ? error.message : String(error);
+      throw new Error(
+        `mergeApprovalAttachments: payloadJson is not valid JSON (${detail}). ` +
+          `payloadJson must be a JSON object or null; free-text payloads are not supported.`,
+      );
+    }
+    if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
+      throw new Error(
+        `mergeApprovalAttachments: payloadJson must be a JSON object (got ${
+          Array.isArray(parsed) ? "array" : parsed === null ? "null" : typeof parsed
+        }).`,
+      );
+    }
+    base = parsed as Record<string, unknown>;
+  }
+
+  if (hasDocs) base.documentIds = documentIds;
+  if (hasProducts) base.workProductIds = workProductIds;
+  return JSON.stringify(base);
+}
+
 export const approvalDecisionSchema = z.object({
   companyId: z.string().uuid(),
   approvalId: z.string().uuid(),