fix: restore server XLS roster conversion

2026-08-31 16:04:26 +08:00
parent 4ba2d43710
commit d7a821da16
8 changed files with 218 additions and 63 deletions
--- a/.project-docs/30-worklog/tasks/20260831-fix-xls-conversion-83b17d6a.md
+++ b/.project-docs/30-worklog/tasks/20260831-fix-xls-conversion-83b17d6a.md
@@ -0,0 +1,62 @@
+# Task: Fix server XLS roster conversion
+
+## Identity
+
+- Task ID: 20260831-fix-xls-conversion-83b17d6a
+- Mode: Feature
+- Branch: main
+- Worktree: /Users/inmanx/Documents/lwltAPI
+- Base commit: 4ba2d437102f5f2bbe0381c0f1069686f3c04efb
+- Owner: codex
+- Status: Ready for integration
+
+## Scope
+
+- Inspect the user-supplied production task event and server-log excerpt for `roster_workbook_conversion_failed` after an internal AgentBus `.xls` download.
+- Trace the legacy XLS normalization path through the container image, document converter, workbook validator, task event, and attachment idempotency logic.
+- Restore production `.xls` conversion support, expose a privacy-safe conversion subcode, and allow a new message to revalidate previously rejected bytes.
+- Add regression coverage and run the full repository gates.
+
+## Intent And Constraints
+
+- Continue accepting one genuine legacy `.xls` or `.xlsx` roster while preserving format/MIME/magic checks, macro rejection, bounded isolated conversion, XLSX archive inspection, deterministic template validation, and encrypted normalized storage.
+- Never log or persist workbook bytes, plaintext file names, cell values, converter paths, or raw converter output.
+- A replay of the same AgentBus message must remain idempotent; a later newly delivered message may revalidate a rejected digest because runtime or converter fixes can change the result.
+- Do not deploy, restart services, mutate Kubernetes, access ERP, read secrets, retry the live task, or send an external message.
+
+## Outcome
+
+- Confirmed that attachment correlation and internal HTTPS download now work. The supplied event kept the original task in `awaiting_attachment`, recorded 31,744 downloaded bytes, and failed only at `roster_workbook_conversion_failed`; no new business task or ERP execution was started.
+- The workbook reached legacy conversion only after passing the `.xls` extension, accepted MIME, and OLE compound-file magic checks. The accompanying 99-line log excerpt contains only healthy HTTP polling after the event and no converter stderr, so it does not establish a malformed workbook.
+- Identified the production image defect: the converter requests the Calc-only `Calc MS Excel 2007 XML` export filter, while the runtime image installed only `libreoffice-writer`. Debian packages Calc as the separate spreadsheet component and Writer does not depend on it.
+- Added `libreoffice-calc` to the runtime image alongside Writer and added a repository gate requiring both components.
+- Added a typed, bounded conversion error-code contract and carries its safe value into the roster rejection event as `conversion_error_code`; expected values distinguish unavailable converter, timeout, process failure, missing/invalid output, and output-size failure without exposing paths or content.
+- Preserved an explicit workbook conversion error through the normalizer instead of collapsing it to a generic code.
+- Changed rejected-attachment deduplication so a later message revalidates the same SHA-256 and can upgrade the existing rejected record to normalized after a server fix. The same AgentBus idempotency key still short-circuits as a duplicate, and already normalized bytes remain deduplicated.
+- No deployment, restart, Kubernetes mutation, ERP access, secret read, live-task retry, or external message occurred.
+
+## Verification
+
+- Focused converter, workbook, intake, and retry tests: 20/20 passed.
+- `node --run check:repo`: 10/10 passed, including the new runtime Calc/Writer dependency gate.
+- `node --run check`: passed.
+- `node --run test:control-plane`: 137/137 passed.
+- `node --run test:legacy`: 256/256 passed.
+- `node --run build`: passed.
+- `check_project_docs.py`: passed.
+- `git diff --check`: passed.
+- Docker and Podman CLIs are unavailable on this workstation, so an actual image build was not performed locally; runtime package installation remains to be exercised by the authorized deployment pipeline.
+
+## Follow-ups
+
+- Build and deploy the new image only under separate authorization, set `DEPLOYMENT_REVISION`, and confirm the image contains `libreoffice-calc`.
+- After deployment, resend the same workbook in a new attachment message. The expected path is conversion success, a normalized row count, and transition from `awaiting_attachment` to `parse_queued`; no file modification is required to change its digest.
+
+## Promotion Candidates
+
+- Target canonical documents: `.project-docs/30-worklog/current-state.md` and `.project-docs/50-evidence/evidence-index.md`.
+- Proposal: record that the live environment progressed through the internal download fix to a runtime spreadsheet-converter packaging failure, and that the repository now requires both Writer and Calc plus safe conversion subcodes and rejected-byte revalidation.
+- Evidence: the user-supplied 2026-08-31 task event, current Dockerfile/package boundary, official Debian package descriptions, and this task's focused/full verification.
+- Future impact: deployment checks should treat the Calc component and a non-`unknown` deployment revision as required evidence before live `.xls` verification.
+- Semantic conflicts: the integrated snapshot still says the internal attachment fix is not active; the supplied runtime behavior proves the download boundary advanced, but `deployment_revision` remains `unknown`, so the exact deployed commit cannot be identified.
+- Human confirmation required: no for recording the bounded runtime observation; deployment itself remains separately authorized.
--- a/2
+++ b/2
@@ -13,7 +13,7 @@ FROM node:22-bookworm-slim AS runtime
 ENV NODE_ENV=production
 WORKDIR /app
 RUN apt-get update \
