Address PR review feedback

- Remove redundant docblock comments
- Use readonly properties instead of readonly class
- Rename variable to $event for consistency
- Simplify event subscriber method signature

Author: chr-hertel

src/agent/CHANGELOG.md

@@ -4,10 +4,8 @@ CHANGELOG
 0.4
 ---
 
- * Add human-in-the-loop tool confirmation system with `PolicyInterface`, `ConfirmationHandlerInterface`, and `ConfirmationSubscriber`
+ * Add human-in-the-loop tool confirmation system
  * Add `ToolCallRequested` event dispatched before tool execution
- * Add `DefaultPolicy` for smart auto-allow of read operations with configurable allow/deny lists
- * Add `AlwaysAllowPolicy` for bypassing confirmation checks
 
 0.3
 ---

src/agent/src/Toolbox/Confirmation/AlwaysAllowPolicy.php

@@ -15,7 +15,6 @@
 
 /**
  * Policy that always allows tool execution without confirmation.
- *
  * Use with caution - this bypasses all safety checks.
  *
  * @author Christopher Hertel <mail@christopher-hertel.de>

src/agent/src/Toolbox/Confirmation/ConfirmationResult.php

@@ -12,15 +12,13 @@
 namespace Symfony\AI\Agent\Toolbox\Confirmation;
 
 /**
- * Represents the result of a user confirmation request.
- *
  * @author Christopher Hertel <mail@christopher-hertel.de>
  */
-final readonly class ConfirmationResult
+final class ConfirmationResult
 {
     private function __construct(
-        public bool $confirmed,
-        public bool $rememberChoice,
+        public readonly bool $confirmed,
+        public readonly bool $rememberChoice,
     ) {
     }
 

src/agent/src/Toolbox/Confirmation/ConfirmationSubscriber.php

@@ -15,13 +15,6 @@
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 /**
- * Listens to ToolCallRequested events and applies policy-based confirmation.
- *
- * This subscriber enables human-in-the-loop patterns by:
- * 1. Checking the policy for each tool call
- * 2. Asking the user for confirmation when required
- * 3. Denying or allowing the tool call based on the result
- *
  * @author Christopher Hertel <mail@christopher-hertel.de>
  */
 final class ConfirmationSubscriber implements EventSubscriberInterface
@@ -35,7 +28,7 @@ public function __construct(
     public static function getSubscribedEvents(): array
     {
         return [
-            ToolCallRequested::class => ['onToolCallRequested', 0],
+            ToolCallRequested::class => 'onToolCallRequested',
         ];
     }
 

src/agent/src/Toolbox/Confirmation/PolicyDecision.php

@@ -12,24 +12,11 @@
 namespace Symfony\AI\Agent\Toolbox\Confirmation;
 
 /**
- * Represents the decision made by a policy for a tool call.
- *
  * @author Christopher Hertel <mail@christopher-hertel.de>
  */
 enum PolicyDecision: string
 {
-    /**
-     * Allow the tool execution without asking the user.
-     */
     case Allow = 'allow';
-
-    /**
-     * Deny the tool execution without asking the user.
-     */
     case Deny = 'deny';
-
-    /**
-     * Ask the user for confirmation before executing the tool.
-     */
     case AskUser = 'ask_user';
 }

src/agent/src/Toolbox/Event/ToolCallRequested.php

@@ -17,13 +17,6 @@
 use Symfony\AI\Platform\Tool\Tool;
 
 /**
- * Dispatched before a tool is executed, allowing listeners to deny or modify the execution.
- *
- * This event supports human-in-the-loop patterns by allowing:
- * - Denying tool execution with a reason
- * - Setting a custom result to skip execution
- * - Inspecting the tool call before it happens
- *
  * @author Christopher Hertel <mail@christopher-hertel.de>
  */
 final class ToolCallRequested implements StoppableEventInterface

src/agent/src/Toolbox/Toolbox.php

@@ -79,18 +79,17 @@ public function execute(ToolCall $toolCall): ToolResult
     {
         $metadata = $this->getMetadata($toolCall);
 
-        // Dispatch stoppable event before execution for human-in-the-loop patterns
-        $requestedEvent = new ToolCallRequested($toolCall, $metadata);
-        $this->eventDispatcher?->dispatch($requestedEvent);
+        $event = new ToolCallRequested($toolCall, $metadata);
+        $this->eventDispatcher?->dispatch($event);
 
-        if ($requestedEvent->isDenied()) {
-            $this->logger->debug(\sprintf('Tool "%s" denied: %s', $toolCall->getName(), $requestedEvent->getDenialReason()));
+        if ($event->isDenied()) {
+            $this->logger->debug(\sprintf('Tool "%s" denied: %s', $toolCall->getName(), $event->getDenialReason()));
 
-            return new ToolResult($toolCall, $requestedEvent->getDenialReason() ?? 'Tool execution denied.');
+            return new ToolResult($toolCall, $event->getDenialReason() ?? 'Tool execution denied.');
         }
 
-        if ($requestedEvent->hasResult()) {
-            return $requestedEvent->getResult();
+        if ($event->hasResult()) {
+            return $event->getResult();
         }
 
         $tool = $this->getExecutable($metadata);

src/agent/tests/Fixtures/Tool/DeleteFileTool.php

@@ -0,0 +1,23 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\AI\Agent\Tests\Fixtures\Tool;
+
+use Symfony\AI\Agent\Toolbox\Attribute\AsTool;
+
+#[AsTool('delete_file', 'Deletes a file')]
+final class DeleteFileTool
+{
+    public function __invoke(string $path): string
+    {
+        return 'Deleted '.$path;
+    }
+}

src/agent/tests/Fixtures/Tool/ReadFileTool.php

@@ -0,0 +1,23 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\AI\Agent\Tests\Fixtures\Tool;
+
+use Symfony\AI\Agent\Toolbox\Attribute\AsTool;
+
+#[AsTool('read_file', 'Reads content from a file')]
+final class ReadFileTool
+{
+    public function __invoke(string $path): string
+    {
+        return 'Content of '.$path;
+    }
+}

src/agent/tests/Fixtures/Tool/WriteFileTool.php

@@ -0,0 +1,23 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\AI\Agent\Tests\Fixtures\Tool;
+
+use Symfony\AI\Agent\Toolbox\Attribute\AsTool;
+
+#[AsTool('write_file', 'Writes content to a file')]
+final class WriteFileTool
+{
+    public function __invoke(string $path, string $content): string
+    {
+        return 'Written to '.$path;
+    }
+}