feat: allow user override of hardcoded disallowed tools

[ashwin-ant]

Allow users to override hardcoded disallowed tools (WebSearch, WebFetch) by including them in their allowed_tools configuration.

Resolves #49

Generated with Claude Code

[claude]

Claude finished @ashwin-ant's task —— View job

---

PR Review: Allow user override of hardcoded disallowed tools

Review Progress:

• Analyze modified files and understand changes
• Review code quality and implementation approach
• Check test coverage and quality
• Provide comprehensive feedback and recommendations

Code Analysis

This PR introduces the ability for users to override hardcoded disallowed tools (WebSearch, WebFetch) by including them in their allowed_tools configuration. Here's my analysis:

✅ Strengths

1. Clean Implementation: The logic in buildDisallowedToolsString() at src/create-prompt/index.ts:59-80 is straightforward and well-implemented.

2. Proper Separation of Concerns: The function correctly:

   • Starts with hardcoded disallowed tools
   • Filters out tools that are explicitly allowed by the user
   • Appends any additional custom disallowed tools

3. Comprehensive Test Coverage: The test file adds 41 lines of tests covering:

   • Base functionality when no allowed tools are provided
   • Partial override (allowing only WebSearch)
   • Complete override (allowing both WebSearch and WebFetch)
   • Edge cases with custom disallowed tools

4. Backwards Compatibility: Existing behavior is preserved - the default disallowed tools remain unless explicitly overridden.

🔍 Code Quality Observations

src/create-prompt/index.ts:59-80

export function buildDisallowedToolsString(
  customDisallowedTools?: string,
  allowedTools?: string,
): string {
  let disallowedTools = [...DISALLOWED_TOOLS];
  
  // If user has explicitly allowed some hardcoded disallowed tools, remove them from disallowed list
  if (allowedTools) {
    const allowedToolsArray = allowedTools.split(",").map(tool => tool.trim());
    disallowedTools = disallowedTools.filter(tool => !allowedToolsArray.includes(tool));
  }
  
  let allDisallowedTools = disallowedTools.join(",");
  if (customDisallowedTools) {
    if (allDisallowedTools) {
      allDisallowedTools = `${allDisallowedTools},${customDisallowedTools}`;
    } else {
      allDisallowedTools = customDisallowedTools;
    }
  }
  return allDisallowedTools;
}

Positive aspects:

• Uses spread operator for array cloning to avoid mutations
• Proper string trimming to handle whitespace
• Handles edge case where all hardcoded tools are overridden
• Clear variable naming and logical flow

📝 Minor Suggestions

1. Consider More Robust String Handling: The current implementation splits on commas and trims, which works well. However, consider if you want to handle empty strings in the comma-separated list (e.g., "Tool1,,Tool2").

2. Documentation: Consider adding JSDoc comments to explain the override behavior:

   /**
    * Builds the disallowed tools string, allowing users to override hardcoded restrictions
    * @param customDisallowedTools - Additional tools to disallow
    * @param allowedTools - Tools explicitly allowed (overrides hardcoded disallowed tools)
    * @returns Comma-separated string of disallowed tools
    */

🧪 Test Quality Assessment

The tests in test/create-prompt.test.ts:699-766 are well-structured:

• Good coverage: Tests base case, partial override, complete override, and combinations
• Clear test names: Each test clearly describes what scenario it's testing
• Proper assertions: Tests verify both presence and absence of tools as expected
• Edge cases covered: Handles scenarios where all hardcoded tools are overridden

🏗️ Architecture & Design

The implementation fits well within the existing architecture:

• Follows existing patterns in the codebase
• Integrates cleanly with the prompt generation system
• Maintains the existing API while adding flexibility
• The change is isolated and doesn't affect other functionality

🔒 Security Considerations

This change actually improves security flexibility by allowing users to grant access to previously hardcoded-blocked tools only when explicitly requested. The default behavior remains secure.

✅ Overall Assessment

This is a high-quality implementation that:

• Solves the stated problem effectively
• Maintains backwards compatibility
• Includes comprehensive test coverage
• Follows good coding practices
• Has minimal complexity and clear logic

The code is ready to merge. Great work on addressing issue #49!