mirror of
https://github.com/KeygraphHQ/shannon.git
synced 2026-02-12 17:22:50 +00:00
fix: add file_path parameter to save_deliverable for large reports (#123)
* fix: add file_path parameter to save_deliverable for large reports Large deliverable reports can exceed output token limits when passed as inline content. This change allows agents to write reports to disk first and pass a file_path instead. Changes: - Add file_path parameter to save_deliverable MCP tool with path traversal protection - Pass CLAUDE_CODE_MAX_OUTPUT_TOKENS env var to SDK subprocesses - Fix false positive error detection by extracting only text content (not tool_use JSON) when checking for API errors - Update all prompts to instruct agents to use file_path for large reports and stop immediately after completion * docs: simplify and condense CLAUDE.md Reduce verbosity while preserving all essential information for AI assistance. Makes the documentation more scannable and focused. * feat: add issue number detection to pr command The /pr command now automatically detects issue numbers from: 1. Explicit arguments (e.g., /pr 123 or /pr 123,456) 2. Branch name patterns (e.g., fix/123-bug, issue-456-feature) Adds "Closes #X" lines to PR body to auto-close issues on merge. * chore: remove CLAUDE_CODE_MAX_OUTPUT_TOKENS env var handling No longer needed with the new Claude Agent SDK version. * fix: restore max_output_tokens error handling
This commit is contained in:
committed by
GitHub
parent
1710bd93f7
commit
ae4c4ed402
@@ -16,6 +16,8 @@
|
||||
|
||||
import { tool } from '@anthropic-ai/claude-agent-sdk';
|
||||
import { z } from 'zod';
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { DeliverableType, DELIVERABLE_FILENAMES, isQueueType } from '../types/deliverables.js';
|
||||
import { createToolResult, type ToolResult, type SaveDeliverableResponse } from '../types/tool-responses.js';
|
||||
import { validateQueueJson } from '../validation/queue-validator.js';
|
||||
@@ -27,13 +29,68 @@ import { createValidationError, createGenericError } from '../utils/error-format
|
||||
*/
|
||||
export const SaveDeliverableInputSchema = z.object({
|
||||
deliverable_type: z.nativeEnum(DeliverableType).describe('Type of deliverable to save'),
|
||||
content: z.string().min(1).describe('File content (markdown for analysis/evidence, JSON for queues)'),
|
||||
content: z.string().min(1).optional().describe('File content (markdown for analysis/evidence, JSON for queues). Optional if file_path is provided.'),
|
||||
file_path: z.string().optional().describe('Path to a file whose contents should be used as the deliverable content. Relative paths are resolved against the deliverables directory. Use this instead of content for large reports to avoid output token limits.'),
|
||||
});
|
||||
|
||||
export type SaveDeliverableInput = z.infer<typeof SaveDeliverableInputSchema>;
|
||||
|
||||
/**
|
||||
* Create save_deliverable handler with targetDir captured in closure
|
||||
* Check if a path is contained within a base directory.
|
||||
* Prevents path traversal attacks (e.g., ../../../etc/passwd).
|
||||
*/
|
||||
function isPathContained(basePath: string, targetPath: string): boolean {
|
||||
const resolvedBase = path.resolve(basePath);
|
||||
const resolvedTarget = path.resolve(targetPath);
|
||||
return resolvedTarget === resolvedBase || resolvedTarget.startsWith(resolvedBase + path.sep);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve deliverable content from either inline content or a file path.
|
||||
* Returns the content string on success, or a ToolResult error on failure.
|
||||
*/
|
||||
function resolveContent(
|
||||
args: SaveDeliverableInput,
|
||||
targetDir: string,
|
||||
): string | ToolResult {
|
||||
if (args.content) {
|
||||
return args.content;
|
||||
}
|
||||
|
||||
if (!args.file_path) {
|
||||
return createToolResult(createValidationError(
|
||||
'Either "content" or "file_path" must be provided',
|
||||
true,
|
||||
{ deliverableType: args.deliverable_type },
|
||||
));
|
||||
}
|
||||
|
||||
const resolvedPath = path.isAbsolute(args.file_path)
|
||||
? args.file_path
|
||||
: path.resolve(targetDir, args.file_path);
|
||||
|
||||
// Security: Prevent path traversal outside targetDir
|
||||
if (!isPathContained(targetDir, resolvedPath)) {
|
||||
return createToolResult(createValidationError(
|
||||
`Path "${args.file_path}" resolves outside allowed directory`,
|
||||
false,
|
||||
{ deliverableType: args.deliverable_type, allowedBase: targetDir },
|
||||
));
|
||||
}
|
||||
|
||||
try {
|
||||
return fs.readFileSync(resolvedPath, 'utf-8');
|
||||
} catch (readError) {
|
||||
return createToolResult(createValidationError(
|
||||
`Failed to read file at ${resolvedPath}: ${readError instanceof Error ? readError.message : String(readError)}`,
|
||||
true,
|
||||
{ deliverableType: args.deliverable_type, filePath: resolvedPath },
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create save_deliverable handler with targetDir captured in closure.
|
||||
*
|
||||
* This factory pattern ensures each MCP server instance has its own targetDir,
|
||||
* preventing race conditions when multiple workflows run in parallel.
|
||||
@@ -41,29 +98,28 @@ export type SaveDeliverableInput = z.infer<typeof SaveDeliverableInputSchema>;
|
||||
function createSaveDeliverableHandler(targetDir: string) {
|
||||
return async function saveDeliverable(args: SaveDeliverableInput): Promise<ToolResult> {
|
||||
try {
|
||||
const { deliverable_type, content } = args;
|
||||
const { deliverable_type } = args;
|
||||
|
||||
const contentOrError = resolveContent(args, targetDir);
|
||||
if (typeof contentOrError !== 'string') {
|
||||
return contentOrError;
|
||||
}
|
||||
const content = contentOrError;
|
||||
|
||||
// Validate queue JSON if applicable
|
||||
if (isQueueType(deliverable_type)) {
|
||||
const queueValidation = validateQueueJson(content);
|
||||
if (!queueValidation.valid) {
|
||||
const errorResponse = createValidationError(
|
||||
return createToolResult(createValidationError(
|
||||
queueValidation.message ?? 'Invalid queue JSON',
|
||||
true,
|
||||
{
|
||||
deliverableType: deliverable_type,
|
||||
expectedFormat: '{"vulnerabilities": [...]}',
|
||||
}
|
||||
);
|
||||
return createToolResult(errorResponse);
|
||||
{ deliverableType: deliverable_type, expectedFormat: '{"vulnerabilities": [...]}' },
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
// Get filename and save file (targetDir captured from closure)
|
||||
const filename = DELIVERABLE_FILENAMES[deliverable_type];
|
||||
const filepath = saveDeliverableFile(targetDir, filename, content);
|
||||
|
||||
// Success response
|
||||
const successResponse: SaveDeliverableResponse = {
|
||||
status: 'success',
|
||||
message: `Deliverable saved successfully: ${filename}`,
|
||||
@@ -74,13 +130,11 @@ function createSaveDeliverableHandler(targetDir: string) {
|
||||
|
||||
return createToolResult(successResponse);
|
||||
} catch (error) {
|
||||
const errorResponse = createGenericError(
|
||||
return createToolResult(createGenericError(
|
||||
error,
|
||||
false,
|
||||
{ deliverableType: args.deliverable_type }
|
||||
);
|
||||
|
||||
return createToolResult(errorResponse);
|
||||
{ deliverableType: args.deliverable_type },
|
||||
));
|
||||
}
|
||||
};
|
||||
}
|
||||
@@ -94,7 +148,7 @@ function createSaveDeliverableHandler(targetDir: string) {
|
||||
export function createSaveDeliverableTool(targetDir: string) {
|
||||
return tool(
|
||||
'save_deliverable',
|
||||
'Saves deliverable files with automatic validation. Queue files must have {"vulnerabilities": [...]} structure.',
|
||||
'Saves deliverable files with automatic validation. Queue files must have {"vulnerabilities": [...]} structure. For large reports, write the file to disk first then pass file_path instead of inline content to avoid output token limits.',
|
||||
SaveDeliverableInputSchema.shape,
|
||||
createSaveDeliverableHandler(targetDir)
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user