🩹 fix: CI — preserve legacy download path for empty-text office attachments

Two regressions surfaced after the textFormat security gate landed.

1. **Client** (`LogContent.test.tsx` "falls back to the legacy download
   branch for an office file with no extracted text"):

   When the security gate downgraded an office type without
   `textFormat: 'html'` to PLAIN_TEXT, the lenient empty-text gate on
   PLAIN_TEXT then accepted a missing `text` field and rendered a
   half-empty panel card. The historical contract is "office type +
   no text → legacy download UI"; the downgrade should only fire when
   there's actual plain text that needs safe-escaping.

   Fix in `detectArtifactTypeFromFile`: short-circuit to null when the
   office type lands in the security-gate branch with no text. The
   PLAIN_TEXT downgrade still fires for legacy attachments that DO
   carry plain text.

2. **API** (`process.spec.js` + `process-traversal.spec.js`): the
   `@librechat/api` mocks didn't expose `getExtractedTextFormat`, so
   `processCodeOutput` called `undefined(...)` → TypeError → tests got
   undefined results. Added the helper to both mocks with a faithful
   default (returns 'text' for non-null extractor output, null
   otherwise).

Tests: new regression in `artifacts.test.ts` pinning the empty-text
+ no-textFormat → null contract for all four office types
(.docx/.csv/.xlsx/.pptx), so a future refactor can't silently
re-introduce the half-empty card.
This commit is contained in:
Danny Avila 2026-05-04 17:40:49 -04:00
parent 2dc32c7a42
commit 832b8f7bc7
4 changed files with 48 additions and 1 deletions

View file

@ -27,6 +27,11 @@ jest.mock('@librechat/api', () => {
createAxiosInstance: jest.fn(() => mockAxios),
classifyCodeArtifact: jest.fn(() => 'other'),
extractCodeArtifactText: jest.fn(async () => null),
/* `processCodeOutput` calls this to derive the trust flag persisted
* on `IMongoFile.textFormat` — Codex P1 review on PR #12934. The
* mock returns null in lockstep with the null `text` above so
* downstream consumers don't see a phantom format. */
getExtractedTextFormat: jest.fn(() => null),
codeServerHttpAgent: new http.Agent({ keepAlive: false }),
codeServerHttpsAgent: new https.Agent({ keepAlive: false }),
};

View file

@ -43,6 +43,7 @@ mockAxios.isAxiosError = jest.fn(() => false);
const mockClassifyCodeArtifact = jest.fn(() => 'other');
const mockExtractCodeArtifactText = jest.fn(async () => null);
const mockGetExtractedTextFormat = jest.fn((_name, _mime, text) => (text == null ? null : 'text'));
jest.mock('@librechat/api', () => {
const http = require('http');
const https = require('https');
@ -63,6 +64,15 @@ jest.mock('@librechat/api', () => {
*/
classifyCodeArtifact: (...args) => mockClassifyCodeArtifact(...args),
extractCodeArtifactText: (...args) => mockExtractCodeArtifactText(...args),
/* `processCodeOutput` derives the `textFormat` trust flag for
* `IMongoFile` from this helper — Codex P1 review on PR #12934.
* The mock returns 'text' for non-null extractor output and null
* otherwise so the downstream `file.textFormat` field is set to
* a believable shape without modeling the office-HTML branch
* (the dispatcher under test isn't exercising that path). Per-
* test overrides via `mockGetExtractedTextFormat.mockReturnValue`
* if a case needs to assert the 'html' value. */
getExtractedTextFormat: (...args) => mockGetExtractedTextFormat(...args),
codeServerHttpAgent: new http.Agent({ keepAlive: false }),
codeServerHttpsAgent: new https.Agent({ keepAlive: false }),
};

View file

@ -225,6 +225,27 @@ describe('detectArtifactTypeFromFile', () => {
).toBe(TOOL_ARTIFACT_TYPES.PLAIN_TEXT);
});
/* Regression: an office attachment with no `text` AND no `textFormat`
* must NOT downgrade to PLAIN_TEXT (which has the lenient empty-text
* gate and would render as a half-empty panel card). The historical
* contract for office types with missing extraction is "fall through
* to the legacy download UI"; the security gate's empty-text exception
* preserves that. CI regression on PR #12934 — `LogContent.test.tsx`
* "falls back to the legacy download branch for an office file with
* no extracted text" was the canary. */
it.each([
['report.docx', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'],
['data.csv', 'text/csv'],
['workbook.xlsx', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'],
['slides.pptx', 'application/vnd.openxmlformats-officedocument.presentationml.presentation'],
])(
'%s with no text and no textFormat returns null (preserves legacy download path)',
(filename, type) => {
expect(detectArtifactTypeFromFile({ filename, type, text: '' })).toBeNull();
expect(detectArtifactTypeFromFile({ filename, type, text: undefined })).toBeNull();
},
);
it('falls back to MIME when the extension is unknown', () => {
expect(detectArtifactTypeFromFile({ filename: 'noext', type: 'text/html', text: 'x' })).toBe(
TOOL_ARTIFACT_TYPES.HTML,

View file

@ -719,8 +719,19 @@ export function detectArtifactTypeFromFile(
* pre-flag period and the safe default ('text' bucket) is OK for
* those — they were rendering correctly before this flag, and the
* markdown viewer escaping HTML is a safety upgrade, not a
* regression. Codex P1 review on PR #12934. */
* regression. Codex P1 review on PR #12934.
*
* Empty-text exception: if the office attachment has NO text at all,
* downgrading to PLAIN_TEXT would route an empty-text-tolerant
* bucket onto the panel (a half-rendered card showing only the
* filename). The historical contract for office types with missing
* text is "fall through to the legacy download path"; preserve that
* by returning null here. The downgrade only matters when there's
* actual text content that needs safe-escaping. */
if (OFFICE_HTML_BUCKETS.has(type) && attachment.textFormat !== 'html') {
if (!attachment.text) {
return null;
}
return TOOL_ARTIFACT_TYPES.PLAIN_TEXT;
}
if (