Skip to content

Add comprehensive tests verifying RequiresApproval attribute blocks on approval service#70

Merged
johnkord merged 2 commits intomainfrom
copilot/fix-69
Jun 25, 2025
Merged

Add comprehensive tests verifying RequiresApproval attribute blocks on approval service#70
johnkord merged 2 commits intomainfrom
copilot/fix-69

Conversation

Copy link
Copy Markdown

Copilot AI commented Jun 25, 2025

This PR adds comprehensive test coverage to verify that the RequiresApproval attribute correctly blocks tool execution waiting for approval from the approval service, and ensures that tools without the attribute execute immediately without approval checks.

Problem

The existing tests only verified static attribute detection and wrapper creation, but lacked integration tests demonstrating the actual blocking behavior during tool execution. There was no verification that:

  • Tools with [RequiresApproval] actually block and wait for approval
  • Tools without the attribute bypass the approval system entirely
  • Different approval scenarios (approved, denied, timeout) work correctly

Solution

Added 5 new test files with 21 comprehensive tests:

New Test Infrastructure

  • TestApprovalProvider.cs - Controllable test approval provider for simulating approval responses
  • Supports queued responses, delays, timeouts, and request tracking

Core Functionality Tests

  • RequiresApprovalBlockingTests.cs - Core tests verifying:
    • Tools with [RequiresApproval] are wrapped by ToolApprovalWrapper
    • Tools without attribute execute immediately (not wrapped)
    • Tools with [RequiresApproval(false)] execute immediately
    • Attribute detection works correctly

Integration Tests

  • ApprovalSystemIntegrationTests.cs - Tests approval provider behavior:
    • Approval and denial scenarios
    • Request timing and delays
    • Timeout handling
    • Multiple request queuing

End-to-End Tests

  • EndToEndApprovalTests.cs - Complete approval flow verification
  • ComprehensiveApprovalBlockingTests.cs - Realistic scenarios with file/network operations

Test Coverage

The new tests verify:

  • Tools with [RequiresApproval] are wrapped and would block for approval
  • Tools without RequiresApproval execute immediately without approval checks
  • Tools with [RequiresApproval(false)] execute immediately
  • Approval requests are properly queued and processed
  • Integration with existing tools like ShellTools.RunCommand
  • Multiple approval scenarios (approve, deny, timeout)
  • Request timing and ordering behavior

Results

  • Total tests: 61 (40 existing + 21 new)
  • All tests passing: ✅
  • No breaking changes to existing functionality

The implementation successfully demonstrates that the RequiresApproval system works as intended - dangerous operations are properly gated by the approval service while safe operations execute immediately.

Fixes #69.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…n approval service

Co-authored-by: johnkord <16021727+johnkord@users.noreply.github.com>
Copilot AI changed the title [WIP] Write tests that verifies that the RequiresApproval attribute blocks on the Approval service Add comprehensive tests verifying RequiresApproval attribute blocks on approval service Jun 25, 2025
Copilot AI requested a review from johnkord June 25, 2025 17:13
@johnkord johnkord marked this pull request as ready for review June 25, 2025 19:33
@johnkord johnkord merged commit e68cf8c into main Jun 25, 2025
@johnkord johnkord deleted the copilot/fix-69 branch June 25, 2025 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Write tests that verifies that the RequiresApproval attribute blocks on the Approval service

2 participants