From d7a821da1627548186e8834350e09a66c44140d7 Mon Sep 17 00:00:00 2001 From: inman Date: Mon, 31 Aug 2026 16:04:26 +0800 Subject: [PATCH] fix: restore server XLS roster conversion --- .../20260831-fix-xls-conversion-83b17d6a.md | 62 +++++++ Dockerfile | 2 +- control-plane/src/document-converter.ts | 13 +- .../src/passenger-roster-workbook.ts | 13 +- control-plane/src/task-service.ts | 162 ++++++++++++------ .../test/passenger-roster-intake.test.ts | 8 + .../test/passenger-roster-workbook.test.ts | 15 ++ tools/repository-hygiene.test.mjs | 6 + 8 files changed, 218 insertions(+), 63 deletions(-) create mode 100644 .project-docs/30-worklog/tasks/20260831-fix-xls-conversion-83b17d6a.md diff --git a/.project-docs/30-worklog/tasks/20260831-fix-xls-conversion-83b17d6a.md b/.project-docs/30-worklog/tasks/20260831-fix-xls-conversion-83b17d6a.md new file mode 100644 index 0000000..5c6206e --- /dev/null +++ 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. diff --git a/Dockerfile b/Dockerfile index f489f23..8f2f647 100644 --- a/Dockerfile +++ b/Dockerfile @@ -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 diff --git a/control-plane/src/document-converter.ts b/control-plane/src/document-converter.ts index 34935e1..3d88d14 100644 --- 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; diff --git a/control-plane/src/passenger-roster-workbook.ts b/control-plane/src/passenger-roster-workbook.ts index 775be6c..d3e87b2 100644 --- 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 ( diff --git a/control-plane/src/task-service.ts b/control-plane/src/task-service.ts index 14a290f..7aad073 100644 --- 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 | 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 } | 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; + const previousAttachment = existingAttachment.rowCount + ? existingAttachment.rows[0] as Record + : 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; 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() diff --git a/control-plane/test/passenger-roster-intake.test.ts b/control-plane/test/passenger-roster-intake.test.ts index f49abfd..fd36fc6 100644 --- 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\)/); diff --git a/control-plane/test/passenger-roster-workbook.test.ts b/control-plane/test/passenger-roster-workbook.test.ts index 2da5a03..03be723 100644 --- 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( diff --git a/tools/repository-hygiene.test.mjs b/tools/repository-hygiene.test.mjs index 6c4a8bd..ba2ca19 100644 --- 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); +});