mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix: ignore Read pages for non-PDF files (#928)
Treat pages as PDF-only during validation and execution so model-supplied pages on images or text files do not loop on invalid parameter errors. Tested: - bun test src/tools/FileReadTool/FileReadTool.test.ts - bun run check:server Confidence: high Scope-risk: narrow
This commit is contained in:
parent
e64cd80662
commit
c15c465b0b
67
src/tools/FileReadTool/FileReadTool.test.ts
Normal file
67
src/tools/FileReadTool/FileReadTool.test.ts
Normal file
@ -0,0 +1,67 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
import { PDF_MAX_PAGES_PER_READ } from '../../constants/apiLimits.js'
|
||||
import type { ToolUseContext } from '../../Tool.js'
|
||||
import { getEmptyToolPermissionContext } from '../../Tool.js'
|
||||
import { FileReadTool } from './FileReadTool.js'
|
||||
|
||||
function makeToolUseContext(): ToolUseContext {
|
||||
return {
|
||||
getAppState: () => ({
|
||||
toolPermissionContext: getEmptyToolPermissionContext(),
|
||||
}),
|
||||
} as unknown as ToolUseContext
|
||||
}
|
||||
|
||||
describe('FileReadTool pages validation', () => {
|
||||
test('ignores invalid PDF pages values for non-PDF files', async () => {
|
||||
const context = makeToolUseContext()
|
||||
|
||||
await expect(
|
||||
FileReadTool.validateInput(
|
||||
{ file_path: '/tmp/screenshot.png', pages: '0' },
|
||||
context,
|
||||
),
|
||||
).resolves.toEqual({ result: true })
|
||||
|
||||
await expect(
|
||||
FileReadTool.validateInput(
|
||||
{ file_path: '/tmp/example.ts', pages: '' },
|
||||
context,
|
||||
),
|
||||
).resolves.toEqual({ result: true })
|
||||
|
||||
await expect(
|
||||
FileReadTool.validateInput(
|
||||
{ file_path: 'C:\\tmp\\SCREENSHOT.PNG', pages: '0' },
|
||||
context,
|
||||
),
|
||||
).resolves.toEqual({ result: true })
|
||||
})
|
||||
|
||||
test('keeps PDF pages validation strict', async () => {
|
||||
const context = makeToolUseContext()
|
||||
|
||||
await expect(
|
||||
FileReadTool.validateInput(
|
||||
{ file_path: '/tmp/document.pdf', pages: '0' },
|
||||
context,
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
result: false,
|
||||
errorCode: 7,
|
||||
})
|
||||
|
||||
await expect(
|
||||
FileReadTool.validateInput(
|
||||
{
|
||||
file_path: '/tmp/document.pdf',
|
||||
pages: `1-${PDF_MAX_PAGES_PER_READ + 1}`,
|
||||
},
|
||||
context,
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
result: false,
|
||||
errorCode: 8,
|
||||
})
|
||||
})
|
||||
})
|
||||
@ -417,8 +417,15 @@ export const FileReadTool = buildTool({
|
||||
},
|
||||
renderToolUseErrorMessage,
|
||||
async validateInput({ file_path, pages }, toolUseContext: ToolUseContext) {
|
||||
// Validate pages parameter (pure string parsing, no I/O)
|
||||
if (pages !== undefined) {
|
||||
// Path expansion + extension checks are string-only and avoid I/O before
|
||||
// permission evaluation.
|
||||
const fullFilePath = expandPath(file_path)
|
||||
const ext = path.extname(fullFilePath).toLowerCase()
|
||||
|
||||
// Validate pages parameter only for PDF files. Models sometimes send
|
||||
// PDF-only pages values when reading images or text files; those should
|
||||
// not block the read path.
|
||||
if (pages !== undefined && isPDFExtension(ext)) {
|
||||
const parsed = parsePDFPageRange(pages)
|
||||
if (!parsed) {
|
||||
return {
|
||||
@ -440,9 +447,6 @@ export const FileReadTool = buildTool({
|
||||
}
|
||||
}
|
||||
|
||||
// Path expansion + deny rule check (no I/O)
|
||||
const fullFilePath = expandPath(file_path)
|
||||
|
||||
const appState = toolUseContext.getAppState()
|
||||
const denyRule = matchingRuleForInput(
|
||||
fullFilePath,
|
||||
@ -469,7 +473,6 @@ export const FileReadTool = buildTool({
|
||||
|
||||
// Binary extension check (string check on extension only, no I/O).
|
||||
// PDF, images, and SVG are excluded - this tool renders them natively.
|
||||
const ext = path.extname(fullFilePath).toLowerCase()
|
||||
if (
|
||||
hasBinaryExtension(fullFilePath) &&
|
||||
!isPDFExtension(ext) &&
|
||||
@ -517,6 +520,7 @@ export const FileReadTool = buildTool({
|
||||
}
|
||||
|
||||
const ext = path.extname(file_path).toLowerCase().slice(1)
|
||||
const effectivePages = isPDFExtension(ext) ? pages : undefined
|
||||
// Use expandPath for consistent path normalization with FileEditTool/FileWriteTool
|
||||
// (especially handles whitespace trimming and Windows path separators)
|
||||
const fullFilePath = expandPath(file_path)
|
||||
@ -599,7 +603,7 @@ export const FileReadTool = buildTool({
|
||||
ext,
|
||||
offset,
|
||||
limit,
|
||||
pages,
|
||||
effectivePages,
|
||||
maxSizeBytes,
|
||||
maxTokens,
|
||||
readFileState,
|
||||
@ -622,7 +626,7 @@ export const FileReadTool = buildTool({
|
||||
ext,
|
||||
offset,
|
||||
limit,
|
||||
pages,
|
||||
effectivePages,
|
||||
maxSizeBytes,
|
||||
maxTokens,
|
||||
readFileState,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user