-  && apt-get install --no-install-recommends -y libreoffice-writer fonts-noto-cjk \
+  && apt-get install --no-install-recommends -y libreoffice-calc libreoffice-writer fonts-noto-cjk \
  && rm -rf /var/lib/apt/lists/*
 COPY package*.json ./
 RUN npm ci --omit=dev --no-audit --no-fund
--- a/control-plane/src/document-converter.ts
+++ b/control-plane/src/document-converter.ts
@@ -21,6 +21,13 @@ const HTML_WIDTH_ATTRIBUTE_PATTERN = /(\bwidth\s*=\s*["']?)(\d+(?:\.\d+)?)(["']?
 const HTML_WIDTH_STYLE_PATTERN = /(\bwidth\s*:\s*)(\d+(?:\.\d+)?)(pt|px)/gi;

 export type DocumentConversionStatus = 'converted' | 'already_pdf' | 'converted_xlsx' | 'already_xlsx' | 'source_fallback';
+export type DocumentConversionErrorCode =
+  | 'converter_unavailable'
+  | 'converter_timeout'
+  | 'conversion_failed'
+  | 'conversion_output_missing'
+  | 'conversion_output_invalid'
+  | 'converted_file_too_large';

 export interface DocumentConversionInput {
  fileName: string;
@@ -37,7 +44,7 @@ export interface DocumentConversionResult {
  sha256: string;
  converted: boolean;
  conversion_status: DocumentConversionStatus;
-  conversion_error_code?: string;
+  conversion_error_code?: DocumentConversionErrorCode;
 }

 function sourceExtension(fileName: string, contentType: string): string {
@@ -79,7 +86,7 @@ function isXlsx(contentType: string, content: Buffer): boolean {

 function sourceResult(
  input: DocumentConversionInput,
-  errorCode?: string
+  errorCode?: DocumentConversionErrorCode
 ): DocumentConversionResult {
  return {
    fileName: normalizeTaskArtifactFileName(input.fileName) || 'team-file.bin',
@@ -93,7 +100,7 @@ function sourceResult(
  };
 }

-function conversionErrorCode(error: unknown): string {
+function conversionErrorCode(error: unknown): DocumentConversionErrorCode {
  const value = error && typeof error === 'object' ? error as {
    code?: unknown;
    killed?: unknown;
--- a/control-plane/src/passenger-roster-workbook.ts
+++ b/control-plane/src/passenger-roster-workbook.ts
@@ -1,6 +1,7 @@
 import { extname } from 'node:path';
 import ExcelJS from 'exceljs';
 import { fromBufferPromise, type Entry, type ZipFile } from 'yauzl';
+import type { DocumentConversionErrorCode } from './document-converter.js';

 const XLSX_MAGIC = Buffer.from([0x50, 0x4b, 0x03, 0x04]);
 const XLS_MAGIC = Buffer.from([0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1]);
@@ -89,13 +90,18 @@ export class PassengerRosterWorkbookError extends Error {
  readonly code: PassengerRosterWorkbookErrorCode;
  readonly row?: number;
  readonly column?: number;
+  readonly conversionErrorCode?: DocumentConversionErrorCode;

-  constructor(code: PassengerRosterWorkbookErrorCode, location: { row?: number; column?: number } = {}) {
+  constructor(
+    code: PassengerRosterWorkbookErrorCode,
+    location: { row?: number; column?: number; conversionErrorCode?: DocumentConversionErrorCode } = {}
+  ) {
    super(ERROR_MESSAGES[code]);
    this.name = 'PassengerRosterWorkbookError';
    this.code = code;
    this.row = location.row;
    this.column = location.column;
+    this.conversionErrorCode = location.conversionErrorCode;
  }
 }

@@ -132,7 +138,7 @@ type ScalarCellValue = string | number | boolean | Date | null;

 function fail(
  code: PassengerRosterWorkbookErrorCode,
-  location: { row?: number; column?: number } = {}
+  location: { row?: number; column?: number; conversionErrorCode?: DocumentConversionErrorCode } = {}
 ): never {
  throw new PassengerRosterWorkbookError(code, location);
 }
@@ -553,7 +559,8 @@ export async function normalizePassengerRosterWorkbook(
    if (!options.convertLegacyXls) fail('roster_workbook_converter_unavailable');
    try {
      xlsxContent = await options.convertLegacyXls(input);
-    } catch {
+    } catch (error) {
+      if (error instanceof PassengerRosterWorkbookError) throw error;
      fail('roster_workbook_conversion_failed');
    }
    if (
--- a/control-plane/src/task-service.ts
+++ b/control-plane/src/task-service.ts
@@ -2774,7 +2774,9 @@ export class TaskService {
          maxBytes: this.config.ARTIFACT_MAX_BYTES
        });
        if (!converted.converted || converted.conversion_status !== 'converted_xlsx') {
-          throw new Error(converted.conversion_error_code || 'legacy_xls_conversion_failed');
+          throw new PassengerRosterWorkbookError('roster_workbook_conversion_failed', {
+            conversionErrorCode: converted.conversion_error_code || 'conversion_failed'
+          });
        }
        return converted.content;
      }
@@ -2800,13 +2802,18 @@ export class TaskService {
        WHERE task_id = $1 AND purpose = 'passenger_list' AND sha256 = $2`,
      [target.rowId, digest]
    );
-    if (!prior.rowCount && target.status !== 'awaiting_attachment') {
+    // A rejection is not a permanent verdict on the same bytes: converter or
+    // runtime fixes can make a previously rejected workbook valid. A replay of
+    // the same AgentBus message is still stopped by its idempotency key below.
+    const shouldValidateAttachment = !prior.rowCount
+      || text((prior.rows[0] as Record<string, unknown> | undefined)?.status) === 'rejected';
+    if (shouldValidateAttachment && target.status !== 'awaiting_attachment') {
      throw new TaskError('invalid_transition', `任务当前状态为 ${target.status}，不能接收名单附件。`);
    }

    let normalized: NormalizedPassengerRosterWorkbook | null = null;
    let rejection: { code: string; details: Record<string, unknown> } | null = null;
-    if (!prior.rowCount) {
+    if (shouldValidateAttachment) {
      try {
        normalized = await this.normalizePassengerRosterAttachment(attachment);
      } catch (error) {
@@ -2815,7 +2822,10 @@ export class TaskService {
            code: error.code,
            details: {
              ...(error.row == null ? {} : { row: error.row }),
-              ...(error.column == null ? {} : { column: error.column })
+              ...(error.column == null ? {} : { column: error.column }),
+              ...(error.conversionErrorCode == null
+                ? {}
+                : { conversion_error_code: error.conversionErrorCode })
            }
          };
        } else {
@@ -2877,14 +2887,16 @@ export class TaskService {
      }

      const existingAttachment = await client.query(
-        `SELECT status, row_count, error_code
+        `SELECT id, status, row_count, error_code
           FROM task_input_attachments
          WHERE task_id = $1 AND purpose = 'passenger_list' AND sha256 = $2
          FOR UPDATE`,
        [target.rowId, digest]
      );
-      if (existingAttachment.rowCount) {
-        const previous = existingAttachment.rows[0] as Record<string, unknown>;
+      const previousAttachment = existingAttachment.rowCount
+        ? existingAttachment.rows[0] as Record<string, unknown>
+        : null;
+      if (previousAttachment && text(previousAttachment.status) !== 'rejected') {
        if (scopedIdempotencyKey) {
          await client.query(
            `INSERT INTO idempotency_keys (organization_id, scope, idempotency_key, request_hash, task_id)
@@ -2903,17 +2915,13 @@ export class TaskService {
        return {
          row: taskRow,
          event: null,
-          inputAttachment: text(previous.status) === 'rejected'
-            ? {
-                status: 'rejected' as const,
-                error_code: text(previous.error_code) || 'roster_workbook_rejected',
-                message: `该名单附件此前已校验失败【${text(previous.error_code) || 'roster_workbook_rejected'}】，请修正后重新发送。`
-              }
-            : {
-                status: 'duplicate' as const,
-                ...(Number(previous.row_count) > 0 ? { row_count: Number(previous.row_count) } : {}),
-                message: '该名单附件已经接收，无需重复发送。'
-              }
+          inputAttachment: {
+            status: 'duplicate' as const,
+            ...(Number(previousAttachment.row_count) > 0
+              ? { row_count: Number(previousAttachment.row_count) }
+              : {}),
+            message: '该名单附件已经接收，无需重复发送。'
+          }
        };
      }
      if (text(taskRow.status) !== 'awaiting_attachment') {
@@ -2930,26 +2938,48 @@ export class TaskService {
      let inputAttachment: NonNullable<TaskMessageResult['input_attachment']>;
      let event: TaskEvent;
      if (normalized) {
-        await client.query(
-          `INSERT INTO task_input_attachments
-            (organization_id, task_id, purpose, source, file_name_ciphertext,
-             content_type, byte_size, sha256, status, normalizer_version,
-             normalized_text_ciphertext, row_count)
-           VALUES ($1, $2, 'passenger_list', $3, $4, $5, $6, $7,
-                   'normalized', $8, $9, $10)`,
-          [
-            context.organizationId,
-            target.rowId,
-            attachment.source,
-            encryptedFileName,
-            attachment.contentType,
-            attachment.content.byteLength,
-            digest,
-            PASSENGER_ROSTER_WORKBOOK_VERSION,
-            encryptText(this.config, normalized.canonicalTsv),
-            normalized.rowCount
-          ]
-        );
+        const encryptedCanonicalTsv = encryptText(this.config, normalized.canonicalTsv);
+        if (previousAttachment) {
+          await client.query(
+            `UPDATE task_input_attachments
+                SET source = $2, file_name_ciphertext = $3, content_type = $4,
+                    byte_size = $5, status = 'normalized', normalizer_version = $6,
+                    normalized_text_ciphertext = $7, row_count = $8,
+                    error_code = NULL, error_details = '{}'::jsonb, updated_at = now()
+              WHERE id = $1 AND status = 'rejected'`,
+            [
+              previousAttachment.id,
+              attachment.source,
+              encryptedFileName,
+              attachment.contentType,
+              attachment.content.byteLength,
+              PASSENGER_ROSTER_WORKBOOK_VERSION,
+              encryptedCanonicalTsv,
+              normalized.rowCount
+            ]
+          );
+        } else {
+          await client.query(
+            `INSERT INTO task_input_attachments
+              (organization_id, task_id, purpose, source, file_name_ciphertext,
+               content_type, byte_size, sha256, status, normalizer_version,
+               normalized_text_ciphertext, row_count)
+             VALUES ($1, $2, 'passenger_list', $3, $4, $5, $6, $7,
+                     'normalized', $8, $9, $10)`,
+            [
+              context.organizationId,
+              target.rowId,
+              attachment.source,
+              encryptedFileName,
+              attachment.contentType,
+              attachment.content.byteLength,
+              digest,
+              PASSENGER_ROSTER_WORKBOOK_VERSION,
+              encryptedCanonicalTsv,
+              normalized.rowCount
+            ]
+          );
+        }
        const updated = await client.query(
          `UPDATE tasks
              SET status = 'parse_queued', stage = 'parse',
@@ -2984,24 +3014,44 @@ export class TaskService {
        });
      } else {
        const safeRejection = rejection || { code: 'roster_workbook_processing_failed', details: {} };
-        await client.query(
-          `INSERT INTO task_input_attachments
-            (organization_id, task_id, purpose, source, file_name_ciphertext,
-             content_type, byte_size, sha256, status, error_code, error_details)
-           VALUES ($1, $2, 'passenger_list', $3, $4, $5, $6, $7,
-                   'rejected', $8, $9::jsonb)`,
-          [
-            context.organizationId,
-            target.rowId,
-            attachment.source,
-            encryptedFileName,
-            attachment.contentType,
-            attachment.content.byteLength,
-            digest,
-            safeRejection.code,
-            safeRejection.details
-          ]
-        );
+        if (previousAttachment) {
+          await client.query(
+            `UPDATE task_input_attachments
+                SET source = $2, file_name_ciphertext = $3, content_type = $4,
+                    byte_size = $5, status = 'rejected', normalizer_version = NULL,
+                    normalized_text_ciphertext = NULL, row_count = NULL,
+                    error_code = $6, error_details = $7::jsonb, updated_at = now()
+              WHERE id = $1 AND status = 'rejected'`,
+            [
+              previousAttachment.id,
+              attachment.source,
+              encryptedFileName,
+              attachment.contentType,
+              attachment.content.byteLength,
+              safeRejection.code,
+              safeRejection.details
+            ]
+          );
+        } else {
+          await client.query(
+            `INSERT INTO task_input_attachments
+              (organization_id, task_id, purpose, source, file_name_ciphertext,
+               content_type, byte_size, sha256, status, error_code, error_details)
+             VALUES ($1, $2, 'passenger_list', $3, $4, $5, $6, $7,
+                     'rejected', $8, $9::jsonb)`,
+            [
+              context.organizationId,
+              target.rowId,
+              attachment.source,
+              encryptedFileName,
+              attachment.contentType,
+              attachment.content.byteLength,
+              digest,
+              safeRejection.code,
+              safeRejection.details
+            ]
+          );
+        }
        const updated = await client.query(
          `UPDATE tasks
              SET message = $2, error = '', updated_at = now()
--- a/control-plane/test/passenger-roster-intake.test.ts
+++ b/control-plane/test/passenger-roster-intake.test.ts
@@ -48,6 +48,14 @@ test('roster attachment source contract gates claims and scrubs normalized plain
  assert.doesNotMatch(source, /raw_(?:file|workbook)_ciphertext|content_base64[^\n]*task_input_attachments/);
 });

+test('a newly delivered message revalidates previously rejected attachment bytes', async () => {
+  const source = await readFile(new URL('../src/task-service.ts', import.meta.url), 'utf8');
+  assert.match(source, /const shouldValidateAttachment = !prior\.rowCount[\s\S]*?=== 'rejected'/u);
+  assert.match(source, /previousAttachment && text\(previousAttachment\.status\) !== 'rejected'/u);
+  assert.match(source, /SET source = \$2[\s\S]*?status = 'normalized'[\s\S]*?WHERE id = \$1 AND status = 'rejected'/u);
+  assert.match(source, /i\.idempotency_key = \$2[\s\S]*?status: 'duplicate' as const/u);
+});
+
 test('manual API accepts exactly one bounded inline roster attachment', async () => {
  const source = await readFile(new URL('../src/server.ts', import.meta.url), 'utf8');
  assert.match(source, /attachments: z\.array\(encodedInputAttachmentSchema\)\.max\(1\)/);
--- a/control-plane/test/passenger-roster-workbook.test.ts
+++ b/control-plane/test/passenger-roster-workbook.test.ts
@@ -339,6 +339,21 @@ test('format, MIME, conversion output and technical limits are validated before
    }, { convertLegacyXls: async () => Buffer.from('not xlsx') }),
    'roster_workbook_conversion_failed'
  );
+  const conversionFailure = await expectRosterError(
+    () => normalizePassengerRosterWorkbook({
+      content: legacy,
+      fileName: 'synthetic.xls',
+      contentType: 'application/vnd.ms-excel'
+    }, {
+      convertLegacyXls: async () => {
+        throw new PassengerRosterWorkbookError('roster_workbook_conversion_failed', {
+          conversionErrorCode: 'conversion_output_missing'
+        });
+      }
+    }),
+    'roster_workbook_conversion_failed'
+  );
+  assert.equal(conversionFailure.conversionErrorCode, 'conversion_output_missing');

  const macroLegacy = Buffer.concat([legacy, Buffer.from('_VBA_PROJECT_CUR', 'utf16le')]);
  await expectRosterError(
--- a/tools/repository-hygiene.test.mjs
+++ b/tools/repository-hygiene.test.mjs
@@ -323,3 +323,9 @@ test('single-source and generated-output boundaries remain explicit', async () =
  assert.ok(!existsSync(path.join(DIST, 'control-plane')));
  assert.ok(!existsSync(path.join(DIST, 'ltjt-order-assistant.zip')));
 });
+
+test('runtime image includes both LibreOffice document converters', async () => {
+  const dockerfile = await readFile(path.join(ROOT, 'Dockerfile'), 'utf8');
+  assert.match(dockerfile, /apt-get install[^\n]*libreoffice-calc/u);
+  assert.match(dockerfile, /apt-get install[^\n]*libreoffice-writer/u);
+});