Repository navigation
fix(testmanagement): send mimeType and mcp source for test case generation, limit file types - #569
Conversation
… file types createTestCasesFromFile sent documents for generation without a mimeType, so every file was treated as a PDF and other types failed to parse. uploadProductRequirementFile also accepted file types that test case generation cannot read (.md, .doc/.docx, .csv, .xls/.xlsx, .json, .html, .gif, .zip). Limit the upload to .pdf, .txt, .png, .jpg and .jpeg, store each file's mimeType with its document id, and send it on the generation request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sarve-shreyas
left a comment
sarve-shreyas
left a comment
There was a problem hiding this comment.
Reviewed the full diff main...2e4e3033. npm run build passes and the full vitest run passes locally (exit 0). The four touched test files pass 42/42.
The core fix is correct. I traced the request end to end: TM's tcg#suggest_test_cases (v1, build_suggest_test_cases_params) forwards the body as is. bstack_parser routes/testGeneration.js destructures mimeType = "application/pdf", and testGenerationProcessor hands it to processDocumentV2. Without mimeType every file was parsed as a PDF, so sending it fixes .txt. The fallback for older map entries with no mimeType stays safe: the key is dropped from the JSON and the backend defaults to PDF.
Main point (inline on upload-validator.ts): the allow-list is narrower than what generation actually reads. processDocumentV2 also converts DOCX/DOC, XLSX/XLS, PPTX, CSV and RTF (Document_MIME_Types in document_test_generation/constants.js, plus the documentMimeValidator). Those uploads failed before only because no mimeType was sent, which is exactly what this PR fixes. So the PR description's claim that generation can't read .doc/.docx/.csv/.xls/.xlsx doesn't match the backend. I'd map them instead of rejecting them.
Smaller items:
TEST_MANAGEMENT_ATTACHMENT_EXTENSIONSis now unused insrc/(onlyupload-validator.test.tsimports it). Either drop it or leave a comment saying who it's for.- The
uploadProductRequirementFiletool description (testmanagement.ts, outside the diff) still says "e.g., PDRs, PDFs". It's worth listing the accepted types there too, because that's what the model reads when picking a file.
| ".txt": "text/plain", | ||
| ".png": "image/png", | ||
| ".jpg": "image/jpeg", | ||
| ".jpeg": "image/jpeg", |
There was a problem hiding this comment.
Should fix: this list is narrower than what generation reads.
The v1 path (processDocumentV2 in bstack_parser/test_selection_workers/services/document_test_generation/index.js) handles DOCX/DOC (converted to PDF), XLSX/XLS, PPTX, CSV and RTF, not just PDF/TXT/PNG/JPEG. See Document_MIME_Types in document_test_generation/constants.js. Those types failed before only because no mimeType was sent, so everything was parsed as a PDF, and this PR fixes that.
Scenario: a user uploads requirements.docx, which is probably the most common PRD format. Before this PR the upload succeeded and generation failed. After it, the upload is rejected outright, although generation would now work if the mime type were sent.
Suggestion: add the extra mappings using the exact strings the backend compares with ===:
".docx": "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
".doc": "application/msword",
".xlsx": "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
".xls": "application/vnd.ms-excel",
".pptx": "application/vnd.openxmlformats-officedocument.presentationml.presentation",
".csv": "text/csv",
".rtf": "application/rtf",Then keep rejecting .md, .json, .html, .gif and .zip. If some of these were deliberately left out (for example a known conversion problem), please say so in the PR description.
| .body.find((p: { type: string }) => p.type === "file"); | ||
| expect(file.accept).toEqual( | ||
| expect.arrayContaining([...TEST_MANAGEMENT_ATTACHMENT_EXTENSIONS]), | ||
| expect.arrayContaining(Object.keys(TEST_CASE_GENERATION_MIME_TYPES)), |
There was a problem hiding this comment.
Non-blocking: this weakens the guard. Before, the test asserted that upload_generic_attachments accepts every generic TM attachment type. Now it only checks the five generation types, so the index could drop e.g. .zip or .xlsx and nothing would catch it. If the intent is still "the capability accepts at least what uploadFile accepts", that's fine. Consider keeping a second assertion against TEST_MANAGEMENT_ATTACHMENT_EXTENSIONS, since the capability is a generic attachment upload, not a TCG-only one.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sarve-shreyas
left a comment
sarve-shreyas
left a comment
There was a problem hiding this comment.
Re-reviewed 2e4e3033..f4bc7e69 (one commit: send mcp as the request source). npm run build passes, and the four touched test files pass 43/43.
The code change itself is clean. source now reaches the trigger body, the request-source header on trigger and poll, and the new bulk-create header. The new test covers both. The bulk-create header has no side effects on TM: test_cases_helper only creates a polling key for jira-on-prem, and MCP doesn't poll after bulk create.
Still open from my last round (no code change or reply yet):
- The allow-list is narrower than what generation reads: DOCX/DOC/XLSX/XLS/PPTX/CSV/RTF are rejected at upload even though
processDocumentV2handles them oncemimeTypeis sent. See the inline comment onupload-validator.ts. - The
capabilityRegistryFileUploadguard is weaker. TEST_MANAGEMENT_ATTACHMENT_EXTENSIONSis unused insrc/, and theuploadProductRequirementFiletool description still says "e.g., PDRs, PDFs".
New: the mcp source needs the server side deployed first (inline on testcase-from-file.ts). Please also add the source change to the PR description. It currently only covers the mimeType work.
| const mimeType = (documentObj as { mimeType?: string }).mimeType; | ||
|
|
||
| const source = "jira-on-prem"; | ||
| const source = "mcp"; |
There was a problem hiding this comment.
Blocking (release order): mcp only works once the server side is live in every prod region.
TM support is browserstack/teststack#17380 (PTMAA-1026), which merged into main today. Its own rollout order lists this change as step 3, after ai-tm#411 (the TCM_MCP product whitelist) and ai-tm-infra-ops#219 (the prod GRR bypass).
Scenario if this ships first:
- TM without #17380:
suggest_test_casesonly creates the polling key for[jira-on-prem, azure]. The TCG webhook stores results only if that key exists (test_cases_helper.rb,ai_request_exists?), so results are dropped.createTestCasesFromFilethen polls untilTCG_POLL_MAX_WAIT_MS, 10 minutes, and returns nothing. - TM with #17380 but ai-tm without feat(security): require MCP_UPLOAD_BASE_DIR for the upload-PRD tool #411: TM sends
x-bstack-product: TCM_MCP, and per #17380 ai-tm rejects unknown products. So generation fails at trigger.
I can't see the ai-tm repos with my access, so I couldn't check #411 or #219 myself. Can you confirm all three are deployed to prod (all regions) before this is merged or released? Also, bstack_parser on misc-services main doesn't list TCM_MCP in documentTestCaseGenerationSupportedProducts or productGRRBypass. If ai-tm forwards that header to it, that's a fourth dependency.
Suggestion: hold the merge until those are confirmed. The other option is to split this commit into its own PR, so the mimeType fix can ship now without waiting on the rollout.
…eration Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SavioBS629
changed the title
sarve-shreyas
left a comment
sarve-shreyas
left a comment
There was a problem hiding this comment.
Re-reviewed f4bc7e69..53518fce. npm run build passes, and the four touched test files pass 47/47.
Resolved:
- Allow-list:
.docx,.xlsx,.csvand.rtfare added with MIME strings that match the backend'sDocument_MIME_Typesexactly. They're also all in TM'sVALID_AI_CONTENT_TYPE, so the AI upload accepts them. Leaving out.doc(prod returns400 Invalid file type for AI uploads) and the untested.xls/.pptxis a sensible, conservative choice. One side note:application/mswordis in TM'sVALID_AI_CONTENT_TYPE, so the.doc400 is probably content-type sniffing (legacy OLE files are often detected as something else). That's not an issue for this PR. - Capability test: it asserts the full
TEST_MANAGEMENT_ATTACHMENT_EXTENSIONSagain, which also gives that constant a clear owner. - Tool description: it lists the accepted types, and the PR description now covers the
mcpsource change. mcpsource rollout: verified on prod US by the author. It was a.txtrun of 145 test cases, with every suggest, details and bulk call sent asrequest-source: mcp.
Remaining caveat (question, not a code change): the prod check was US only. If any other TM region (EU/APS/USW) isn't yet on teststack#17380 and ai-tm#411, MCP users there would hit the 10-minute empty poll. It's worth a quick confirmation that those deploys are global before the next mcp-server release.
Nit: the file_path schema description became "Full path to the file". The types are in the tool description now, so that's fine.
No further code findings from me.
Summary
createTestCasesFromFilesent the uploaded document for generation without amimeType, so every file was treated as a PDF and anything else failed to parse, including plain.txt.uploadProductRequirementFilealso accepted file types that test case generation can't read (.md,.doc,.xls,.json,.html,.gif,.zip), so those uploads only failed later, at generation. Generation requests also identified themselves asjira-on-prem; they now use sourcemcp.This PR:
TEST_CASE_GENERATION_MIME_TYPES, the file types generation reads mapped to their MIME types:.pdf,.txt,.png,.jpg,.jpeg,.docx,.xlsx,.csv,.rtfuploadProductRequirementFileto those extensions, so unsupported files are rejected at upload with the allowed list in the errormimeTypewith its document id and sends it asmimeTypeon the generation requestmcprequest source instead ofjira-on-prem, on generation, test case details and the bulk saveTEST_MANAGEMENT_ATTACHMENT_EXTENSIONSis unchanged; it still describes generic test management attachments, and the capability test asserts against it. TheuploadProductRequirementFiledescription now lists the accepted types.Tests
tests/tools/uploadFile.test.ts: unsupported types are rejected before any request; each supported type stores the rightmimeType, including an upper-case extensiontests/tools/tcg-trigger.test.ts: the generation request carriesmimeTypeand themcpsource; the bulk save sends themcprequest sourcenpm run buildpasses (Node 22).txtrequirements file generates and saves test cases, with every generation, details and save request sent as sourcemcp;.docx,.xlsx,.csvand.rtffiles also generate and save test cases; a.docupload is rejected with the allowed list🤖 Generated with Claude Code