From 4317a4d14af5a42f3bbb4ff7e76b98a55a139109 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 14:35:22 +0000 Subject: [PATCH 01/17] feat: add publishCustomEvent with TypeScript-parity server and session forms --- src/AgentCat.php | 27 + src/Client.php | 80 +++ src/CustomEvent.php | 39 ++ src/Internal/Engine/Publisher.php | 42 +- src/McpSdk/Internal/Access/ServerAccess.php | 36 ++ src/McpSdk/Internal/CallToolInterceptor.php | 6 + src/McpSdk/Internal/Drift.php | 14 +- src/McpSdk/McpSdkAdapter.php | 34 ++ tests/Contract/ManifestTest.php | 2 + tests/Contract/McpSdk/ServerMembersTest.php | 123 +++++ tests/Integration/PublishCustomEventTest.php | 525 +++++++++++++++++++ tests/Unit/AgentCatTest.php | 26 + tests/Unit/CustomEventTest.php | 60 +++ tests/Unit/Internal/Engine/PublisherTest.php | 82 +++ 14 files changed, 1082 insertions(+), 14 deletions(-) create mode 100644 src/CustomEvent.php create mode 100644 src/McpSdk/Internal/Access/ServerAccess.php create mode 100644 tests/Contract/McpSdk/ServerMembersTest.php create mode 100644 tests/Integration/PublishCustomEventTest.php create mode 100644 tests/Unit/CustomEventTest.php diff --git a/src/AgentCat.php b/src/AgentCat.php index 186c1c1..5b833cb 100644 --- a/src/AgentCat.php +++ b/src/AgentCat.php @@ -42,6 +42,33 @@ public static function track(object $server, ?string $projectId = null, ?Options return self::client()->track($server, $projectId, $options); } + /** + * Publishes a custom event, for a tracked server or for a session. + * + * With a server tracked by any client (the Builder, or the server built from it), the event goes to the project + * the server was tracked with, whatever `$projectId` says, and carries the server name and version and the SDK + * fields. The server's `redactEvent` option applies to it; `redactSensitiveInformation` does not. When the server + * was tracked with `enableTracing: false`, the event is dropped. Without `$event->sessionId`, the event has no + * session. + * + * With a session ID string, the event goes to `$projectId` for that session, used as it is; `$event->sessionId` + * wins over it. No server or SDK fields and no hooks apply. + * + * The event is queued, not sent: AgentCat delivers it like any other event. Apart from the two errors below, it + * never throws: a failure is logged. In test environments it validates `$projectId` and then does nothing, like + * `track()` (see `AGENTCAT_ENABLED` there). + * + * @param object|string $serverOrSessionId A tracked server, or the session ID of the event. + * @param string|null $projectId Your AgentCat project ID (`proj_…`); required, even for a tracked server. + * @param CustomEvent|null $event The event data; null sends an event with no data. + * + * @throws \InvalidArgumentException when `$projectId` is null or empty, or when the server is not tracked + */ + public static function publishCustomEvent(object|string $serverOrSessionId, ?string $projectId, ?CustomEvent $event = null): void + { + self::client()->publishCustomEvent($serverOrSessionId, $projectId, $event); + } + /** * Sends the queued events now and waits for them. Rarely needed: AgentCat delivers on its own, between requests * and when the process ends. diff --git a/src/Client.php b/src/Client.php index 9ae9e9e..25466e6 100644 --- a/src/Client.php +++ b/src/Client.php @@ -5,15 +5,19 @@ namespace AgentCat; use AgentCat\Internal\ClientSeams; +use AgentCat\Internal\Constants; use AgentCat\Internal\Delivery\EventQueue; use AgentCat\Internal\Engine\CallInterceptor; use AgentCat\Internal\Engine\Publisher; use AgentCat\Internal\Engine\TrackedServer; use AgentCat\Internal\Engine\TrackedServers; use AgentCat\Internal\Logging\Log; +use AgentCat\Internal\Pipeline\QueuedEvent; use AgentCat\Internal\Support\Env; use AgentCat\Internal\Support\ErrorText; use AgentCat\Internal\Support\TestMode; +use AgentCat\Internal\Support\Values; +use AgentCat\Internal\Tags\TagValidator; use AgentCat\McpSdk\McpSdkAdapter; use Psr\Log\LoggerInterface; @@ -124,6 +128,82 @@ public function track(object $server, ?string $projectId = null, ?Options $optio } } + /** + * Publishes a custom event, for a tracked server or for a session. + * + * With a server tracked by any client (the Builder, or the server built from it), the event goes to the project + * the server was tracked with, whatever `$projectId` says, and carries the server name and version and the SDK + * fields. The server's `redactEvent` option applies to it; `redactSensitiveInformation` does not. When the server + * was tracked with `enableTracing: false`, the event is dropped. Without `$event->sessionId`, the event has no + * session. + * + * With a session ID string, the event goes to `$projectId` for that session, used as it is; `$event->sessionId` + * wins over it. No server or SDK fields and no hooks apply. + * + * The event is queued, not sent: AgentCat delivers it like any other event. Apart from the two errors below, it + * never throws: a failure is logged. In test environments it validates `$projectId` and then does nothing, like + * `track()` (see `AGENTCAT_ENABLED` there). + * + * @param object|string $serverOrSessionId A tracked server, or the session ID of the event. + * @param string|null $projectId Your AgentCat project ID (`proj_…`); required, even for a tracked server. + * @param CustomEvent|null $event The event data; null sends an event with no data. + * + * @throws \InvalidArgumentException when `$projectId` is null or empty, or when the server is not tracked + */ + public function publishCustomEvent(object|string $serverOrSessionId, ?string $projectId, ?CustomEvent $event = null): void + { + if ($projectId === null || $projectId === '') { + throw new \InvalidArgumentException('projectId is required for publishCustomEvent'); + } + if (!($this->enabled)()) { + return; // D44 AGENTCAT_ENABLED, else D31: track() tracked nothing here, so no server is looked up + } + $event ??= new CustomEvent(); + $sessionId = $event->sessionId ?? ''; + $server = null; + if (is_string($serverOrSessionId)) { + $sessionId = $sessionId !== '' ? $sessionId : $serverOrSessionId; // verbatim: no derivation, '' is sessionless + } else { + $server = TrackedServers::get($serverOrSessionId) ?? McpSdkAdapter::trackedServer($serverOrSessionId); + if ($server === false) { + return; // mcp/sdk internals changed: the adapter logged the compatibility line + } + if (!$server instanceof TrackedServer) { + throw new \InvalidArgumentException('Server is not tracked. Please call agentcat.track() first or provide a session ID string.'); + } + if ($sessionId === '') { + Log::write('publishCustomEvent: no sessionId provided; event will be published without a session. Pass eventData.sessionId to attribute it to a task.'); + } + } + + try { + $custom = new Event( + sessionId: $sessionId, + projectId: $projectId, + eventType: Constants::CUSTOM_EVENT_TYPE, + duration: $event->duration, + resourceName: $event->resourceName, + parameters: $event->parameters, + response: $event->response, + userIntent: $event->message, + isError: $event->isError, + error: $event->error, // sent as it is: no exception capture + tags: TagValidator::validate($event->tags), // null for null or no valid entry + ); + if (Values::isObjectLike($event->properties) && Values::entries($event->properties) !== []) { + $custom->properties = $event->properties; + } + if ($server instanceof TrackedServer) { + $server->publisher()->publishCustom($server, $custom); // the queue of the client that tracked the server + } else { + $this->publisher->queue()->add(new QueuedEvent($custom)); + } + Log::write(sprintf("Published custom event %s with type 'agentcat:custom'", $sessionId !== '' ? 'for session ' . $sessionId : 'without a session')); + } catch (\Throwable $e) { + Log::write('Warning: Failed to publish custom event - ' . ErrorText::of($e)); + } + } + /** * Sends the queued events now and waits for them. Rarely needed: AgentCat delivers on its own, between requests * and when the process ends. diff --git a/src/CustomEvent.php b/src/CustomEvent.php new file mode 100644 index 0000000..16248f3 --- /dev/null +++ b/src/CustomEvent.php @@ -0,0 +1,39 @@ +|null $tags Tags: string keys and string values. Invalid entries are left out and + * logged, like the tags of the `eventTags` option. + * @param mixed $properties JSON-encodable properties; sent only when they have at least one entry. + */ + public function __construct( + public ?string $sessionId = null, + public ?string $resourceName = null, + public mixed $parameters = null, + public mixed $response = null, + public ?string $message = null, + public int|float|null $duration = null, + public ?bool $isError = null, + public mixed $error = null, + public ?array $tags = null, + public mixed $properties = null, + ) {} +} diff --git a/src/Internal/Engine/Publisher.php b/src/Internal/Engine/Publisher.php index c824ac0..7ee7f9a 100644 --- a/src/Internal/Engine/Publisher.php +++ b/src/Internal/Engine/Publisher.php @@ -11,7 +11,8 @@ use AgentCat\Internal\Support\Js; /** - * Stamps tool-call events and hands them to the event queue (TS `publishEvent`). Nothing customer-written runs here. + * Stamps the events of tracked servers, tool calls and custom events, and hands them to the event queue (TS + * `publishEvent`). Nothing customer-written runs here. * * @internal */ @@ -38,15 +39,23 @@ public function publishToolCall(TrackedServer $server, Event $event, ?string $cl if (!$server->options->enableTracing) { return; // TS publishEvent; tool calls never get here with tracing off } - [$serverName, $serverVersion] = $server->serverInfo(); - (new SessionInfo($serverName, $serverVersion, $clientName, $clientVersion))->stamp($event); - $event->projectId = $server->projectId !== '' ? $server->projectId : null; // telemetry-only mode has no project (C16) - if (!Js::truthy($event->duration)) { - $event->duration = self::elapsedMs($event->timestamp, ($this->clock)()); - } + $this->stamp($server, $event, $clientName, $clientVersion); $this->queue->add(new QueuedEvent($event, $server->options->redactEvent, $server->options->redactSensitiveInformation, $pending)); } + /** + * A custom event of a tracked server (TS `publishEvent` from `publishCustomEvent`): the tracked project replaces + * the one the event has (B09), and only the `redactEvent` hook applies (D08). Nothing when tracing is off. + */ + public function publishCustom(TrackedServer $server, Event $event): void + { + if (!$server->options->enableTracing) { + return; // B09: dropped silently + } + $this->stamp($server, $event, null, null); + $this->queue->add(new QueuedEvent($event, $server->options->redactEvent)); + } + public function pump(): void { $this->queue->pump(); @@ -60,6 +69,23 @@ public function queue(): EventQueue /** Whole milliseconds like the difference of two `Date.getTime()` values. */ public static function elapsedMs(\DateTimeImmutable $from, \DateTimeImmutable $to): int { - return (int) $to->format('Uv') - (int) $from->format('Uv'); + return self::epochMs($to) - self::epochMs($from); + } + + /** `Date.getTime()`: whole milliseconds since the Unix epoch. */ + private static function epochMs(\DateTimeImmutable $time): int + { + return (int) $time->format('Uv'); + } + + /** Server and SDK fields, the tracked project, and the elapsed time when the event has no duration. */ + private function stamp(TrackedServer $server, Event $event, ?string $clientName, ?string $clientVersion): void + { + [$serverName, $serverVersion] = $server->serverInfo(); + (new SessionInfo($serverName, $serverVersion, $clientName, $clientVersion))->stamp($event); + $event->projectId = $server->projectId !== '' ? $server->projectId : null; // telemetry-only mode has no project (C16) + if (!Js::truthy($event->duration)) { + $event->duration = self::elapsedMs($event->timestamp, ($this->clock)()); + } } } diff --git a/src/McpSdk/Internal/Access/ServerAccess.php b/src/McpSdk/Internal/Access/ServerAccess.php new file mode 100644 index 0000000..7a256c1 --- /dev/null +++ b/src/McpSdk/Internal/Access/ServerAccess.php @@ -0,0 +1,36 @@ + $s->protocol, null, Server::class); + + return $read($server); + } + + /** @return array the handlers build() assembled, AgentCat's included; the entries are not checked here */ + #[TouchesPrivateMember(class: Protocol::class, member: 'requestHandlers', type: 'array', readonly: true)] + public static function requestHandlers(Protocol $protocol): array + { + $read = \Closure::bind(static fn(Protocol $p): array => $p->requestHandlers, null, Protocol::class); + + return $read($protocol); + } +} diff --git a/src/McpSdk/Internal/CallToolInterceptor.php b/src/McpSdk/Internal/CallToolInterceptor.php index 78fa68f..799d84f 100644 --- a/src/McpSdk/Internal/CallToolInterceptor.php +++ b/src/McpSdk/Internal/CallToolInterceptor.php @@ -35,6 +35,12 @@ public function __construct( private RequestReader $reader, ) {} + /** The tracking state of the server: a server built from a tracked Builder finds it here. */ + public function server(): TrackedServer + { + return $this->server; + } + #[\Override] public function supports(Request $request): bool { diff --git a/src/McpSdk/Internal/Drift.php b/src/McpSdk/Internal/Drift.php index a09df1f..0670878 100644 --- a/src/McpSdk/Internal/Drift.php +++ b/src/McpSdk/Internal/Drift.php @@ -6,26 +6,28 @@ use AgentCat\Internal\Logging\Log; use AgentCat\McpSdk\McpSdkAdapter; +use Mcp\Server; use Mcp\Server\Builder; /** - * Runtime drift in what a Builder's build() assembled (C6, README §4.5 rule 4). The first drift on a Builder logs the - * compatibility line; later ones on the same Builder log nothing. Entries go away with their Builder. + * Runtime drift in what a Builder's build() assembled, or in a built server (C6, README §4.5 rule 4). The first drift + * on an object logs the compatibility line; later ones on the same object log nothing. Entries go away with their + * object. * * @internal */ final class Drift { - /** @var \WeakMap|null */ + /** @var \WeakMap|null */ private static ?\WeakMap $reported = null; - public static function report(Builder $builder): void + public static function report(Builder|Server $object): void { self::$reported ??= new \WeakMap(); - if (self::$reported[$builder] ?? false) { + if (self::$reported[$object] ?? false) { return; } - self::$reported[$builder] = true; + self::$reported[$object] = true; Log::warning(McpSdkAdapter::COMPATIBILITY_LINE); } } diff --git a/src/McpSdk/McpSdkAdapter.php b/src/McpSdk/McpSdkAdapter.php index 6e2d301..677a99a 100644 --- a/src/McpSdk/McpSdkAdapter.php +++ b/src/McpSdk/McpSdkAdapter.php @@ -7,11 +7,13 @@ use AgentCat\Internal\Engine\CallInterceptor; use AgentCat\Internal\Engine\Publisher; use AgentCat\Internal\Engine\TrackedServer; +use AgentCat\Internal\Engine\TrackedServers; use AgentCat\Internal\Logging\Log; use AgentCat\Internal\SdkVersion; use AgentCat\Internal\Support\ErrorText; use AgentCat\Internal\Support\StrictErrors; use AgentCat\McpSdk\Internal\Access\BuilderAccess; +use AgentCat\McpSdk\Internal\Access\ServerAccess; use AgentCat\McpSdk\Internal\CallToolInterceptor; use AgentCat\McpSdk\Internal\CapturingReferenceHandler; use AgentCat\McpSdk\Internal\Delegates; @@ -133,6 +135,38 @@ public static function serverInfo(Builder $builder): \Closure }; } + /** + * The tracking state of a server built from a tracked Builder (01 AC-69): AgentCat's `tools/call` handler, among + * the server's request handlers, holds it. A state found is kept for the server. Never throws. + * + * @return TrackedServer|false|null null when the object is not an mcp/sdk server or its Builder was not tracked; + * false when its request handlers cannot be read (C6), after the compatibility + * line, which is logged once per server + */ + public static function trackedServer(object $server): TrackedServer|false|null + { + if (!$server instanceof Server) { + return null; + } + try { + $handlers = StrictErrors::run(static fn(): array => ServerAccess::requestHandlers(ServerAccess::protocol($server))); + } catch (\Throwable) { + Drift::report($server); + + return false; + } + foreach ($handlers as $handler) { + if ($handler instanceof CallToolInterceptor) { + $tracked = $handler->server(); + TrackedServers::add($server, $tracked); + + return $tracked; + } + } + + return null; + } + /** * Wraps the reference handler, then registers AgentCat's request handlers, which run before the library's own * (02 §1.4). Every read comes before the first change, so a read that throws leaves the Builder untouched. diff --git a/tests/Contract/ManifestTest.php b/tests/Contract/ManifestTest.php index d193b54..b13a7a8 100644 --- a/tests/Contract/ManifestTest.php +++ b/tests/Contract/ManifestTest.php @@ -19,10 +19,12 @@ public function testTheManifestListsExactlyTheMembersAgentCatDependsOn(): void [ 'Mcp\Schema\Result\CallToolResult::__construct(array $content, bool $isError = false, mixed $structuredContent = null, ?array $meta = null)', 'Mcp\Schema\Result\ListToolsResult::__construct(array $tools, ?string $nextCursor = null)', + 'Mcp\Server::$protocol', 'Mcp\Server\Builder::$container', 'Mcp\Server\Builder::$logger', 'Mcp\Server\Builder::$parts', 'Mcp\Server\Builder::$referenceHandler', + 'Mcp\Server\Protocol::$requestHandlers', ], PrivateMemberManifest::members(), ); diff --git a/tests/Contract/McpSdk/ServerMembersTest.php b/tests/Contract/McpSdk/ServerMembersTest.php new file mode 100644 index 0000000..b1cfa98 --- /dev/null +++ b/tests/Contract/McpSdk/ServerMembersTest.php @@ -0,0 +1,123 @@ +fixture = new TrackingFixture(); + } + + #[\Override] + protected function tearDown(): void + { + Log::reset(); + } + + public function testTheProtocolHoldsTheHandlersBuildAssembled(): void + { + $builder = TrackingFixture::builder(); + $server = $builder->build(); + + $protocol = ServerAccess::protocol($server); + $handlers = ServerAccess::requestHandlers($protocol); + + $this->assertSame($protocol, ServerAccess::protocol($server), 'the same protocol on every read'); + $this->assertSame(BuilderAccess::requestHandlers($builder), $handlers); + $this->assertNotSame([], array_filter($handlers, static fn(mixed $handler): bool => $handler instanceof CallToolHandler)); + } + + public function testATrackedServerHoldsAgentCatsCallHandler(): void + { + $builder = TrackingFixture::builder(); + $state = $this->fixture->track($builder); + $server = $builder->build(); + + $handlers = ServerAccess::requestHandlers(ServerAccess::protocol($server)); + $calls = array_values(array_filter($handlers, static fn(mixed $handler): bool => $handler instanceof CallToolInterceptor)); + + $this->assertSame([$state], array_map(static fn(CallToolInterceptor $handler): TrackedServer => $handler->server(), $calls)); + } + + public function testTheAdapterFindsAndKeepsTheTrackingStateOfABuiltServer(): void + { + $builder = TrackingFixture::builder(); + $state = $this->fixture->track($builder); + $server = $builder->build(); + $this->assertNull(TrackedServers::get($server)); + + $this->assertSame($state, McpSdkAdapter::trackedServer($server)); + + $this->assertSame($state, TrackedServers::get($server)); + $this->assertSame([], $this->fixture->lines()); + } + + public function testTheAdapterFindsNoStateForAnUntrackedServerOrAnotherObject(): void + { + $this->assertNull(McpSdkAdapter::trackedServer(TrackingFixture::builder()->build())); + $this->assertNull(McpSdkAdapter::trackedServer(TrackingFixture::builder())); + $this->assertNull(McpSdkAdapter::trackedServer(new \stdClass())); + $this->assertSame([], $this->fixture->lines()); + } + + public function testAnUnreadableProtocolThrows(): void + { + $server = (new \ReflectionClass(Server::class))->newInstanceWithoutConstructor(); + + $error = Thrown::by(\Error::class, static fn(): Protocol => ServerAccess::protocol($server)); + + $this->assertStringContainsString('Mcp\Server::$protocol must not be accessed before initialization', $error->getMessage()); + } + + public function testUnreadableRequestHandlersThrow(): void + { + $protocol = (new \ReflectionClass(Protocol::class))->newInstanceWithoutConstructor(); + + $error = Thrown::by(\Error::class, static fn(): array => ServerAccess::requestHandlers($protocol)); + + $this->assertStringContainsString('Mcp\Server\Protocol::$requestHandlers must not be accessed before initialization', $error->getMessage()); + } + + public function testDriftIsReportedOncePerServer(): void + { + $servers = [ + (new \ReflectionClass(Server::class))->newInstanceWithoutConstructor(), + new Server((new \ReflectionClass(Protocol::class))->newInstanceWithoutConstructor()), + ]; + + foreach ($servers as $server) { + $this->assertFalse(McpSdkAdapter::trackedServer($server)); + $this->assertFalse(McpSdkAdapter::trackedServer($server)); + } + + $line = ['level' => LogLevel::WARNING, 'message' => McpSdkAdapter::COMPATIBILITY_LINE]; + $this->assertSame([$line, $line], $this->fixture->logger->records); + } +} diff --git a/tests/Integration/PublishCustomEventTest.php b/tests/Integration/PublishCustomEventTest.php new file mode 100644 index 0000000..689935c --- /dev/null +++ b/tests/Integration/PublishCustomEventTest.php @@ -0,0 +1,525 @@ +environment = Environment::clear('AGENTCAT_PROJECT_ID', 'AGENTCAT_API_URL', 'MCPCAT_API_URL', 'AGENTCAT_ENABLED', 'APP_ENV'); + $this->logger = new MemoryLogger(); + Log::setLogger($this->logger); + $this->transport = new RecordingTransport(); + $this->exporter = new RecordingExporter(); + $this->queue = new EventQueue(transport: $this->transport, exporters: $this->exporter); + $this->client = new Client(ClientSeams::of($this->queue, static fn(): bool => true)); + } + + #[\Override] + protected function tearDown(): void + { + Log::reset(); + $this->environment->restore(); + } + + /** @return iterable */ + public static function noProject(): iterable + { + yield 'null' => [null]; + yield 'empty' => ['']; + } + + #[DataProvider('noProject')] + #[Group('ac-01-66')] + public function testAProjectIdIsRequiredAndCheckedFirst(?string $project): void + { + $untracked = TrackingFixture::builder(); + + foreach ([$untracked, 'ses_1'] as $first) { + $error = Thrown::by(\InvalidArgumentException::class, fn() => $this->client->publishCustomEvent($first, $project, new CustomEvent())); + $this->assertSame(self::NO_PROJECT, $error->getMessage()); + } + $this->assertNothingQueued(); + $this->assertSame([], $this->logger->messages()); + } + + #[Group('ac-01-67')] + public function testAFirstArgumentThatIsNeitherObjectNorStringIsATypeError(): void + { + $bad = 42; + + $this->expectException(\TypeError::class); + + $this->client->publishCustomEvent($bad, 'p'); // @phpstan-ignore argument.type (proves PHP's own type check) + } + + /** @return iterable */ + public static function untracked(): iterable + { + yield 'a Builder' => [TrackingFixture::builder(...)]; + yield 'a built server' => [static fn(): object => TrackingFixture::builder()->build()]; + yield 'any object' => [static fn(): object => new \stdClass()]; + } + + /** @param \Closure(): object $make */ + #[DataProvider('untracked')] + #[Group('ac-01-68')] + public function testAnUntrackedServerIsRejected(\Closure $make): void + { + $server = $make(); + + $error = Thrown::by(\InvalidArgumentException::class, fn() => $this->client->publishCustomEvent($server, self::PROJECT, new CustomEvent(sessionId: 'ses_1'))); + + $this->assertSame(self::NOT_TRACKED, $error->getMessage()); + $this->assertNothingQueued(); + $this->assertSame([], $this->logger->messages()); + } + + #[Group('ac-01-70')] + #[Group('ac-03-33')] + #[Group('ac-01-77')] + public function testASessionIdStringIsUsedVerbatimWithoutSdkFields(): void + { + $this->client->publishCustomEvent('user-session-12345', 'proj_s', new CustomEvent(resourceName: 'r')); + + $body = $this->body(); + $this->assertSame( + ['user-session-12345', 'proj_s', Constants::CUSTOM_EVENT_TYPE, 'r'], + [JsonAssert::at($body, 'session_id'), JsonAssert::at($body, 'project_id'), JsonAssert::at($body, 'event_type'), JsonAssert::at($body, 'resource_name')], + ); + foreach (['sdk_language', 'agentcat_version', 'server_name', 'server_version', 'client_name', 'identify_data', 'duration', 'tags'] as $key) { + $this->assertObjectNotHasProperty($key, $body); + } + $this->assertSame(["Published custom event for session user-session-12345 with type 'agentcat:custom'"], $this->logger->messages()); + } + + #[Group('ac-01-71')] + public function testTheSessionIdOfTheEventWinsOverTheString(): void + { + $this->client->publishCustomEvent('ignored-string', 'proj_s', new CustomEvent(sessionId: 'from-data')); + + $this->assertSame('from-data', JsonAssert::at($this->body(), 'session_id')); + $this->assertSame(["Published custom event for session from-data with type 'agentcat:custom'"], $this->logger->messages()); + } + + public function testAnEmptySessionIdStringPublishesWithoutASession(): void + { + $this->client->publishCustomEvent('', 'proj_s', new CustomEvent(sessionId: '')); + + $this->assertNull(JsonAssert::at($this->body(), 'session_id')); + $this->assertSame([self::PUBLISHED_WITHOUT_SESSION], $this->logger->messages(), 'the string form never says that a session is missing'); + } + + /** @return iterable */ + public static function noSessionId(): iterable + { + yield 'none' => [null]; + yield 'empty' => ['']; + } + + #[DataProvider('noSessionId')] + #[Group('ac-01-72')] + public function testATrackedServerWithoutASessionIdPublishesWithoutASession(?string $sessionId): void + { + $builder = $this->tracked(); + + $this->client->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: $sessionId)); + + $this->assertNull(JsonAssert::at($this->body(), 'session_id')); + $this->assertSame([self::NO_SESSION, self::PUBLISHED_WITHOUT_SESSION], $this->logger->messages()); + } + + #[Group('ac-01-72')] + public function testATrackedServerWithASessionIdPublishesForIt(): void + { + $builder = $this->tracked(); + + $this->client->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_task')); + + $this->assertSame('ses_task', JsonAssert::at($this->body(), 'session_id')); + $this->assertSame(["Published custom event for session ses_task with type 'agentcat:custom'"], $this->logger->messages()); + } + + #[Group('ac-01-73')] + public function testTheMessageIsTheUserIntentAndTheTimestampIsTheCallTime(): void + { + $before = new \DateTimeImmutable('now', new \DateTimeZone('UTC')); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(message: 'why')); + $after = new \DateTimeImmutable('now', new \DateTimeZone('UTC')); + + $body = $this->body(); + $this->assertSame('why', JsonAssert::at($body, 'user_intent')); + $timestamp = JsonAssert::at($body, 'timestamp'); + $this->assertIsString($timestamp); + $sent = (float) (new \DateTimeImmutable($timestamp))->format('U.v'); + $this->assertGreaterThanOrEqual((float) $before->format('U.v') - 1.0, $sent); + $this->assertLessThanOrEqual((float) $after->format('U.v') + 1.0, $sent); + } + + #[Group('ac-01-74')] + public function testOnlyValidTagsAreSentAndNoSdkTags(): void + { + $builder = $this->tracked(); + + $this->client->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_1', tags: ['valid' => 'v', 'bad!key' => 'v'])); + + $this->assertEquals((object) ['valid' => 'v'], JsonAssert::at($this->body(), 'tags')); + $this->assertContains('Dropping invalid tag: "bad!key" — key contains invalid characters or is empty', $this->logger->messages()); + } + + #[Group('ac-01-74')] + public function testTagsWithoutAValidEntryAreLeftOut(): void + { + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(tags: ['bad!key' => 'v'])); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(tags: [])); + + foreach ($this->bodies(2) as $body) { + $this->assertObjectNotHasProperty('tags', $body); + } + } + + #[Group('ac-01-75')] + public function testPropertiesAreSentOnlyWithAnEntry(): void + { + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(properties: [])); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(properties: new \stdClass())); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(properties: 'not an object')); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(properties: ['a' => 1])); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(properties: (object) ['b' => 2])); + + $bodies = $this->bodies(5); + foreach (array_slice($bodies, 0, 3) as $body) { + $this->assertObjectNotHasProperty('properties', $body); + } + $this->assertEquals((object) ['a' => 1], JsonAssert::at($bodies, 3, 'properties')); + $this->assertEquals((object) ['b' => 2], JsonAssert::at($bodies, 4, 'properties')); + } + + #[Group('ac-01-76')] + public function testWithoutEventDataOnlyTheCoreFieldsAreSent(): void + { + $this->client->publishCustomEvent('ses_1', 'proj_s'); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent()); + + foreach ($this->bodies(2) as $body) { + $keys = array_keys(get_object_vars($body)); + sort($keys); + $this->assertSame(['event_type', 'id', 'project_id', 'session_id', 'timestamp'], $keys); + } + } + + #[Group('ac-01-77')] + public function testATrackedServerStampsTheServerAndSdkFields(): void + { + $builder = $this->tracked(); + + $this->client->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $body = $this->body(); + $this->assertSame( + ['harness-server', '1.2.3', 'PHP', SdkVersion::get(), Constants::CUSTOM_EVENT_TYPE], + [ + JsonAssert::at($body, 'server_name'), + JsonAssert::at($body, 'server_version'), + JsonAssert::at($body, 'sdk_language'), + JsonAssert::at($body, 'agentcat_version'), + JsonAssert::at($body, 'event_type'), + ], + ); + $this->assertEquals(new \stdClass(), JsonAssert::at($body, 'identify_data')); + $this->assertObjectNotHasProperty('client_name', $body); + } + + #[Group('ac-01-78')] + #[Group('ac-03-32')] + public function testATrackedServerSendsToItsTrackedProject(): void + { + $builder = $this->tracked(); + + $this->client->publishCustomEvent($builder, 'proj_other', new CustomEvent(sessionId: 'ses_1')); + + $this->assertSame(self::PROJECT, JsonAssert::at($this->body(), 'project_id')); + } + + #[Group('ac-01-78')] + #[Group('ac-03-32')] + public function testATelemetryOnlyServerExportsAndNeverPosts(): void + { + $builder = $this->tracked(project: ''); + + $this->client->publishCustomEvent($builder, 'proj_other', new CustomEvent(sessionId: 'ses_1')); + + $this->assertTrue($this->client->flush()); + $this->assertSame([], $this->transport->deliveries); + $this->assertCount(1, $this->exporter->events); + $this->assertNull($this->exporter->events[0]->projectId); + $this->assertSame(Constants::CUSTOM_EVENT_TYPE, $this->exporter->events[0]->eventType); + } + + public function testATrackedServerWithTracingOffDropsTheEventButSaysItPublished(): void + { + $builder = $this->tracked(new Options(enableTracing: false)); + + $this->client->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $this->assertNothingQueued(); + $this->assertSame(["Published custom event for session ses_1 with type 'agentcat:custom'"], $this->logger->messages()); + } + + public function testTheServerFormFillsAMissingDurationAndTheStringFormDoesNot(): void + { + $builder = $this->tracked(); + + $this->client->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent()); + $this->client->publishCustomEvent('ses_1', 'proj_s', new CustomEvent(duration: 12.5)); + + $duration = JsonAssert::at($this->body(0, 3), 'duration'); + $this->assertIsInt($duration); + $this->assertGreaterThanOrEqual(0, $duration); + $this->assertObjectNotHasProperty('duration', $this->body(1, 3)); + $this->assertSame(13, JsonAssert::at($this->body(2, 3), 'duration'), 'whole milliseconds on the wire'); + } + + #[Group('ac-01-54')] + #[Group('ac-05-P7')] + public function testRedactEventAppliesToTheServerFormOnlyAndRedactSensitiveInformationToNeither(): void + { + $builder = $this->tracked(new Options( + redactSensitiveInformation: static fn(string $text): string => '[R]', + redactEvent: static function (Event $event): Event { + $event->resourceName = 'redacted'; + + return $event; + }, + )); + $data = new CustomEvent(sessionId: 'ses_1', resourceName: 'r', parameters: ['msg' => 'secret'], message: 'why'); + + $this->client->publishCustomEvent($builder, self::PROJECT, $data); + $this->client->publishCustomEvent('ses_1', 'proj_s', $data); + + $server = $this->body(0, 2); + $string = $this->body(1, 2); + $this->assertSame('redacted', JsonAssert::at($server, 'resource_name')); + $this->assertSame('r', JsonAssert::at($string, 'resource_name')); + foreach ([$server, $string] as $body) { + $this->assertSame('secret', JsonAssert::at($body, 'parameters', 'msg')); + $this->assertSame('why', JsonAssert::at($body, 'user_intent')); + $this->assertSame('ses_1', JsonAssert::at($body, 'session_id')); + } + } + + public function testUnderPhpUnitTheDefaultClientValidatesTheProjectAndQueuesNothing(): void + { + $client = new Client(ClientSeams::of($this->queue)); + $untracked = TrackingFixture::builder(); + + $client->publishCustomEvent($untracked, 'proj_x'); + $client->publishCustomEvent('ses_1', 'proj_x', new CustomEvent()); + + $this->assertNothingQueued(); + $this->assertSame([], $this->logger->messages()); + $error = Thrown::by(\InvalidArgumentException::class, static fn() => $client->publishCustomEvent($untracked, null)); + $this->assertSame(self::NO_PROJECT, $error->getMessage()); + } + + /** Review Focus 4: payloads that PHP cannot encode as they are. */ + public function testAPayloadPhpCannotEncodeNeverThrows(): void + { + $self = ['name' => 'loop']; + $self['self'] = &$self; + $data = new CustomEvent(parameters: ['f' => static fn(): int => 1, 'r' => STDIN, 'self' => $self], response: NAN, message: "caf\xE9"); + + $this->client->publishCustomEvent('ses_1', 'proj_s', $data); + $this->assertTrue($this->client->flush()); + + $failures = array_values(array_filter($this->logger->messages(), static fn(string $line): bool => str_starts_with($line, 'Failed to '))); + $this->assertSame([], $this->transport->deliveries, 'the cycle drops the event'); + $this->assertSame(['Failed to sanitize event: RangeException: Maximum call stack size exceeded'], $failures); + } + + /** Review Focus 4: what remains encodable is delivered with the pipeline's markers and substitutions. */ + public function testAPayloadWithoutACycleIsDeliveredWithMarkers(): void + { + $data = new CustomEvent(parameters: ['f' => static fn(): int => 1, 'r' => STDIN], response: NAN, message: "caf\xE9"); + + $this->client->publishCustomEvent('ses_1', 'proj_s', $data); + + $body = $this->body(); + $intent = JsonAssert::at($body, 'user_intent'); + $this->assertIsString($intent); + $this->assertStringContainsString("\u{FFFD}", $intent); + $this->assertSame('[Resource: stream]', JsonAssert::at($body, 'parameters', 'r')); + } + + public function testAFailureAfterValidationIsLoggedAndSwallowed(): void + { + $object = new \stdClass(); + $publisher = new Publisher($this->queue); + TrackedServers::add($object, new TrackedServer(self::PROJECT, new Options(), static fn(): array => throw new \RuntimeException('boom'), $publisher)); + + $this->client->publishCustomEvent($object, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $this->assertNothingQueued(); + $this->assertSame(['Warning: Failed to publish custom event - RuntimeException: boom'], $this->logger->messages()); + } + + /** Ruling R1: the event of a server goes out through the client that tracked it. */ + public function testATrackedServerPublishesThroughTheClientThatTrackedIt(): void + { + $builder = $this->tracked(); + $otherTransport = new RecordingTransport(); + $other = new Client(ClientSeams::of(new EventQueue(transport: $otherTransport), static fn(): bool => true)); + + $other->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $this->assertTrue($other->flush()); + $this->assertSame([], $otherTransport->deliveries); + $this->assertSame('ses_1', JsonAssert::at($this->body(), 'session_id')); + } + + /** Ruling R3, `01` AC-69: the server built from a tracked Builder is accepted too. */ + #[Group('ac-01-69')] + public function testTheServerBuiltFromATrackedBuilderIsAccepted(): void + { + $builder = TrackingFixture::builder(); + $this->client->track($builder, self::PROJECT); + $server = $builder->build(); + $this->logger->records = []; + + $this->client->publishCustomEvent($server, 'proj_other', new CustomEvent(sessionId: 'ses_1')); + $this->client->publishCustomEvent($server, 'proj_other', new CustomEvent(sessionId: 'ses_2')); + + $bodies = $this->bodies(2); + $this->assertSame([self::PROJECT, 'harness-server', 'ses_1'], [JsonAssert::at($bodies, 0, 'project_id'), JsonAssert::at($bodies, 0, 'server_name'), JsonAssert::at($bodies, 0, 'session_id')]); + $this->assertSame('ses_2', JsonAssert::at($bodies, 1, 'session_id')); + $this->assertSame(TrackedServers::get($builder), TrackedServers::get($server), 'kept for the server'); + } + + /** Ruling R3: the state kept for a server is used without reading the server again. */ + public function testTheStateKeptForAServerIsUsedFirst(): void + { + $server = new Server((new \ReflectionClass(Protocol::class))->newInstanceWithoutConstructor()); // unreadable + TrackedServers::add($server, new TrackedServer(self::PROJECT, new Options(), static fn(): array => ['kept', '1.0'], new Publisher($this->queue))); + + $this->client->publishCustomEvent($server, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $this->assertSame('kept', JsonAssert::at($this->body(), 'server_name')); + $this->assertSame(["Published custom event for session ses_1 with type 'agentcat:custom'"], $this->logger->messages()); + } + + /** Ruling R3: a server whose members AgentCat cannot read logs the compatibility line once, and nothing else. */ + public function testAServerWhoseInternalsChangedLogsTheCompatibilityLineOnce(): void + { + $server = new Server((new \ReflectionClass(Protocol::class))->newInstanceWithoutConstructor()); + + $this->client->publishCustomEvent($server, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + $this->client->publishCustomEvent($server, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $this->assertNothingQueued(); + $this->assertSame([['level' => LogLevel::WARNING, 'message' => self::COMPATIBILITY]], $this->logger->records); + $this->assertNull(TrackedServers::get($server)); + } + + /** Tracks a fresh Builder for `harness-server` 1.2.3 and builds it, so its server info is known; clears the log. */ + private function tracked(Options $options = new Options(), string $project = self::PROJECT): Builder + { + $builder = TrackingFixture::builder(); + $this->client->track($builder, $project, $options); + $builder->build(); + $this->logger->records = []; + + return $builder; + } + + /** + * Delivers everything queued and decodes the bodies; fails unless exactly `$count` were posted. + * + * @return list<\stdClass> + */ + private function bodies(int $count): array + { + $this->assertTrue($this->client->flush()); + $this->assertCount($count, $this->transport->deliveries); + $bodies = []; + foreach ($this->transport->deliveries as $delivery) { + $body = Json::decode($delivery->body); + $this->assertInstanceOf(\stdClass::class, $body); + $bodies[] = $body; + } + + return $bodies; + } + + /** Delivers everything queued and decodes the body at `$index`; fails unless exactly `$of` were posted. */ + private function body(int $index = 0, int $of = 1): \stdClass + { + $body = $this->bodies($of)[$index] ?? null; + $this->assertInstanceOf(\stdClass::class, $body); + + return $body; + } + + private function assertNothingQueued(): void + { + $this->assertSame(['queueLength' => 0, 'activeRequests' => 0], $this->queue->stats()); + $this->assertTrue($this->queue->flush()); + $this->assertSame([], $this->exporter->events); + } +} diff --git a/tests/Unit/AgentCatTest.php b/tests/Unit/AgentCatTest.php index b59ea6f..b02f14b 100644 --- a/tests/Unit/AgentCatTest.php +++ b/tests/Unit/AgentCatTest.php @@ -6,14 +6,18 @@ use AgentCat\AgentCat; use AgentCat\Client; +use AgentCat\CustomEvent; use AgentCat\Event; use AgentCat\Internal\ClientSeams; use AgentCat\Internal\Delivery\EventQueue; +use AgentCat\Internal\Json\Json; use AgentCat\Internal\Logging\Log; use AgentCat\Internal\Pipeline\QueuedEvent; use AgentCat\Tests\Support\Environment; +use AgentCat\Tests\Support\JsonAssert; use AgentCat\Tests\Support\MemoryLogger; use AgentCat\Tests\Support\RecordingTransport; +use AgentCat\Tests\Support\Thrown; use PHPUnit\Framework\TestCase; final class AgentCatTest extends TestCase @@ -110,6 +114,28 @@ public function testFlushPassesItsTimeout(): void $this->assertEqualsWithDelta(2.5, $this->transport->now(), PHP_FLOAT_EPSILON, 'the simulated clock stops at the deadline'); } + public function testPublishCustomEventGoesThroughTheClient(): void + { + AgentCat::setClient($this->client); + + AgentCat::publishCustomEvent('ses_1', 'proj_x', new CustomEvent(resourceName: 'r')); + + $this->assertTrue(AgentCat::flush()); + $this->assertCount(1, $this->transport->deliveries); + $body = Json::decode($this->transport->deliveries[0]->body); + $this->assertSame(['ses_1', 'proj_x', 'r'], [JsonAssert::at($body, 'session_id'), JsonAssert::at($body, 'project_id'), JsonAssert::at($body, 'resource_name')]); + $this->assertSame(["Published custom event for session ses_1 with type 'agentcat:custom'"], $this->logger->messages()); + } + + public function testPublishCustomEventPassesItsValidationErrors(): void + { + AgentCat::setClient($this->client); + + $error = Thrown::by(\InvalidArgumentException::class, static fn() => AgentCat::publishCustomEvent(new \stdClass(), 'proj_x')); + + $this->assertSame('Server is not tracked. Please call agentcat.track() first or provide a session ID string.', $error->getMessage()); + } + public function testShutdownGoesThroughTheClient(): void { AgentCat::setClient($this->client); diff --git a/tests/Unit/CustomEventTest.php b/tests/Unit/CustomEventTest.php new file mode 100644 index 0000000..f0ec440 --- /dev/null +++ b/tests/Unit/CustomEventTest.php @@ -0,0 +1,60 @@ +assertNull($event->sessionId); + $this->assertNull($event->resourceName); + $this->assertNull($event->parameters); + $this->assertNull($event->response); + $this->assertNull($event->message); + $this->assertNull($event->duration); + $this->assertNull($event->isError); + $this->assertNull($event->error); + $this->assertNull($event->tags); + $this->assertNull($event->properties); + } + + public function testNamedArgumentsAreKept(): void + { + $error = (object) ['message' => 'Custom error occurred', 'code' => 'ERR_001']; + $event = new CustomEvent( + sessionId: 'ses_1', + resourceName: 'checkout', + parameters: ['cart_id' => 'c_42'], + response: ['ok' => true], + message: 'Checkout completed', + duration: 12.5, + isError: true, + error: $error, + tags: ['env' => 'prod'], + properties: ['cart_size' => 3], + ); + + $this->assertSame('ses_1', $event->sessionId); + $this->assertSame('checkout', $event->resourceName); + $this->assertSame(['cart_id' => 'c_42'], $event->parameters); + $this->assertSame(['ok' => true], $event->response); + $this->assertSame('Checkout completed', $event->message); + $this->assertEqualsWithDelta(12.5, $event->duration, PHP_FLOAT_EPSILON); + $this->assertTrue($event->isError); + $this->assertSame($error, $event->error); + $this->assertSame(['env' => 'prod'], $event->tags); + $this->assertSame(['cart_size' => 3], $event->properties); + } + + public function testClassIsReadonly(): void + { + $this->assertTrue((new \ReflectionClass(CustomEvent::class))->isReadOnly()); + } +} diff --git a/tests/Unit/Internal/Engine/PublisherTest.php b/tests/Unit/Internal/Engine/PublisherTest.php index 39a29a9..db33494 100644 --- a/tests/Unit/Internal/Engine/PublisherTest.php +++ b/tests/Unit/Internal/Engine/PublisherTest.php @@ -5,9 +5,11 @@ namespace AgentCat\Tests\Unit\Internal\Engine; use AgentCat\Event; +use AgentCat\Internal\Constants; use AgentCat\Internal\Delivery\EventQueue; use AgentCat\Internal\Engine\Publisher; use AgentCat\Internal\Engine\TrackedServer; +use AgentCat\Internal\SdkVersion; use AgentCat\Options; use AgentCat\Tests\Support\RecordingExporter; use AgentCat\Tests\Support\RecordingTransport; @@ -105,6 +107,86 @@ public function testTracingOffQueuesNothing(): void $this->assertSame(0, $this->queue->stats()['queueLength']); } + #[Group('ac-01-77')] + #[Group('ac-01-54')] + #[Group('ac-05-P7')] + public function testPublishCustomStampsTheTrackedServerAndRunsOnlyRedactEvent(): void + { + $redacted = []; + $options = new Options( + redactSensitiveInformation: static fn(string $s): string => '[R]', + redactEvent: static function (Event $event) use (&$redacted): Event { + $redacted[] = $event->eventType; + + return $event; + }, + ); + $server = new TrackedServer('proj_t', $options, static fn(): array => ['srv', '2'], $this->publisher); + $event = new Event(eventType: Constants::CUSTOM_EVENT_TYPE, timestamp: $this->at('2026-10-17T12:00:00.125Z'), clientName: 'stale', clientVersion: 'stale', parameters: (object) ['msg' => 'secret']); + + $this->publisher->publishCustom($server, $event); + $this->queue->flush(); + + $exported = $this->exported(); + $this->assertSame(['srv', '2', null, null, 'PHP', SdkVersion::get()], [ + $exported->serverName, + $exported->serverVersion, + $exported->clientName, + $exported->clientVersion, + $exported->sdkLanguage, + $exported->agentcatVersion, + ]); + $this->assertEquals(new \stdClass(), $exported->identifyActorData); + $this->assertSame('proj_t', $exported->projectId); + $this->assertSame(375, $exported->duration, 'the elapsed time since the event was made'); + $this->assertSame([Constants::CUSTOM_EVENT_TYPE], $redacted); + $this->assertEquals((object) ['msg' => 'secret'], $exported->parameters, 'D08: redactSensitiveInformation skips custom events'); + $this->assertCount(1, $this->transport->deliveries); + } + + public function testPublishCustomKeepsAGivenDurationAndReplacesZero(): void + { + $server = $this->server('proj_t'); + foreach ([42, 0, 0.0, 12.5] as $duration) { + $this->publisher->publishCustom($server, new Event(eventType: Constants::CUSTOM_EVENT_TYPE, timestamp: $this->at('2026-10-17T12:00:00.250Z'), duration: $duration)); + } + $this->queue->flush(); + + $this->assertSame([42, 250, 250, 12.5], array_map(static fn(Event $e): int|float|null => $e->duration, $this->exporter->events)); + } + + #[Group('ac-01-78')] + public function testPublishCustomOnATelemetryOnlyServerExportsWithoutDelivery(): void + { + $event = new Event(projectId: 'proj_other', eventType: Constants::CUSTOM_EVENT_TYPE); + + $this->publisher->publishCustom($this->server(''), $event); + $this->queue->flush(); + + $this->assertNull($this->exported()->projectId); + $this->assertSame([], $this->transport->deliveries); + } + + #[Group('ac-01-78')] + public function testPublishCustomUsesTheTrackedProject(): void + { + $this->publisher->publishCustom($this->server('proj_t'), new Event(projectId: 'proj_other', eventType: Constants::CUSTOM_EVENT_TYPE)); + $this->queue->flush(); + + $this->assertSame('proj_t', $this->exported()->projectId); + } + + public function testPublishCustomWithTracingOffQueuesNothing(): void + { + $event = new Event(projectId: 'proj_other', eventType: Constants::CUSTOM_EVENT_TYPE); + + $this->publisher->publishCustom($this->server('proj_t', new Options(enableTracing: false)), $event); + + $this->assertSame(0, $this->queue->stats()['queueLength']); + $this->assertSame('proj_other', $event->projectId, 'the event is left as it was'); + $this->assertNull($event->sdkLanguage); + } + public function testElapsedMsCountsWholeMilliseconds(): void { $this->assertSame(250, Publisher::elapsedMs($this->at('2026-10-17T12:00:00.250999Z'), $this->at('2026-10-17T12:00:00.500001Z'))); From 059b759123c9bc055c35cf62a65c89411e34345a Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 14:51:32 +0000 Subject: [PATCH 02/17] fix: log instead of throwing when publishCustomEvent cannot find a server after a failed track A track() that AgentCat could not complete (mcp/sdk drift, an unsupported shape, an internal failure) or an unreadable built server now marks the process. publishCustomEvent() then logs one warning per call for a server it cannot find instead of throwing into the customer's code; without such a failure it still throws like TypeScript. The drift cases that need ReflectionClass move to the contract tests, and the PHPDoc says how a coercive caller's scalar first argument is read. --- src/AgentCat.php | 8 ++- src/Client.php | 20 ++++-- src/Internal/Engine/TrackedServers.php | 26 +++++++- src/McpSdk/McpSdkAdapter.php | 13 ++-- tests/Contract/McpSdk/ServerMembersTest.php | 44 ++++++++++++- tests/Integration/ClientTrackTest.php | 4 ++ tests/Integration/PublishCustomEventTest.php | 63 +++++++++++++------ tests/Unit/AgentCatTest.php | 3 + .../Internal/Engine/TrackedServersTest.php | 23 +++++++ 9 files changed, 171 insertions(+), 33 deletions(-) diff --git a/src/AgentCat.php b/src/AgentCat.php index 5b833cb..675abd1 100644 --- a/src/AgentCat.php +++ b/src/AgentCat.php @@ -55,8 +55,12 @@ public static function track(object $server, ?string $projectId = null, ?Options * wins over it. No server or SDK fields and no hooks apply. * * The event is queued, not sent: AgentCat delivers it like any other event. Apart from the two errors below, it - * never throws: a failure is logged. In test environments it validates `$projectId` and then does nothing, like - * `track()` (see `AGENTCAT_ENABLED` there). + * never throws: a failure is logged. When a `track()` in this process could not track a server, a server that is + * not tracked logs a warning instead of the second error. In test environments it validates `$projectId` and then + * does nothing, like `track()` (see `AGENTCAT_ENABLED` there). + * + * In a file without `declare(strict_types=1)`, PHP turns an int, float or bool first argument into a session ID + * string. * * @param object|string $serverOrSessionId A tracked server, or the session ID of the event. * @param string|null $projectId Your AgentCat project ID (`proj_…`); required, even for a tracked server. diff --git a/src/Client.php b/src/Client.php index 25466e6..a67d9f7 100644 --- a/src/Client.php +++ b/src/Client.php @@ -122,6 +122,8 @@ public function track(object $server, ?string $projectId = null, ?Options $optio return $server; } catch (\Throwable $e) { + // Every failure ends here, after the shape or compatibility line where there is one. + TrackedServers::markIncomplete(); Log::write('Warning: Failed to track server - ' . ErrorText::of($e)); return $server; @@ -141,8 +143,12 @@ public function track(object $server, ?string $projectId = null, ?Options $optio * wins over it. No server or SDK fields and no hooks apply. * * The event is queued, not sent: AgentCat delivers it like any other event. Apart from the two errors below, it - * never throws: a failure is logged. In test environments it validates `$projectId` and then does nothing, like - * `track()` (see `AGENTCAT_ENABLED` there). + * never throws: a failure is logged. When a `track()` in this process could not track a server, a server that is + * not tracked logs a warning instead of the second error. In test environments it validates `$projectId` and then + * does nothing, like `track()` (see `AGENTCAT_ENABLED` there). + * + * In a file without `declare(strict_types=1)`, PHP turns an int, float or bool first argument into a session ID + * string. * * @param object|string $serverOrSessionId A tracked server, or the session ID of the event. * @param string|null $projectId Your AgentCat project ID (`proj_…`); required, even for a tracked server. @@ -165,10 +171,14 @@ public function publishCustomEvent(object|string $serverOrSessionId, ?string $pr $sessionId = $sessionId !== '' ? $sessionId : $serverOrSessionId; // verbatim: no derivation, '' is sessionless } else { $server = TrackedServers::get($serverOrSessionId) ?? McpSdkAdapter::trackedServer($serverOrSessionId); - if ($server === false) { - return; // mcp/sdk internals changed: the adapter logged the compatibility line - } if (!$server instanceof TrackedServer) { + if (TrackedServers::incomplete()) { + // AgentCat's own failure, logged before: it must not throw into the customer's code + Log::write('Warning: Failed to publish custom event - the server is not tracked (see the earlier AgentCat log line)'); + + return; + } + throw new \InvalidArgumentException('Server is not tracked. Please call agentcat.track() first or provide a session ID string.'); } if ($sessionId === '') { diff --git a/src/Internal/Engine/TrackedServers.php b/src/Internal/Engine/TrackedServers.php index be3d074..ee1db45 100644 --- a/src/Internal/Engine/TrackedServers.php +++ b/src/Internal/Engine/TrackedServers.php @@ -6,7 +6,8 @@ /** * Every server tracked in this process, whichever `Client` tracked it. The first `track()` of a server owns it, and - * a later one from any client changes nothing. An entry goes away with its server. + * a later one from any client changes nothing. An entry goes away with its server. A process-wide flag also records + * that a server could not be tracked or read. * * @internal */ @@ -15,6 +16,8 @@ final class TrackedServers /** @var \WeakMap|null */ private static ?\WeakMap $servers = null; + private static bool $incomplete = false; + /** The tracking state of the server, or null when no client tracks it. */ public static function get(object $server): ?TrackedServer { @@ -26,6 +29,27 @@ public static function add(object $server, TrackedServer $state): void self::servers()[$server] = $state; } + /** + * Records that AgentCat could not track a server, or could not read a built one, in this process. It logged why. + * A server it cannot find afterwards may be that one: `publishCustomEvent()` then logs instead of throwing. + */ + public static function markIncomplete(): void + { + self::$incomplete = true; + } + + /** True once `markIncomplete()` ran in this process. */ + public static function incomplete(): bool + { + return self::$incomplete; + } + + /** Tests only: forgets `markIncomplete()`. The tracked servers stay. */ + public static function resetIncomplete(): void + { + self::$incomplete = false; + } + /** @return \WeakMap */ private static function servers(): \WeakMap { diff --git a/src/McpSdk/McpSdkAdapter.php b/src/McpSdk/McpSdkAdapter.php index 677a99a..2b595b6 100644 --- a/src/McpSdk/McpSdkAdapter.php +++ b/src/McpSdk/McpSdkAdapter.php @@ -139,11 +139,13 @@ public static function serverInfo(Builder $builder): \Closure * The tracking state of a server built from a tracked Builder (01 AC-69): AgentCat's `tools/call` handler, among * the server's request handlers, holds it. A state found is kept for the server. Never throws. * - * @return TrackedServer|false|null null when the object is not an mcp/sdk server or its Builder was not tracked; - * false when its request handlers cannot be read (C6), after the compatibility - * line, which is logged once per server + * When the request handlers cannot be read (C6), the compatibility line is logged once per server, and + * `TrackedServers::incomplete()` turns true. + * + * @return TrackedServer|null null when the object is not an mcp/sdk server, its Builder was not tracked, or its + * request handlers cannot be read */ - public static function trackedServer(object $server): TrackedServer|false|null + public static function trackedServer(object $server): ?TrackedServer { if (!$server instanceof Server) { return null; @@ -152,8 +154,9 @@ public static function trackedServer(object $server): TrackedServer|false|null $handlers = StrictErrors::run(static fn(): array => ServerAccess::requestHandlers(ServerAccess::protocol($server))); } catch (\Throwable) { Drift::report($server); + TrackedServers::markIncomplete(); - return false; + return null; } foreach ($handlers as $handler) { if ($handler instanceof CallToolInterceptor) { diff --git a/tests/Contract/McpSdk/ServerMembersTest.php b/tests/Contract/McpSdk/ServerMembersTest.php index b1cfa98..6aedc51 100644 --- a/tests/Contract/McpSdk/ServerMembersTest.php +++ b/tests/Contract/McpSdk/ServerMembersTest.php @@ -4,6 +4,10 @@ namespace AgentCat\Tests\Contract\McpSdk; +use AgentCat\Client; +use AgentCat\CustomEvent; +use AgentCat\Event; +use AgentCat\Internal\ClientSeams; use AgentCat\Internal\Engine\TrackedServer; use AgentCat\Internal\Engine\TrackedServers; use AgentCat\Internal\Logging\Log; @@ -11,6 +15,7 @@ use AgentCat\McpSdk\Internal\Access\ServerAccess; use AgentCat\McpSdk\Internal\CallToolInterceptor; use AgentCat\McpSdk\McpSdkAdapter; +use AgentCat\Options; use AgentCat\Tests\Support\McpSdk\TrackingFixture; use AgentCat\Tests\Support\Thrown; use Mcp\Server; @@ -33,11 +38,13 @@ final class ServerMembersTest extends TestCase protected function setUp(): void { $this->fixture = new TrackingFixture(); + TrackedServers::resetIncomplete(); } #[\Override] protected function tearDown(): void { + TrackedServers::resetIncomplete(); Log::reset(); } @@ -85,6 +92,7 @@ public function testTheAdapterFindsNoStateForAnUntrackedServerOrAnotherObject(): $this->assertNull(McpSdkAdapter::trackedServer(TrackingFixture::builder())); $this->assertNull(McpSdkAdapter::trackedServer(new \stdClass())); $this->assertSame([], $this->fixture->lines()); + $this->assertFalse(TrackedServers::incomplete(), 'an untracked server is no failure of AgentCat'); } public function testAnUnreadableProtocolThrows(): void @@ -113,11 +121,43 @@ public function testDriftIsReportedOncePerServer(): void ]; foreach ($servers as $server) { - $this->assertFalse(McpSdkAdapter::trackedServer($server)); - $this->assertFalse(McpSdkAdapter::trackedServer($server)); + $this->assertNull(McpSdkAdapter::trackedServer($server)); + $this->assertNull(McpSdkAdapter::trackedServer($server)); + $this->assertNull(TrackedServers::get($server)); } $line = ['level' => LogLevel::WARNING, 'message' => McpSdkAdapter::COMPATIBILITY_LINE]; $this->assertSame([$line, $line], $this->fixture->logger->records); + $this->assertTrue(TrackedServers::incomplete(), 'publishCustomEvent() then logs instead of throwing'); + } + + /** Ruling R3 and finding F1: `publishCustomEvent()` on such a server logs and queues nothing, and never throws. */ + public function testPublishingForAServerWhoseInternalsChangedLogsAndReturns(): void + { + $server = new Server((new \ReflectionClass(Protocol::class))->newInstanceWithoutConstructor()); + + $this->client()->publishCustomEvent($server, TrackingFixture::PROJECT, new CustomEvent(sessionId: 'ses_1')); + $this->client()->publishCustomEvent($server, TrackingFixture::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $this->assertSame(['queueLength' => 0, 'activeRequests' => 0], $this->fixture->queue->stats()); + $warning = ['level' => LogLevel::WARNING, 'message' => 'Warning: Failed to publish custom event - the server is not tracked (see the earlier AgentCat log line)']; + $this->assertSame([['level' => LogLevel::WARNING, 'message' => McpSdkAdapter::COMPATIBILITY_LINE], $warning, $warning], $this->fixture->logger->records); + } + + /** Ruling R3: the state kept for a server is used without reading the server again. */ + public function testTheStateKeptForAServerIsUsedFirst(): void + { + $server = new Server((new \ReflectionClass(Protocol::class))->newInstanceWithoutConstructor()); // unreadable + TrackedServers::add($server, new TrackedServer(TrackingFixture::PROJECT, new Options(), static fn(): array => ['kept', '1.0'], $this->fixture->publisher)); + + $this->client()->publishCustomEvent($server, TrackingFixture::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $this->assertSame(['kept'], array_map(static fn(Event $event): ?string => $event->serverName, $this->fixture->events())); + $this->assertSame(["Published custom event for session ses_1 with type 'agentcat:custom'"], $this->fixture->lines()); + } + + private function client(): Client + { + return new Client(ClientSeams::of($this->fixture->queue, static fn(): bool => true)); } } diff --git a/tests/Integration/ClientTrackTest.php b/tests/Integration/ClientTrackTest.php index e7cdc17..822b7f2 100644 --- a/tests/Integration/ClientTrackTest.php +++ b/tests/Integration/ClientTrackTest.php @@ -68,6 +68,7 @@ final class ClientTrackTest extends TestCase protected function setUp(): void { $this->environment = Environment::clear('AGENTCAT_PROJECT_ID', 'AGENTCAT_API_URL', 'MCPCAT_API_URL', 'AGENTCAT_ENABLED', 'APP_ENV'); + TrackedServers::resetIncomplete(); $this->logger = new MemoryLogger(); Log::setLogger($this->logger); $this->transport = new RecordingTransport(); @@ -79,6 +80,7 @@ protected function setUp(): void #[\Override] protected function tearDown(): void { + TrackedServers::resetIncomplete(); // a failed track() marks the process Log::reset(); $this->environment->restore(); } @@ -98,6 +100,7 @@ public function testTrackInstrumentsTheBuilderAndReturnsIt(): void 'AgentCat setup complete | project ' . self::PROJECT . ' | tracing=true context=true reportMissing=true exporters=0', ], $this->logger->messages()); $this->assertSame(['echo', GetMoreTools::NAME], $this->names($this->harness($builder)->listTools())); + $this->assertFalse(TrackedServers::incomplete()); } #[Group('ac-01-4')] @@ -162,6 +165,7 @@ public function testAnInternalFailureReturnsTheServerAndLogsIt(): void $this->assertSame($builder, $client->track($builder, self::PROJECT)); $this->assertSame(['Warning: Failed to track server - RuntimeException: boom'], $this->logger->messages()); + $this->assertTrue(TrackedServers::incomplete(), 'publishCustomEvent() then logs instead of throwing'); } #[Group('ac-01-3')] diff --git a/tests/Integration/PublishCustomEventTest.php b/tests/Integration/PublishCustomEventTest.php index 689935c..e6a0a21 100644 --- a/tests/Integration/PublishCustomEventTest.php +++ b/tests/Integration/PublishCustomEventTest.php @@ -19,14 +19,13 @@ use AgentCat\Options; use AgentCat\Tests\Support\Environment; use AgentCat\Tests\Support\JsonAssert; +use AgentCat\Tests\Support\McpSdk\BuilderMembers; use AgentCat\Tests\Support\McpSdk\TrackingFixture; use AgentCat\Tests\Support\MemoryLogger; use AgentCat\Tests\Support\RecordingExporter; use AgentCat\Tests\Support\RecordingTransport; use AgentCat\Tests\Support\Thrown; -use Mcp\Server; use Mcp\Server\Builder; -use Mcp\Server\Protocol; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; @@ -45,7 +44,7 @@ final class PublishCustomEventTest extends TestCase private const string PUBLISHED_WITHOUT_SESSION = "Published custom event without a session with type 'agentcat:custom'"; - private const string COMPATIBILITY = 'AgentCat SDK Compatibility: AgentCat supports mcp/sdk 0.8.1+ (pass the Mcp\Server\Builder to AgentCat::track() before build()).'; + private const string NOT_TRACKED_AFTER_FAILURE = 'Warning: Failed to publish custom event - the server is not tracked (see the earlier AgentCat log line)'; private Environment $environment; @@ -69,11 +68,13 @@ protected function setUp(): void $this->exporter = new RecordingExporter(); $this->queue = new EventQueue(transport: $this->transport, exporters: $this->exporter); $this->client = new Client(ClientSeams::of($this->queue, static fn(): bool => true)); + TrackedServers::resetIncomplete(); // no failed track() before this test } #[\Override] protected function tearDown(): void { + TrackedServers::resetIncomplete(); Log::reset(); $this->environment->restore(); } @@ -452,29 +453,55 @@ public function testTheServerBuiltFromATrackedBuilderIsAccepted(): void $this->assertSame(TrackedServers::get($builder), TrackedServers::get($server), 'kept for the server'); } - /** Ruling R3: the state kept for a server is used without reading the server again. */ - public function testTheStateKeptForAServerIsUsedFirst(): void + /** @return iterable */ + public static function failedTracks(): iterable { - $server = new Server((new \ReflectionClass(Protocol::class))->newInstanceWithoutConstructor()); // unreadable - TrackedServers::add($server, new TrackedServer(self::PROJECT, new Options(), static fn(): array => ['kept', '1.0'], new Publisher($this->queue))); + yield 'mcp/sdk internals changed' => [static function (Client $client, Builder $builder): void { + BuilderMembers::uninitialize('referenceHandler')($builder); + $client->track($builder, self::PROJECT); + }]; + yield 'an internal failure' => [static function (Client $client, Builder $builder): void { + $failing = new Client(ClientSeams::of(new EventQueue(transport: new RecordingTransport()), static fn(): bool => throw new \RuntimeException('boom'))); + $failing->track($builder, self::PROJECT); + }]; + yield 'another object of an unsupported shape' => [static function (Client $client): void { + $client->track(new \stdClass(), self::PROJECT); + }]; + } + + /** + * Finding F1: after a track() that AgentCat could not complete (it logged why), a server it cannot find logs a + * warning instead of throwing into the customer's code, for example after an mcp/sdk upgrade. + * + * @param \Closure(Client, Builder): void $fail + */ + #[DataProvider('failedTracks')] + public function testAfterAFailedTrackAnUntrackedServerLogsInsteadOfThrowing(\Closure $fail): void + { + $builder = TrackingFixture::builder(); + $fail($this->client, $builder); + $this->assertCount(1, array_filter($this->logger->messages(), static fn(string $line): bool => str_starts_with($line, 'Warning: Failed to track server - '))); + $this->logger->records = []; - $this->client->publishCustomEvent($server, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + $this->client->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + $this->client->publishCustomEvent($builder->build(), self::PROJECT, new CustomEvent(sessionId: 'ses_1')); - $this->assertSame('kept', JsonAssert::at($this->body(), 'server_name')); - $this->assertSame(["Published custom event for session ses_1 with type 'agentcat:custom'"], $this->logger->messages()); + $this->assertNothingQueued(); + $line = ['level' => LogLevel::WARNING, 'message' => self::NOT_TRACKED_AFTER_FAILURE]; + $this->assertSame([$line, $line], $this->logger->records, 'one line per call'); + $error = Thrown::by(\InvalidArgumentException::class, fn() => $this->client->publishCustomEvent($builder, null)); + $this->assertSame(self::NO_PROJECT, $error->getMessage(), 'the project ID is still checked first'); } - /** Ruling R3: a server whose members AgentCat cannot read logs the compatibility line once, and nothing else. */ - public function testAServerWhoseInternalsChangedLogsTheCompatibilityLineOnce(): void + #[Group('ac-01-68')] + public function testAfterASuccessfulTrackAnUntrackedServerStillThrows(): void { - $server = new Server((new \ReflectionClass(Protocol::class))->newInstanceWithoutConstructor()); + $this->tracked(); - $this->client->publishCustomEvent($server, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); - $this->client->publishCustomEvent($server, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + $error = Thrown::by(\InvalidArgumentException::class, fn() => $this->client->publishCustomEvent(TrackingFixture::builder(), self::PROJECT)); - $this->assertNothingQueued(); - $this->assertSame([['level' => LogLevel::WARNING, 'message' => self::COMPATIBILITY]], $this->logger->records); - $this->assertNull(TrackedServers::get($server)); + $this->assertSame(self::NOT_TRACKED, $error->getMessage()); + $this->assertFalse(TrackedServers::incomplete()); } /** Tracks a fresh Builder for `harness-server` 1.2.3 and builds it, so its server info is known; clears the log. */ diff --git a/tests/Unit/AgentCatTest.php b/tests/Unit/AgentCatTest.php index b02f14b..8f04c3f 100644 --- a/tests/Unit/AgentCatTest.php +++ b/tests/Unit/AgentCatTest.php @@ -10,6 +10,7 @@ use AgentCat\Event; use AgentCat\Internal\ClientSeams; use AgentCat\Internal\Delivery\EventQueue; +use AgentCat\Internal\Engine\TrackedServers; use AgentCat\Internal\Json\Json; use AgentCat\Internal\Logging\Log; use AgentCat\Internal\Pipeline\QueuedEvent; @@ -43,11 +44,13 @@ protected function setUp(): void $this->transport = new RecordingTransport(); $this->queue = new EventQueue(transport: $this->transport); $this->client = new Client(ClientSeams::of($this->queue, static fn(): bool => true)); + TrackedServers::resetIncomplete(); } #[\Override] protected function tearDown(): void { + TrackedServers::resetIncomplete(); // a failed track() marks the process AgentCat::setClient(null); EventQueue::setGlobal(null); // N9: the default client of setClient(null) builds the global queue Log::reset(); diff --git a/tests/Unit/Internal/Engine/TrackedServersTest.php b/tests/Unit/Internal/Engine/TrackedServersTest.php index f686b2b..0bc4088 100644 --- a/tests/Unit/Internal/Engine/TrackedServersTest.php +++ b/tests/Unit/Internal/Engine/TrackedServersTest.php @@ -14,6 +14,12 @@ final class TrackedServersTest extends TestCase { + #[\Override] + protected function tearDown(): void + { + TrackedServers::resetIncomplete(); + } + public function testAServerHasTheStateItWasAddedWith(): void { $server = new \stdClass(); @@ -38,6 +44,23 @@ public function testAnEntryGoesAwayWithItsServer(): void $this->assertNull($gone->get()); } + public function testTheIncompleteFlagIsProcessWideUntilReset(): void + { + $server = new \stdClass(); + $state = $this->state($server); + TrackedServers::add($server, $state); + TrackedServers::resetIncomplete(); + $this->assertFalse(TrackedServers::incomplete()); + + TrackedServers::markIncomplete(); + TrackedServers::markIncomplete(); + + $this->assertTrue(TrackedServers::incomplete()); + TrackedServers::resetIncomplete(); + $this->assertFalse(TrackedServers::incomplete()); + $this->assertSame($state, TrackedServers::get($server), 'the reset keeps the tracked servers'); + } + private function state(object $server): TrackedServer { return new TrackedServer('proj', new Options(), static fn(): array => [$server::class, null], new Publisher(new EventQueue(transport: new RecordingTransport()))); From 6885277c326defb1e85309abc8b44f85d93512d0 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 14:51:32 +0000 Subject: [PATCH 03/17] test: pin the closure marker and the NAN substitution of a custom event --- tests/Integration/PublishCustomEventTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/Integration/PublishCustomEventTest.php b/tests/Integration/PublishCustomEventTest.php index e6a0a21..e487c23 100644 --- a/tests/Integration/PublishCustomEventTest.php +++ b/tests/Integration/PublishCustomEventTest.php @@ -406,7 +406,12 @@ public function testAPayloadWithoutACycleIsDeliveredWithMarkers(): void $intent = JsonAssert::at($body, 'user_intent'); $this->assertIsString($intent); $this->assertStringContainsString("\u{FFFD}", $intent); - $this->assertSame('[Resource: stream]', JsonAssert::at($body, 'parameters', 'r')); + $this->assertEquals((object) ['f' => '[Function: ]', 'r' => '[Resource: stream]'], JsonAssert::at($body, 'parameters')); + $this->assertCount(1, $this->exporter->events); + $event = $this->exporter->events[0]; + $this->assertSame('[NaN]', $event->response, 'the pipeline replaces NAN'); + $this->assertObjectNotHasProperty('response', $body, 'a string response is not sent'); + $this->assertContains(sprintf('WARN: Event %s: response is not a JSON object or array and was not sent', $event->id), $this->logger->messages()); } public function testAFailureAfterValidationIsLoggedAndSwallowed(): void From b52356a93f361c2d27e807023d723c16307b2083 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 14:59:12 +0000 Subject: [PATCH 04/17] feat: add AgentCat::fake() for customer test suites --- composer-dependency-analyser.php | 2 + src/AgentCat.php | 19 ++ src/Testing/AgentCatFake.php | 121 +++++++++ src/Testing/Internal/InstantTransport.php | 56 +++++ src/Testing/Internal/RecordingSink.php | 25 ++ tests/Unit/Testing/AgentCatFakeTest.php | 233 ++++++++++++++++++ .../Testing/Internal/InstantTransportTest.php | 58 +++++ 7 files changed, 514 insertions(+) create mode 100644 src/Testing/AgentCatFake.php create mode 100644 src/Testing/Internal/InstantTransport.php create mode 100644 src/Testing/Internal/RecordingSink.php create mode 100644 tests/Unit/Testing/AgentCatFakeTest.php create mode 100644 tests/Unit/Testing/Internal/InstantTransportTest.php diff --git a/composer-dependency-analyser.php b/composer-dependency-analyser.php index 2722aa2..0037951 100644 --- a/composer-dependency-analyser.php +++ b/composer-dependency-analyser.php @@ -12,6 +12,8 @@ ->ignoreUnknownFunctionsRegex('~^AgentCat\\\\Tests\\\\Fixtures\\\\Errors\\\\~') // mcp/sdk is optional (01 AC-90): a dev dependency and a suggestion, used only by the adapter. ->ignoreErrorsOnPackageAndPath('mcp/sdk', __DIR__ . '/src/McpSdk', [ErrorType::DEV_DEPENDENCY_IN_PROD]) + // AgentCatFake asserts with PHPUnit only when the class is loaded (class_exists()); phpunit stays a dev dependency. + ->ignoreErrorsOnPackageAndPath('phpunit/phpunit', __DIR__ . '/src/Testing', [ErrorType::DEV_DEPENDENCY_IN_PROD]) // Psr\Container\ContainerInterface arrives with mcp/sdk; the adapter only passes it through. ->ignoreErrorsOnPackageAndPath('psr/container', __DIR__ . '/src/McpSdk', [ErrorType::SHADOW_DEPENDENCY]) // Psr\EventDispatcher\EventDispatcherInterface arrives with mcp/sdk; the test double stands in for a customer's dispatcher. diff --git a/src/AgentCat.php b/src/AgentCat.php index 675abd1..d521dfc 100644 --- a/src/AgentCat.php +++ b/src/AgentCat.php @@ -4,6 +4,8 @@ namespace AgentCat; +use AgentCat\Testing\AgentCatFake; + /** * The AgentCat entry point: `AgentCat::track($builder, 'proj_…')` before `$builder->build()`. * @@ -96,6 +98,23 @@ public static function shutdown(): void self::client()->shutdown(); } + /** + * Starts recording for your test suite: installs a client that records every event and sends nothing, and + * returns the fake to assert on. Restore the real client with `AgentCat::setClient(null)` in `tearDown()`. + * + * Call it before the server is tracked: a Builder tracked earlier keeps the client that tracked it. + * + * The fake records whatever `AGENTCAT_ENABLED` says, so a CI that sets `AGENTCAT_ENABLED=false` still works. Test + * environments without `fake()` stay silent: `track()` does nothing there. + */ + public static function fake(): AgentCatFake + { + $fake = new AgentCatFake(); + self::setClient($fake->client()); + + return $fake; + } + /** The shared client; the first call creates it. */ public static function client(): Client { diff --git a/src/Testing/AgentCatFake.php b/src/Testing/AgentCatFake.php new file mode 100644 index 0000000..7322fc5 --- /dev/null +++ b/src/Testing/AgentCatFake.php @@ -0,0 +1,121 @@ +sink = new RecordingSink(); + $this->client = new Client(ClientSeams::of( + new EventQueue(transport: new InstantTransport(), exporters: $this->sink), + static fn(): bool => true, + )); + } + + /** The client that records. `AgentCat::fake()` already installed it with `AgentCat::setClient()`. */ + public function client(): Client + { + return $this->client; + } + + /** + * Every recorded event, in publish order: hooks applied, redacted, truncated. Calling it runs the hooks that + * are still waiting. + * + * @return list + */ + public function events(): array + { + $this->client->flush(); + + return $this->sink->events; + } + + /** + * Asserts that at least one tool call (`mcp:tools/call`) was tracked, and one matches `$filter` when given. + * + * @param (\Closure(Event): bool)|null $filter + */ + public function assertTracked(?\Closure $filter = null): void + { + $this->check($this->matching($this->ofType(Constants::TOOLS_CALL_EVENT_TYPE), $filter) !== [], 'Failed asserting that a tool call matching the filter was tracked.'); + } + + /** Asserts that no event of any type was recorded. */ + public function assertNothingTracked(): void + { + $count = count($this->events()); + $this->check($count === 0, sprintf('Failed asserting that nothing was tracked: %d event(s) were recorded.', $count)); + } + + /** Asserts the number of tracked tool calls (`mcp:tools/call`); custom events do not count. */ + public function assertTrackedCount(int $count): void + { + $actual = count($this->ofType(Constants::TOOLS_CALL_EVENT_TYPE)); + $this->check($actual === $count, sprintf('Failed asserting that %d tool call(s) were tracked: %d were.', $count, $actual)); + } + + /** + * Asserts that at least one custom event (`agentcat:custom`) was published, and one matches `$filter` when given. + * + * @param (\Closure(Event): bool)|null $filter + */ + public function assertCustomEventPublished(?\Closure $filter = null): void + { + $this->check($this->matching($this->ofType(Constants::CUSTOM_EVENT_TYPE), $filter) !== [], 'Failed asserting that a custom event matching the filter was published.'); + } + + /** @return array */ + private function ofType(string $eventType): array + { + return array_filter($this->events(), static fn(Event $e): bool => $e->eventType === $eventType); + } + + /** + * @param array $events + * @param (\Closure(Event): bool)|null $filter + * + * @return array + */ + private function matching(array $events, ?\Closure $filter): array + { + return array_filter($events, static fn(Event $e): bool => !$filter instanceof \Closure || $filter($e)); + } + + private function check(bool $condition, string $message): void + { + if ($condition) { + return; + } + if (class_exists(Assert::class)) { + Assert::fail($message); // assertTrue() would append "Failed asserting that false is true." + } + + // PHPUnit is always loaded in our own suite; this is the path of a suite without it. + // @codeCoverageIgnoreStart + throw new \AssertionError($message); + // @codeCoverageIgnoreEnd + } +} diff --git a/src/Testing/Internal/InstantTransport.php b/src/Testing/Internal/InstantTransport.php new file mode 100644 index 0000000..ac146f8 --- /dev/null +++ b/src/Testing/Internal/InstantTransport.php @@ -0,0 +1,56 @@ +beforeAttempt instanceof \Closure) { + ($delivery->beforeAttempt)(); + } + } + + #[\Override] + public function pump(): void {} + + #[\Override] + public function waitForActivity(float $deadline): void {} + + #[\Override] + public function drain(float $deadline): bool + { + return true; + } + + #[\Override] + public function send(Delivery $delivery, float $deadline = INF): bool + { + $this->start($delivery); + + return true; + } + + #[\Override] + public function inFlight(): int + { + return 0; + } + + #[\Override] + public function now(): float + { + return hrtime(true) / 1e9; + } +} diff --git a/src/Testing/Internal/RecordingSink.php b/src/Testing/Internal/RecordingSink.php new file mode 100644 index 0000000..311dcb8 --- /dev/null +++ b/src/Testing/Internal/RecordingSink.php @@ -0,0 +1,25 @@ + */ + public array $events = []; + + #[\Override] + public function export(Event $event): void + { + $this->events[] = $event; + } +} diff --git a/tests/Unit/Testing/AgentCatFakeTest.php b/tests/Unit/Testing/AgentCatFakeTest.php new file mode 100644 index 0000000..fabef7b --- /dev/null +++ b/tests/Unit/Testing/AgentCatFakeTest.php @@ -0,0 +1,233 @@ +environment = Environment::clear('AGENTCAT_ENABLED', 'AGENTCAT_API_URL', 'MCPCAT_API_URL', 'AGENTCAT_PROJECT_ID'); + $this->logger = new MemoryLogger(); + Log::setLogger($this->logger); + } + + #[\Override] + protected function tearDown(): void + { + TrackedServers::resetIncomplete(); + AgentCat::setClient(null); + EventQueue::setGlobal(null); + Log::reset(); + $this->environment->restore(); + } + + #[Group('ac-01-69')] + public function testWithoutFakeTheSdkStaysOffInATestSuite(): void + { + $builder = TrackingFixture::builder(); + Tools::echo($builder); + + $this->assertSame($builder, AgentCat::track($builder, self::PROJECT)); + AgentCat::publishCustomEvent($builder, self::PROJECT, new CustomEvent(message: 'm')); + + $this->assertSame(['echo'], $this->toolNames($builder), 'no get_more_tools was injected'); + $this->assertSame(['queueLength' => 0, 'activeRequests' => 0], EventQueue::global()->stats()); + $this->assertFileDoesNotExist($this->home() . '/agentcat.log'); + } + + public function testFakeInstallsItsClient(): void + { + $fake = AgentCat::fake(); + + $this->assertSame($fake->client(), AgentCat::client()); + } + + public function testATrackedToolCallIsRecordedWithItsHooksApplied(): void + { + $fake = AgentCat::fake(); + $harness = $this->trackedHarness(new Options(identify: static fn(ToolCall $call): Identity => new Identity('u1'))); + + $harness->callTool('echo', ['msg' => 'hi']); + + $fake->assertTracked(static fn(Event $e): bool => $e->resourceName === 'echo' && $e->identifyActorGivenId === 'u1'); + $fake->assertTrackedCount(1); + $events = $fake->events(); + $this->assertCount(1, $events); + $this->assertMatchesRegularExpression('/\Aevt_[0-9A-Za-z]{27}\z/', $events[0]->id); + } + + public function testAssertTrackedWithoutAFilterNeedsOneToolCall(): void + { + $fake = AgentCat::fake(); + $message = $this->failureOf(static fn() => $fake->assertTracked()); + $this->assertSame('Failed asserting that a tool call matching the filter was tracked.', $message); + + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + $fake->assertTracked(); + } + + public function testAssertNothingTrackedPassesOnAnEmptyFakeAndFailsAfterACall(): void + { + $fake = AgentCat::fake(); + $fake->assertNothingTracked(); + + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + + $this->assertSame( + 'Failed asserting that nothing was tracked: 1 event(s) were recorded.', + $this->failureOf(static fn() => $fake->assertNothingTracked()), + ); + } + + public function testFailingAssertionsSayWhy(): void + { + $fake = AgentCat::fake(); + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + + $this->assertSame( + 'Failed asserting that a tool call matching the filter was tracked.', + $this->failureOf(static fn() => $fake->assertTracked(static fn(Event $e): bool => false)), + ); + $this->assertSame( + 'Failed asserting that 2 tool call(s) were tracked: 1 were.', + $this->failureOf(static fn() => $fake->assertTrackedCount(2)), + ); + $this->assertSame( + 'Failed asserting that a custom event matching the filter was published.', + $this->failureOf(static fn() => $fake->assertCustomEventPublished()), + ); + } + + public function testACustomEventIsNotAToolCall(): void + { + $fake = AgentCat::fake(); + + AgentCat::publishCustomEvent('s-1', self::PROJECT, new CustomEvent(resourceName: 'c')); + + $fake->assertCustomEventPublished(static fn(Event $e): bool => $e->resourceName === 'c'); + $fake->assertCustomEventPublished(); + $fake->assertTrackedCount(0); + $this->assertSame( + 'Failed asserting that a custom event matching the filter was published.', + $this->failureOf(static fn() => $fake->assertCustomEventPublished(static fn(Event $e): bool => $e->resourceName === 'other')), + ); + } + + #[Group('ac-01-69')] + public function testNothingGoesToTheNetwork(): void + { + putenv('AGENTCAT_API_URL=' . MockCollector::closedPortUrl()); + $fake = AgentCat::fake(); + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + + $fake->assertTrackedCount(1); + $this->assertSame([], array_filter($this->logger->messages(), static fn(string $m): bool => str_contains($m, 'Failed to send event'))); + putenv('AGENTCAT_API_URL'); + } + + public function testTelemetryOnlyServersAreRecordedToo(): void + { + $fake = AgentCat::fake(); + $builder = TrackingFixture::builder(); + Tools::echo($builder); + AgentCat::track($builder, ''); + $harness = new HttpHarness($builder->build()); + $harness->initialize(); + + $harness->callTool('echo', ['msg' => 'hi']); + + $fake->assertTrackedCount(1); + } + + #[Group('ac-01-69')] + public function testTheFakeRecordsWhateverAgentcatEnabledSays(): void + { + putenv('AGENTCAT_ENABLED=false'); + $fake = AgentCat::fake(); + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + + $fake->assertTrackedCount(1); + putenv('AGENTCAT_ENABLED'); + } + + public function testASecondFakeStartsEmpty(): void + { + $first = AgentCat::fake(); + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + $first->assertTrackedCount(1); + + $second = AgentCat::fake(); + + $second->assertNothingTracked(); + $this->assertNotSame($first->client(), $second->client()); + } + + private function trackedHarness(?Options $options = null): HttpHarness + { + $builder = TrackingFixture::builder(); + Tools::echo($builder); + AgentCat::track($builder, self::PROJECT, $options); + $harness = new HttpHarness($builder->build()); + $harness->initialize(); + + return $harness; + } + + /** @return list */ + private function toolNames(Builder $builder): array + { + $harness = new HttpHarness($builder->build()); + $harness->initialize(); + $tools = $harness->listTools()->tools ?? null; + $this->assertIsList($tools); + + return array_map(static fn(mixed $t): string => $t instanceof \stdClass && is_string($t->name ?? null) ? $t->name : '', $tools); + } + + /** @param \Closure(): mixed $code */ + private function failureOf(\Closure $code): string + { + $failure = Thrown::by(\Throwable::class, $code); + $this->assertStringEndsWith('AssertionFailedError', $failure::class); + + return $failure->getMessage(); + } + + private function home(): string + { + $home = getenv('HOME'); + $this->assertIsString($home); + + return $home; + } +} diff --git a/tests/Unit/Testing/Internal/InstantTransportTest.php b/tests/Unit/Testing/Internal/InstantTransportTest.php new file mode 100644 index 0000000..e2c0ea8 --- /dev/null +++ b/tests/Unit/Testing/Internal/InstantTransportTest.php @@ -0,0 +1,58 @@ +start(new Delivery('http://x.test', '{}', 'evt_1', 's', static function () use (&$count): void { + ++$count; + })); + $transport->pump(); + $transport->waitForActivity(INF); + + $this->assertSame(1, $count); + $this->assertSame(0, $transport->inFlight()); + } + + public function testStartWithoutAHookDoesNothing(): void + { + $transport = new InstantTransport(); + + $transport->start(new Delivery('http://x.test', '{}', 'evt_1', 's')); + + $this->assertSame(0, $transport->inFlight()); + } + + public function testDrainAndSendSucceedAtOnce(): void + { + $transport = new InstantTransport(); + $count = 0; + + $this->assertTrue($transport->drain(0.0)); + $this->assertTrue($transport->send(new Delivery('http://x.test', '{}', 'evt_1', 's', static function () use (&$count): void { + ++$count; + }))); + $this->assertSame(1, $count); + } + + public function testTheClockIsMonotonicSeconds(): void + { + $transport = new InstantTransport(); + $first = $transport->now(); + usleep(1000); + + $this->assertGreaterThan($first, $transport->now()); + $this->assertLessThan(1e7, $first, 'seconds, not nanoseconds'); + } +} From 953e26fb0d1c724836117d7c89d69e3a6c736aac Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 15:03:35 +0000 Subject: [PATCH 05/17] fix: count one PHPUnit assertion when a fake assertion passes --- src/Testing/AgentCatFake.php | 12 ++++++---- tests/Unit/Testing/AgentCatFakeTest.php | 30 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/Testing/AgentCatFake.php b/src/Testing/AgentCatFake.php index 7322fc5..afc5eff 100644 --- a/src/Testing/AgentCatFake.php +++ b/src/Testing/AgentCatFake.php @@ -106,16 +106,20 @@ private function matching(array $events, ?\Closure $filter): array private function check(bool $condition, string $message): void { - if ($condition) { - return; - } if (class_exists(Assert::class)) { + if ($condition) { + Assert::assertNotSame('', $message); // counts one assertion, so a test with only fake assertions is not risky + + return; + } Assert::fail($message); // assertTrue() would append "Failed asserting that false is true." } // PHPUnit is always loaded in our own suite; this is the path of a suite without it. // @codeCoverageIgnoreStart - throw new \AssertionError($message); + if (!$condition) { + throw new \AssertionError($message); + } // @codeCoverageIgnoreEnd } } diff --git a/tests/Unit/Testing/AgentCatFakeTest.php b/tests/Unit/Testing/AgentCatFakeTest.php index fabef7b..6818caa 100644 --- a/tests/Unit/Testing/AgentCatFakeTest.php +++ b/tests/Unit/Testing/AgentCatFakeTest.php @@ -21,6 +21,7 @@ use AgentCat\Tests\Support\Thrown; use AgentCat\ToolCall; use Mcp\Server\Builder; +use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; @@ -192,6 +193,35 @@ public function testASecondFakeStartsEmpty(): void $this->assertNotSame($first->client(), $second->client()); } + public function testEveryPassingAssertionCountsExactlyOnePhpunitAssertion(): void + { + $fake = AgentCat::fake(); + $fake->assertNothingTracked(); + AgentCat::publishCustomEvent('s-1', self::PROJECT, new CustomEvent(resourceName: 'c')); + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + $fake->events(); + + foreach ([ + 'assertTracked' => static fn() => $fake->assertTracked(), + 'assertTrackedCount' => static fn() => $fake->assertTrackedCount(1), + 'assertCustomEventPublished' => static fn() => $fake->assertCustomEventPublished(), + ] as $name => $assertion) { + $before = Assert::getCount(); + $assertion(); + $this->assertSame(1, Assert::getCount() - $before, $name); + } + } + + public function testAssertNothingTrackedCountsOnePhpunitAssertion(): void + { + $fake = AgentCat::fake(); + $before = Assert::getCount(); + + $fake->assertNothingTracked(); + + $this->assertSame(1, Assert::getCount() - $before); + } + private function trackedHarness(?Options $options = null): HttpHarness { $builder = TrackingFixture::builder(); From cc03b5c2dd31e7bf25cdfbc1b9e6d5a10ce9fbca Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 15:30:02 +0000 Subject: [PATCH 06/17] feat: drain the event queue on SIGTERM and SIGINT in CLI servers --- composer-dependency-analyser.php | 4 +- src/Internal/Delivery/EventQueue.php | 11 +- src/Internal/Delivery/SignalDrain.php | 101 +++++++ .../Delivery/SignalDrainDeliveryTest.php | 151 ++++++++++ tests/Integration/Fixtures/signal-drain.php | 39 +++ .../Internal/Delivery/SignalDrainTest.php | 260 ++++++++++++++++++ 6 files changed, 562 insertions(+), 4 deletions(-) create mode 100644 src/Internal/Delivery/SignalDrain.php create mode 100644 tests/Integration/Delivery/SignalDrainDeliveryTest.php create mode 100644 tests/Integration/Fixtures/signal-drain.php create mode 100644 tests/Unit/Internal/Delivery/SignalDrainTest.php diff --git a/composer-dependency-analyser.php b/composer-dependency-analyser.php index 0037951..5b046f2 100644 --- a/composer-dependency-analyser.php +++ b/composer-dependency-analyser.php @@ -6,8 +6,8 @@ use ShipMonk\ComposerDependencyAnalyser\Config\ErrorType; return (new Configuration()) - // ext-posix is optional: FileLogger guards its use with function_exists(). - ->ignoreErrorsOnExtensions(['ext-posix'], [ErrorType::SHADOW_DEPENDENCY]) + // ext-pcntl and ext-posix are optional: FileLogger and SignalDrain guard their use with function_exists(). + ->ignoreErrorsOnExtensions(['ext-pcntl', 'ext-posix'], [ErrorType::SHADOW_DEPENDENCY]) // Test fixture functions, loaded with require_once by ExceptionCaptureTest (not autoloadable). ->ignoreUnknownFunctionsRegex('~^AgentCat\\\\Tests\\\\Fixtures\\\\Errors\\\\~') // mcp/sdk is optional (01 AC-90): a dev dependency and a suggestion, used only by the adapter. diff --git a/src/Internal/Delivery/EventQueue.php b/src/Internal/Delivery/EventQueue.php index 889135b..8640e0e 100644 --- a/src/Internal/Delivery/EventQueue.php +++ b/src/Internal/Delivery/EventQueue.php @@ -66,12 +66,19 @@ public function __construct( $this->endpoint = ApiBaseUrl::endpoint(ApiBaseUrl::resolve()); // environment or default, until configure() } - /** The queue of this process; on first use it registers the shutdown delivery for the first event. */ + /** + * The queue of this process; its first event registers the shutdown delivery and installs the signal drain + * (CLI servers with ext-pcntl). + */ public static function global(): self { if (!self::$global instanceof self) { $flusher = new ShutdownFlusher(static fn(): self => self::global(), null, FinishRequest::detect()); - self::$global = new self(onEnqueue: $flusher->ensureRegistered(...)); + $signals = new SignalDrain(static fn(): self => self::global()); + self::$global = new self(onEnqueue: static function () use ($flusher, $signals): void { + $flusher->ensureRegistered(); + $signals->install(); + }); } return self::$global; diff --git a/src/Internal/Delivery/SignalDrain.php b/src/Internal/Delivery/SignalDrain.php new file mode 100644 index 0000000..271ee64 --- /dev/null +++ b/src/Internal/Delivery/SignalDrain.php @@ -0,0 +1,101 @@ +raise = $raise ?? $this->raiseWithDefaultDisposition(...); + $this->functionExists = $functionExists ?? function_exists(...); + } + + /** Installs the handlers once. Does nothing outside the CLI SAPI or without ext-pcntl. */ + public function install(): void + { + if ($this->installed || $this->sapi !== 'cli') { + return; + } + foreach (['pcntl_signal', 'pcntl_signal_get_handler', 'pcntl_async_signals'] as $function) { + if (!($this->functionExists)($function)) { + return; + } + } + $this->installed = true; + foreach ([\SIGTERM, \SIGINT] as $signal) { + $previous = pcntl_signal_get_handler($signal); + if ($previous === \SIG_IGN) { + continue; // the application ignores this signal: so do we + } + pcntl_signal($signal, function (int $signo) use ($previous): void { + $this->onSignal($signo, $previous); + }); + } + pcntl_async_signals(true); // the stdio loop sleeps in usleep(): handlers must run without pcntl_signal_dispatch() + } + + /** + * Shuts the queue down, then calls the earlier handler (`03` AC-11: the process keeps running) or raises the + * signal again (`03` AC-10). + * + * @param mixed $previous the handler before ours: a callable, or `SIG_DFL` + */ + public function onSignal(int $signal, mixed $previous): void + { + try { + ($this->queue)()->finalShutdown(); // at most EventQueue::SHUTDOWN_TIMEOUT_SECONDS + } catch (\Throwable) { + // a signal handler must never throw + } + if (is_callable($previous)) { + $previous($signal, null); + + return; + } + ($this->raise)($signal); + } + + private function raiseWithDefaultDisposition(int $signal): void + { + pcntl_signal($signal, \SIG_DFL); + if (($this->functionExists)('posix_kill')) { + posix_kill(posix_getpid(), $signal); // the default disposition of SIGTERM and SIGINT ends the process here + + return; + } + exit(128 + $signal); // the shell's exit status for "ended by this signal" + } +} diff --git a/tests/Integration/Delivery/SignalDrainDeliveryTest.php b/tests/Integration/Delivery/SignalDrainDeliveryTest.php new file mode 100644 index 0000000..9472ba4 --- /dev/null +++ b/tests/Integration/Delivery/SignalDrainDeliveryTest.php @@ -0,0 +1,151 @@ +stop(); + self::$collector = null; + } + + #[\Override] + protected function setUp(): void + { + $this->dir = sys_get_temp_dir() . '/agentcat-signal-' . bin2hex(random_bytes(6)); + $this->assertTrue(mkdir($this->dir, 0o700)); + } + + #[\Override] + protected function tearDown(): void + { + if (!is_dir($this->dir)) { + return; + } + $names = scandir($this->dir); + foreach ($names === false ? [] : $names as $name) { + if (is_file($this->dir . '/' . $name)) { + unlink($this->dir . '/' . $name); + } + } + rmdir($this->dir); + } + + #[Group('ac-03-10')] + #[MaximumDuration(10000)] + public function testWithoutAHandlerTheQueueDrainsThenSigtermEndsTheProcess(): void + { + $base = $this->collector()->scenario(); + $process = $this->fixture($base, 'default'); + + $signaled = null; + $started = microtime(true); + + try { + $process->run(); + } catch (ProcessSignaledException $e) { + $signaled = $e; + } + $elapsed = microtime(true) - $started; + + $this->assertInstanceOf(ProcessSignaledException::class, $signaled, sprintf('exit code %s: %s', var_export($process->getExitCode(), true), $process->getErrorOutput())); + $this->assertTrue($process->hasBeenSignaled()); + $this->assertSame(15, $process->getTermSignal()); + $this->assertLessThan(self::SIGNAL_DEADLINE, $elapsed); + $this->assertSame('', $process->getOutput()); + $this->assertSame('', $process->getErrorOutput()); + $this->assertSame(['signal-drain'], $this->resourceNames($base)); + } + + #[Group('ac-03-11')] + #[MaximumDuration(10000)] + public function testWithACustomerHandlerTheQueueDrainsThenTheHandlerRunsAndTheProcessGoesOn(): void + { + $base = $this->collector()->scenario(); + $marker = $this->dir . '/marker'; + $process = $this->fixture($base, 'chain', $marker); + + $process->run(); + + $this->assertSame(0, $process->getExitCode(), $process->getErrorOutput()); + $this->assertSame("done\n", $process->getOutput()); + $this->assertSame('', $process->getErrorOutput()); + $this->assertSame(['signal-drain'], $this->resourceNames($base)); + $this->assertFileExists($marker, 'the customer handler ran'); + $contents = file_get_contents($marker); + $this->assertIsString($contents); + $lines = explode("\n", $contents); + $this->assertCount(2, $lines); + [$who, $time] = $lines; + $this->assertSame('customer', $who); + $this->assertIsNumeric($time); + $requests = $this->collector()->requests($base); + $this->assertCount(1, $requests); + $start = $requests[0]->start; + $this->assertIsNumeric($start); + $this->assertGreaterThan((float) $start, (float) $time, 'the queue drained before the customer handler ran'); + } + + private function fixture(string $base, string $mode, string $marker = ''): Process + { + $process = new Process( + [PHP_BINARY, __DIR__ . '/../Fixtures/signal-drain.php', $base, $mode, $marker], + null, + ['HOME' => $this->dir], + ); + $process->setTimeout(10); + + return $process; + } + + /** @return list the `resource_name` of each request the collector got, in order */ + private function resourceNames(string $base): array + { + $names = []; + foreach ($this->collector()->requests($base) as $request) { + $body = $request->body; + $this->assertIsString($body); + $names[] = JsonAssert::at(Json::decode($body), 'resource_name'); + } + + return $names; + } + + private function collector(): MockCollector + { + $this->assertInstanceOf(MockCollector::class, self::$collector); + + return self::$collector; + } +} diff --git a/tests/Integration/Fixtures/signal-drain.php b/tests/Integration/Fixtures/signal-drain.php new file mode 100644 index 0000000..8a674af --- /dev/null +++ b/tests/Integration/Fixtures/signal-drain.php @@ -0,0 +1,39 @@ + +// default: no SIGTERM handler of its own. AgentCat delivers the event, then SIGTERM ends the process. +// chain: its own SIGTERM handler first. AgentCat delivers the event, then calls that handler, which writes +// "customer" and the time to ; the script goes on and prints "done". +$baseUrl = $argv[1] ?? ''; +$mode = $argv[2] ?? ''; +$markerFile = $argv[3] ?? ''; +if ($mode === 'chain') { + pcntl_signal(SIGTERM, static function () use ($markerFile): void { + file_put_contents($markerFile, sprintf("customer\n%.6F", microtime(true))); + }); +} +$queue = EventQueue::global(); +$queue->configure($baseUrl); +$queue->add(new QueuedEvent(new Event(projectId: 'proj_test', eventType: 'mcp:tools/call', resourceName: 'signal-drain'))); +posix_kill(posix_getpid(), SIGTERM); +if ($mode === 'chain') { + usleep(200_000); + echo "done\n"; + + exit(0); +} +// A server loop that sleeps: SIGTERM must end it long before the 5 s are over. +for ($step = 0; $step < 100; ++$step) { + usleep(50_000); +} diff --git a/tests/Unit/Internal/Delivery/SignalDrainTest.php b/tests/Unit/Internal/Delivery/SignalDrainTest.php new file mode 100644 index 0000000..ebe9190 --- /dev/null +++ b/tests/Unit/Internal/Delivery/SignalDrainTest.php @@ -0,0 +1,260 @@ + each `raise` call, with the deliveries made before it */ + private array $raised = []; + + /** @var array the handlers before the test */ + private array $savedHandlers = []; + + private ?bool $savedAsyncSignals = null; + + #[\Override] + protected function setUp(): void + { + $this->logger = new MemoryLogger(); + Log::setLogger($this->logger); + $this->transport = new RecordingTransport(); + $this->queue = new EventQueue(transport: $this->transport); + if (function_exists('pcntl_signal_get_handler')) { + foreach ([\SIGTERM, \SIGINT, \SIGWINCH] as $signal) { + $this->savedHandlers[$signal] = pcntl_signal_get_handler($signal); + } + $this->savedAsyncSignals = pcntl_async_signals(); + } + } + + #[\Override] + protected function tearDown(): void + { + foreach ($this->savedHandlers as $signal => $handler) { + pcntl_signal($signal, $handler); + } + if ($this->savedAsyncSignals !== null) { + pcntl_async_signals($this->savedAsyncSignals); + } + Log::reset(); + } + + #[Group('ac-03-10')] + public function testWithoutAnEarlierHandlerTheQueueDrainsThenTheSignalIsRaised(): void + { + $this->queue->add($this->queued('waiting')); + + $this->drain()->onSignal(self::SIGTERM, self::SIG_DFL); + + $this->assertCount(1, $this->transport->deliveries, 'the waiting event was delivered'); + $this->assertSame([['signal' => self::SIGTERM, 'delivered' => 1]], $this->raised, 'raised once, after the drain'); + $this->queue->add($this->queued('late')); + $this->assertSame(['Queue is shutting down, event dropped'], $this->logger->messages()); + } + + #[Group('ac-03-11')] + public function testAnEarlierHandlerIsCalledAfterTheDrainAndNothingIsRaised(): void + { + $this->queue->add($this->queued('waiting')); + $calls = []; + $customer = function (mixed ...$arguments) use (&$calls): void { + $calls[] = ['arguments' => $arguments, 'delivered' => count($this->transport->deliveries)]; + }; + + $this->drain()->onSignal(self::SIGTERM, $customer); + + $this->assertSame([['arguments' => [self::SIGTERM, null], 'delivered' => 1]], $calls, 'called once, after the drain'); + $this->assertSame([], $this->raised, 'the earlier handler owns termination'); + } + + public function testASignalHandlerNeverThrows(): void + { + $drain = new SignalDrain( + static fn(): EventQueue => throw new \RuntimeException('no queue'), + $this->raise(...), + static fn(string $function): bool => true, + ); + + $drain->onSignal(self::SIGTERM, self::SIG_DFL); + + $this->assertSame([['signal' => self::SIGTERM, 'delivered' => 0]], $this->raised); + } + + #[RequiresPhpExtension('pcntl')] + public function testOutsideTheCliSapiNothingIsInstalled(): void + { + $this->resetHandlers(); + + $this->drain(sapi: 'fpm-fcgi')->install(); + + $this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGTERM)); + $this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGINT)); + $this->assertFalse(pcntl_async_signals()); + } + + #[RequiresPhpExtension('pcntl')] + #[DataProvider('pcntlFunctions')] + public function testWithoutAPcntlFunctionNothingIsInstalled(string $missing): void + { + $this->resetHandlers(); + + $this->drain(static fn(string $function): bool => $function !== $missing)->install(); + + $this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGTERM)); + $this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGINT)); + $this->assertFalse(pcntl_async_signals()); + } + + /** @return iterable */ + public static function pcntlFunctions(): iterable + { + yield 'pcntl_signal' => ['pcntl_signal']; + yield 'pcntl_signal_get_handler' => ['pcntl_signal_get_handler']; + yield 'pcntl_async_signals' => ['pcntl_async_signals']; + } + + #[Group('ac-03-10')] + #[Group('ac-03-11')] + #[RequiresPhpExtension('pcntl')] + public function testInstallChainsEachSignalToItsEarlierHandlerAndTurnsOnAsyncSignals(): void + { + $this->resetHandlers(); + $calls = []; + $customer = static function (mixed ...$arguments) use (&$calls): void { + $calls[] = $arguments; + }; + pcntl_signal(\SIGTERM, $customer); + + $this->drain()->install(); + + $this->assertTrue(pcntl_async_signals(), 'the stdio loop sleeps: handlers must run without pcntl_signal_dispatch()'); + $onTerm = pcntl_signal_get_handler(\SIGTERM); + $onInt = pcntl_signal_get_handler(\SIGINT); + $this->assertInstanceOf(\Closure::class, $onTerm); + $this->assertInstanceOf(\Closure::class, $onInt); + $this->assertNotSame($customer, $onTerm); + $onTerm(\SIGTERM, null); + $raisedOnTerm = $this->raised; + $onInt(\SIGINT, null); + $this->assertSame([[\SIGTERM, null]], $calls, 'only SIGTERM had an earlier handler'); + $this->assertSame([], $raisedOnTerm, 'the earlier SIGTERM handler owns termination'); + $this->assertSame([['signal' => \SIGINT, 'delivered' => 0]], $this->raised); + } + + #[RequiresPhpExtension('pcntl')] + public function testInstallTwiceInstallsOnce(): void + { + $this->resetHandlers(); + $drain = $this->drain(); + $drain->install(); + $later = static function (): void {}; + pcntl_signal(\SIGTERM, $later); + + $drain->install(); + + $this->assertSame($later, pcntl_signal_get_handler(\SIGTERM), 'the second install() changed nothing'); + } + + /** Review Focus 2: the application chose to ignore the signal, so AgentCat leaves it alone. */ + #[RequiresPhpExtension('pcntl')] + public function testAnIgnoredSignalStaysIgnored(): void + { + $this->resetHandlers(); + pcntl_signal(\SIGTERM, \SIG_IGN); + + $this->drain()->install(); + + $this->assertSame(\SIG_IGN, pcntl_signal_get_handler(\SIGTERM)); + $this->assertInstanceOf(\Closure::class, pcntl_signal_get_handler(\SIGINT)); + } + + /** + * The default `raise`, twice. A forked copy of this process gets SIGTERM and must end by it. This process gets + * SIGWINCH, whose default disposition is "ignore": it shows the handler reset to `SIG_DFL` and survives. + */ + #[Group('ac-03-10')] + #[RequiresPhpExtension('pcntl')] + #[RequiresPhpExtension('posix')] + public function testTheDefaultRaiseEndsTheProcessWithTheDefaultDisposition(): void + { + $drain = new SignalDrain(fn(): EventQueue => $this->queue, null, static fn(string $function): bool => true); + pcntl_signal(\SIGWINCH, static function (): void {}); + + $child = pcntl_fork(); + if ($child === 0) { + try { + $drain->onSignal(\SIGTERM, \SIG_DFL); + } finally { + posix_kill(posix_getpid(), \SIGKILL); // never run the rest of the test suite in the child + } + } + $this->assertGreaterThan(0, $child, 'fork failed'); + $status = 0; + pcntl_waitpid($child, $status); + $drain->onSignal(\SIGWINCH, \SIG_DFL); + + $this->assertIsInt($status); + $this->assertTrue(pcntl_wifsignaled($status), 'the child ended by a signal'); + $this->assertSame(\SIGTERM, pcntl_wtermsig($status)); + $this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGWINCH)); + } + + /** SIG_DFL on both signals and synchronous dispatch, so that a test sees exactly what `install()` changed. */ + private function resetHandlers(): void + { + pcntl_signal(\SIGTERM, \SIG_DFL); + pcntl_signal(\SIGINT, \SIG_DFL); + pcntl_async_signals(false); + } + + /** @param (\Closure(string): bool)|null $functionExists */ + private function drain(?\Closure $functionExists = null, string $sapi = 'cli'): SignalDrain + { + return new SignalDrain( + fn(): EventQueue => $this->queue, + $this->raise(...), + $functionExists ?? static fn(string $function): bool => true, + $sapi, + ); + } + + private function raise(int $signal): void + { + $this->raised[] = ['signal' => $signal, 'delivered' => count($this->transport->deliveries)]; + } + + private function queued(string $name): QueuedEvent + { + return new QueuedEvent(new Event(projectId: 'proj_test', eventType: 'mcp:tools/call', resourceName: $name)); + } +} From 28b39a6477876f57abb4718b9021491a7bc718aa Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 16:07:03 +0000 Subject: [PATCH 07/17] fix: forward siginfo, hand the signal back before the drain, and keep one shutdown budget per queue --- composer.json | 2 + src/Internal/Delivery/EventQueue.php | 87 ++++++++------ src/Internal/Delivery/SignalDrain.php | 32 +++-- src/Internal/Transport/CurlTransport.php | 7 +- .../Delivery/SignalDrainDeliveryTest.php | 109 +++++++++++++++++- tests/Integration/Fixtures/signal-drain.php | 30 ++++- .../Transport/CurlTransportTest.php | 30 +++++ .../Unit/Internal/Delivery/EventQueueTest.php | 19 +++ .../Internal/Delivery/SignalDrainTest.php | 64 +++++++++- 9 files changed, 321 insertions(+), 59 deletions(-) diff --git a/composer.json b/composer.json index f534246..af4e296 100644 --- a/composer.json +++ b/composer.json @@ -62,6 +62,8 @@ "mcp/sdk": "<0.8.1" }, "suggest": { + "ext-pcntl": "Delivers the queued events when a CLI server (an MCP stdio server) gets SIGTERM or SIGINT", + "ext-posix": "Ends the process with the signal after that delivery, and finds the home directory for agentcat.log when HOME is not set", "mcp/sdk": "Track mcp/sdk servers with AgentCat::track($builder, $projectId)" }, "minimum-stability": "stable", diff --git a/src/Internal/Delivery/EventQueue.php b/src/Internal/Delivery/EventQueue.php index 8640e0e..9fd36c6 100644 --- a/src/Internal/Delivery/EventQueue.php +++ b/src/Internal/Delivery/EventQueue.php @@ -49,6 +49,9 @@ final class EventQueue /** True while `pump()` or `flush()` runs: guards against re-entrant calls from hooks and exporters. */ private bool $busy = false; + /** Fixed by the first `shutdown()`: later shutdowns deliver only until then (one budget per queue). */ + private ?float $shutdownDeadline = null; + /** * @param (\Closure(): void)|null $onEnqueue the delivery-trigger registrar, called on every `add()` * @param (\Closure(Event): string)|null $encode builds the HTTP body; default `WireEncoder::encode(...)` @@ -145,6 +148,54 @@ public function pump(): void * limit. A re-entrant call (from a hook or an exporter) returns false at once. */ public function flush(?float $timeoutSeconds = null): bool + { + return $this->deliverUntil(fn(): float => $timeoutSeconds === null ? INF : $this->transport->now() + $this->budget($timeoutSeconds)); + } + + /** + * Stops accepting events, then delivers for at most the shutdown timeout (TypeScript `destroy()`). The first call + * fixes the deadline; a later call delivers only until that same deadline. So a signal drain and the shutdown + * function after it, or a signal during the shutdown function's flush, share one budget. + */ + public function shutdown(): void + { + $this->accepting = false; + $this->deliverUntil(fn(): float => $this->shutdownDeadline ??= $this->transport->now() + $this->budget($this->shutdownTimeoutSeconds)); + if (!$this->waiting->isEmpty()) { + Log::write(sprintf('Shutting down with %d events still in queue', $this->waiting->count())); + } + } + + /** + * `shutdown()` from the last shutdown function or from the signal drain. An `exit()` or a fatal error inside a + * `pump()` or `flush()` frame (in a hook or an exporter) skips the rest of it, `finally` included, and leaves the + * re-entrancy guard set; a signal can arrive while such a frame runs. Clear the guard first, so that the waiting + * events are still delivered. + */ + public function finalShutdown(): void + { + $this->busy = false; + $this->shutdown(); + } + + /** @return array{queueLength: int, activeRequests: int} */ + public function stats(): array + { + return ['queueLength' => $this->waiting->count(), 'activeRequests' => $this->transport->inFlight()]; + } + + /** A NaN or negative timeout means "no time left". max() is no guard: max(0.0, NAN) is NaN on PHP 8.4+. */ + private function budget(float $timeoutSeconds): float + { + return is_nan($timeoutSeconds) ? 0.0 : max(0.0, $timeoutSeconds); + } + + /** + * The delivery loop of `flush()` and `shutdown()`. A re-entrant call returns false at once. + * + * @param \Closure(): float $deadline read once, after the re-entrancy guard is taken + */ + private function deliverUntil(\Closure $deadline): bool { if ($this->busy) { return false; @@ -152,21 +203,18 @@ public function flush(?float $timeoutSeconds = null): bool $this->busy = true; try { - // A NaN or negative timeout means "no time left". max() is no guard: max(0.0, NAN) is NaN on PHP 8.4+. - $deadline = $timeoutSeconds === null - ? INF - : $this->transport->now() + (is_nan($timeoutSeconds) ? 0.0 : max(0.0, $timeoutSeconds)); + $until = $deadline(); while (true) { $this->pumpOnce(); if ($this->waiting->isEmpty() && $this->transport->inFlight() === 0) { return true; } - if ($this->transport->now() >= $deadline) { + if ($this->transport->now() >= $until) { return false; } // Slots freed by the last pump are refilled at once; waiting would sleep with idle slots. if ($this->waiting->isEmpty() || $this->transport->inFlight() >= self::CONCURRENCY) { - $this->transport->waitForActivity($deadline); + $this->transport->waitForActivity($until); } } } catch (\Throwable $e) { @@ -178,33 +226,6 @@ public function flush(?float $timeoutSeconds = null): bool } } - /** Stops accepting events, then delivers for at most the shutdown timeout (TypeScript `destroy()`). */ - public function shutdown(): void - { - $this->accepting = false; - $this->flush($this->shutdownTimeoutSeconds); - if (!$this->waiting->isEmpty()) { - Log::write(sprintf('Shutting down with %d events still in queue', $this->waiting->count())); - } - } - - /** - * `shutdown()` from the last shutdown function. No `pump()` or `flush()` frame is live then, but an `exit()` - * or a fatal error inside one (in a hook or an exporter) skips the rest of it, `finally` included, and leaves - * the re-entrancy guard set: clear it first, so that the waiting events are still delivered. - */ - public function finalShutdown(): void - { - $this->busy = false; - $this->shutdown(); - } - - /** @return array{queueLength: int, activeRequests: int} */ - public function stats(): array - { - return ['queueLength' => $this->waiting->count(), 'activeRequests' => $this->transport->inFlight()]; - } - private function pumpOnce(): void { try { diff --git a/src/Internal/Delivery/SignalDrain.php b/src/Internal/Delivery/SignalDrain.php index 271ee64..e7d1e2e 100644 --- a/src/Internal/Delivery/SignalDrain.php +++ b/src/Internal/Delivery/SignalDrain.php @@ -9,10 +9,16 @@ * a handler, these signals end PHP at once: no shutdown function runs, and the waiting events are lost. * * The global queue calls `install()` when it gets an event, so nothing happens at autoload. It installs once, only - * under the CLI SAPI with ext-pcntl, and keeps the handler each signal had before. On the signal it shuts the queue down - * (`EventQueue::SHUTDOWN_TIMEOUT_SECONDS` at most). Then it calls that earlier handler, and the process keeps - * running, or, when there was none, it raises the signal again with the default disposition (TypeScript - * `runSignalShutdown`). A signal that the application ignores (`SIG_IGN`) stays ignored. + * under the CLI SAPI with ext-pcntl, and keeps the handler each signal had before. A signal that the application + * ignores (`SIG_IGN`) stays ignored. When it installs a handler, `install()` also turns on + * `pcntl_async_signals(true)`, which is process wide: from then on every PHP signal handler of the process runs + * between two opcodes, not at `pcntl_signal_dispatch()` or a tick. + * + * Each handler runs once (TypeScript `process.once`). It first gives the signal back to the earlier handler, so a + * second Ctrl-C during the drain acts as if AgentCat were not there. Then it shuts the queue down + * (`EventQueue::SHUTDOWN_TIMEOUT_SECONDS` at most, shared with the shutdown function), then calls that earlier + * handler with PHP's arguments, and the process keeps running, or, when there was none, it raises the signal again + * with the default disposition (TypeScript `runSignalShutdown`). * * @internal */ @@ -55,16 +61,21 @@ public function install(): void } } $this->installed = true; + $installedAny = false; foreach ([\SIGTERM, \SIGINT] as $signal) { $previous = pcntl_signal_get_handler($signal); if ($previous === \SIG_IGN) { continue; // the application ignores this signal: so do we } - pcntl_signal($signal, function (int $signo) use ($previous): void { - $this->onSignal($signo, $previous); + pcntl_signal($signal, function (int $signo, mixed $siginfo = null) use ($previous): void { + pcntl_signal($signo, $previous); // once: a second signal during the drain goes where it went before + $this->onSignal($signo, $previous, $siginfo); }); + $installedAny = true; + } + if ($installedAny) { + pcntl_async_signals(true); // the stdio loop sleeps in usleep(): handlers must run without pcntl_signal_dispatch() } - pcntl_async_signals(true); // the stdio loop sleeps in usleep(): handlers must run without pcntl_signal_dispatch() } /** @@ -72,16 +83,17 @@ public function install(): void * signal again (`03` AC-10). * * @param mixed $previous the handler before ours: a callable, or `SIG_DFL` + * @param mixed $siginfo PHP's second handler argument (an array), passed on to `$previous` unchanged */ - public function onSignal(int $signal, mixed $previous): void + public function onSignal(int $signal, mixed $previous, mixed $siginfo = null): void { try { - ($this->queue)()->finalShutdown(); // at most EventQueue::SHUTDOWN_TIMEOUT_SECONDS + ($this->queue)()->finalShutdown(); // at most what is left of EventQueue::SHUTDOWN_TIMEOUT_SECONDS } catch (\Throwable) { // a signal handler must never throw } if (is_callable($previous)) { - $previous($signal, null); + $previous($signal, $siginfo); return; } diff --git a/src/Internal/Transport/CurlTransport.php b/src/Internal/Transport/CurlTransport.php index 53c28d6..6f2e904 100644 --- a/src/Internal/Transport/CurlTransport.php +++ b/src/Internal/Transport/CurlTransport.php @@ -272,15 +272,16 @@ private function launch(): void continue; } curl_setopt_array($handle, $this->curlOptions($next['delivery'])); + // Recorded before the add: a signal drain that runs right after the add must see this transfer. + $id = spl_object_id($handle); + $this->active[$id] = ['handle' => $handle] + $next; $code = ($this->multiStatus)('curl_multi_add_handle', curl_multi_add_handle($this->multi(), $handle)); if ($code !== CURLM_OK) { + unset($this->active[$id]); // PHP keeps the handle in its list of the multi handle even when libcurl refused it. curl_multi_remove_handle($this->multi(), $handle); $this->failed($next['delivery'], $next['attempt'], $this->multiError($code)); - - continue; } - $this->active[spl_object_id($handle)] = ['handle' => $handle] + $next; } } diff --git a/tests/Integration/Delivery/SignalDrainDeliveryTest.php b/tests/Integration/Delivery/SignalDrainDeliveryTest.php index 9472ba4..270ffcd 100644 --- a/tests/Integration/Delivery/SignalDrainDeliveryTest.php +++ b/tests/Integration/Delivery/SignalDrainDeliveryTest.php @@ -22,6 +22,9 @@ final class SignalDrainDeliveryTest extends TestCase /** Seconds from start to death by SIGTERM; the fixture's own loop runs for 5 s. */ private const float SIGNAL_DEADLINE = 3.0; + /** `EventQueue::SHUTDOWN_TIMEOUT_SECONDS`. */ + private const float BUDGET = 5.0; + private static ?MockCollector $collector = null; /** HOME of the script (its agentcat.log) and the marker file. */ @@ -106,10 +109,11 @@ public function testWithACustomerHandlerTheQueueDrainsThenTheHandlerRunsAndThePr $contents = file_get_contents($marker); $this->assertIsString($contents); $lines = explode("\n", $contents); - $this->assertCount(2, $lines); - [$who, $time] = $lines; + $this->assertCount(3, $lines); + [$who, $time, $signo] = $lines; $this->assertSame('customer', $who); $this->assertIsNumeric($time); + $this->assertSame('15', $signo, "the handler typed (int \$signo, array \$info) got PHP's siginfo"); $requests = $this->collector()->requests($base); $this->assertCount(1, $requests); $start = $requests[0]->start; @@ -117,6 +121,70 @@ public function testWithACustomerHandlerTheQueueDrainsThenTheHandlerRunsAndThePr $this->assertGreaterThan((float) $start, (float) $time, 'the queue drained before the customer handler ran'); } + /** TypeScript `process.once`: AgentCat gives SIGTERM back before it drains, so a second one ends the process. */ + #[MaximumDuration(10000)] + public function testASecondSigtermDuringTheDrainEndsTheProcessAtOnce(): void + { + [$hole, $url] = $this->blackHole(); + $process = $this->fixture($url, 'twice'); + $process->start(); + $this->assertTrue($this->waitForLine($process, 'signaled'), $process->getErrorOutput()); + $first = microtime(true); + $this->waitUntil($process, $first + 0.3); + + $process->signal(15); + $process->wait(); + $elapsed = microtime(true) - $first; + + $this->assertTrue($process->hasBeenSignaled(), $process->getErrorOutput()); + $this->assertSame(15, $process->getTermSignal()); + $this->assertLessThan(self::SIGNAL_DEADLINE - 1.0, $elapsed, 'the second SIGTERM did not wait for the 5 s drain'); + fclose($hole); + } + + /** One budget: a chained handler that exits runs the shutdown function, which must not drain for 5 s more. */ + #[Group('ac-03-11')] + #[MaximumDuration(10000)] + public function testAChainedHandlerThatExitsEndsWithinOneShutdownBudget(): void + { + [$hole, $url] = $this->blackHole(); + $process = $this->fixture($url, 'chain-exit'); + $process->start(); + $this->assertTrue($this->waitForLine($process, 'signaled'), $process->getErrorOutput()); + $signaled = microtime(true); + + $process->wait(); + $elapsed = microtime(true) - $signaled; + + $this->assertSame(0, $process->getExitCode(), $process->getErrorOutput()); + $this->assertGreaterThan(self::BUDGET - 0.5, $elapsed, 'the drain used its budget (the collector never answers)'); + $this->assertLessThan(self::BUDGET + 0.5, $elapsed, 'one 5 s budget, not two'); + fclose($hole); + } + + /** One budget: SIGTERM 2 s into the shutdown function's flush ends the process when that flush would have. */ + #[Group('ac-03-10')] + #[MaximumDuration(10000)] + public function testASigtermDuringTheShutdownFlushKeepsItsDeadline(): void + { + [$hole, $url] = $this->blackHole(); + $process = $this->fixture($url, 'shutdown'); + $process->start(); + $this->assertTrue($this->waitForLine($process, 'ending'), $process->getErrorOutput()); + $flushStart = microtime(true); + $this->waitUntil($process, $flushStart + 2.0); + + $process->signal(15); + $process->wait(); + $elapsed = microtime(true) - $flushStart; + + $this->assertTrue($process->hasBeenSignaled(), $process->getErrorOutput()); + $this->assertSame(15, $process->getTermSignal()); + $this->assertGreaterThan(self::BUDGET - 0.5, $elapsed, 'the signal drain kept the remaining budget'); + $this->assertLessThan(self::BUDGET + 0.5, $elapsed, 'the signal did not start a new 5 s budget'); + fclose($hole); + } + private function fixture(string $base, string $mode, string $marker = ''): Process { $process = new Process( @@ -129,6 +197,43 @@ private function fixture(string $base, string $mode, string $marker = ''): Proce return $process; } + /** + * A listening socket that never accepts: the kernel completes the connection, curl sends its request, and no answer + * comes (a collector that is reachable but stuck). Keep the socket open while the fixture runs. + * + * @return array{resource, string} the socket and its base URL + */ + private function blackHole(): array + { + $socket = stream_socket_server('tcp://127.0.0.1:0', $errorCode, $errorMessage); + $this->assertIsResource($socket, $errorMessage ?? ''); + $name = stream_socket_get_name($socket, false); + $this->assertIsString($name); + + return [$socket, 'http://' . $name]; + } + + /** Waits (within the process timeout) until the fixture prints `$line`; false when it ended first. */ + private function waitForLine(Process $process, string $line): bool + { + $output = ''; + + return $process->waitUntil(static function (string $type, string $chunk) use (&$output, $line): bool { + $output .= $chunk; + + return str_contains($output, $line . "\n"); + }); + } + + /** Waits until `$deadline` (microtime) while the fixture runs. */ + private function waitUntil(Process $process, float $deadline): void + { + while ($process->isRunning() && microtime(true) < $deadline) { + usleep(5_000); + } + $this->assertTrue($process->isRunning(), 'the fixture ended early: ' . $process->getErrorOutput()); + } + /** @return list the `resource_name` of each request the collector got, in order */ private function resourceNames(string $base): array { diff --git a/tests/Integration/Fixtures/signal-drain.php b/tests/Integration/Fixtures/signal-drain.php index 8a674af..5b130dc 100644 --- a/tests/Integration/Fixtures/signal-drain.php +++ b/tests/Integration/Fixtures/signal-drain.php @@ -10,22 +10,40 @@ require dirname(__DIR__, 3) . '/vendor/autoload.php'; -// A long-running CLI server that gets SIGTERM while one event waits in the global queue. +// A long-running CLI server with one event in the global queue. // Usage: signal-drain.php -// default: no SIGTERM handler of its own. AgentCat delivers the event, then SIGTERM ends the process. -// chain: its own SIGTERM handler first. AgentCat delivers the event, then calls that handler, which writes -// "customer" and the time to ; the script goes on and prints "done". +// default: no SIGTERM handler of its own. It sends itself SIGTERM: AgentCat delivers the event, then SIGTERM +// ends the process. +// chain: its own SIGTERM handler first, typed the way PHP calls it (int $signo, array $info). AgentCat +// delivers the event, then calls that handler, which writes "customer", the time and $info['signo'] to +// ; the script goes on and prints "done". +// chain-exit: its own SIGTERM handler calls exit(0). It prints "signaled" just before it sends itself SIGTERM. +// twice: like default, but it prints "signaled" just before the SIGTERM, so that the test can send another. +// shutdown: no signal of its own: it prints "ending" and ends, so that the shutdown function delivers the event. $baseUrl = $argv[1] ?? ''; $mode = $argv[2] ?? ''; $markerFile = $argv[3] ?? ''; if ($mode === 'chain') { - pcntl_signal(SIGTERM, static function () use ($markerFile): void { - file_put_contents($markerFile, sprintf("customer\n%.6F", microtime(true))); + pcntl_signal(SIGTERM, static function (int $signo, array $info) use ($markerFile): void { + file_put_contents($markerFile, sprintf("customer\n%.6F\n%s", microtime(true), var_export($info['signo'] ?? null, true))); + }); +} +if ($mode === 'chain-exit') { + pcntl_signal(SIGTERM, static function (): never { + exit(0); }); } $queue = EventQueue::global(); $queue->configure($baseUrl); $queue->add(new QueuedEvent(new Event(projectId: 'proj_test', eventType: 'mcp:tools/call', resourceName: 'signal-drain'))); +if ($mode === 'shutdown') { + echo "ending\n"; + + exit(0); +} +if ($mode === 'chain-exit' || $mode === 'twice') { + echo "signaled\n"; +} posix_kill(posix_getpid(), SIGTERM); if ($mode === 'chain') { usleep(200_000); diff --git a/tests/Integration/Transport/CurlTransportTest.php b/tests/Integration/Transport/CurlTransportTest.php index 36c659f..05a209f 100644 --- a/tests/Integration/Transport/CurlTransportTest.php +++ b/tests/Integration/Transport/CurlTransportTest.php @@ -613,6 +613,36 @@ public function testAnAddThatFailsOnceIsRetried(): void $this->assertCount(1, self::collector()->requests($base)); } + /** + * A signal can run a nested drain right after `curl_multi_add_handle()`. The transfer must count as in flight by + * then, or the drain loses its result and the entry written afterwards never finishes. + */ + #[MaximumDuration(1000)] + public function testATransferIsInFlightWhenItsHandleIsAdded(): void + { + $base = self::collector()->scenario(); + $probe = new class { + public ?CurlTransport $transport = null; + + /** @var list `inFlight()` when `curl_multi_add_handle()` returned */ + public array $inFlightAtAdd = []; + }; + $multiStatus = static function (string $function, int $code) use ($probe): int { + if ($function === 'curl_multi_add_handle' && $probe->transport instanceof CurlTransport) { + $probe->inFlightAtAdd[] = $probe->transport->inFlight(); + } + + return $code; + }; + $transport = new CurlTransport(multiStatus: $multiStatus); + $probe->transport = $transport; + + $this->assertTrue($transport->send($this->delivery($base), $this->deadline())); + + $this->assertSame([1], $probe->inFlightAtAdd); + $this->assertSame([self::SUCCESS], $this->logger->messages()); + } + #[Group('ac-07-13')] #[MaximumDuration(1000)] public function testAFailingExecFailsTheTransfersSoNothingWaitsForever(): void diff --git a/tests/Unit/Internal/Delivery/EventQueueTest.php b/tests/Unit/Internal/Delivery/EventQueueTest.php index 7e68f90..d9ffc8e 100644 --- a/tests/Unit/Internal/Delivery/EventQueueTest.php +++ b/tests/Unit/Internal/Delivery/EventQueueTest.php @@ -504,6 +504,25 @@ public function testShutdownUsesItsTimeout(): void $this->assertSame([], $this->logger->messages(), 'in-flight deliveries are not reported'); } + /** A signal drain, the shutdown function after it, and a signal during that flush share one budget. */ + #[Group('ac-03-9')] + public function testLaterShutdownsShareTheFirstShutdownsBudget(): void + { + $transport = new RecordingTransport(complete: false); + $queue = new EventQueue(transport: $transport); + for ($i = 0; $i < 6; ++$i) { + $queue->add(new QueuedEvent($this->event())); + } + + $queue->shutdown(); + $queue->finalShutdown(); + $queue->shutdown(); + + $this->assertEqualsWithDelta(5.0, $transport->now(), PHP_FLOAT_EPSILON, 'one budget in total'); + $this->assertSame(1, $transport->waits); + $this->assertSame(array_fill(0, 3, 'Shutting down with 1 events still in queue'), $this->logger->messages()); + } + public function testShutdownDeliversEverything(): void { $transport = new RecordingTransport(); diff --git a/tests/Unit/Internal/Delivery/SignalDrainTest.php b/tests/Unit/Internal/Delivery/SignalDrainTest.php index ebe9190..dc71a06 100644 --- a/tests/Unit/Internal/Delivery/SignalDrainTest.php +++ b/tests/Unit/Internal/Delivery/SignalDrainTest.php @@ -91,12 +91,28 @@ public function testAnEarlierHandlerIsCalledAfterTheDrainAndNothingIsRaised(): v $calls[] = ['arguments' => $arguments, 'delivered' => count($this->transport->deliveries)]; }; - $this->drain()->onSignal(self::SIGTERM, $customer); + $this->drain()->onSignal(self::SIGTERM, $customer, ['signo' => self::SIGTERM, 'pid' => 42]); - $this->assertSame([['arguments' => [self::SIGTERM, null], 'delivered' => 1]], $calls, 'called once, after the drain'); + $this->assertSame( + [['arguments' => [self::SIGTERM, ['signo' => self::SIGTERM, 'pid' => 42]], 'delivered' => 1]], + $calls, + 'called once, after the drain, with PHP\'s signal information unchanged', + ); $this->assertSame([], $this->raised, 'the earlier handler owns termination'); } + public function testWithoutSignalInformationTheEarlierHandlerGetsNull(): void + { + $calls = []; + $customer = static function (mixed ...$arguments) use (&$calls): void { + $calls[] = $arguments; + }; + + $this->drain()->onSignal(self::SIGTERM, $customer); + + $this->assertSame([[self::SIGTERM, null]], $calls); + } + public function testASignalHandlerNeverThrows(): void { $drain = new SignalDrain( @@ -163,12 +179,35 @@ public function testInstallChainsEachSignalToItsEarlierHandlerAndTurnsOnAsyncSig $this->assertInstanceOf(\Closure::class, $onTerm); $this->assertInstanceOf(\Closure::class, $onInt); $this->assertNotSame($customer, $onTerm); - $onTerm(\SIGTERM, null); + $onTerm(\SIGTERM, ['signo' => \SIGTERM]); $raisedOnTerm = $this->raised; - $onInt(\SIGINT, null); - $this->assertSame([[\SIGTERM, null]], $calls, 'only SIGTERM had an earlier handler'); + $termHandlerAfter = pcntl_signal_get_handler(\SIGTERM); + $onInt(\SIGINT, ['signo' => \SIGINT]); + $this->assertSame([[\SIGTERM, ['signo' => \SIGTERM]]], $calls, 'only SIGTERM had an earlier handler'); $this->assertSame([], $raisedOnTerm, 'the earlier SIGTERM handler owns termination'); $this->assertSame([['signal' => \SIGINT, 'delivered' => 0]], $this->raised); + $this->assertSame($customer, $termHandlerAfter, 'once: a second SIGTERM goes to the earlier handler'); + $this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGINT), 'once: a second SIGINT gets the default disposition'); + } + + /** Hands each signal back before the drain, so a second Ctrl-C during the drain acts as without AgentCat. */ + #[RequiresPhpExtension('pcntl')] + public function testTheHandlerGivesTheSignalBackBeforeTheDrain(): void + { + $this->resetHandlers(); + $seen = []; + $queue = function () use (&$seen): EventQueue { + $seen[] = pcntl_signal_get_handler(\SIGINT); + + return $this->queue; + }; + (new SignalDrain($queue, $this->raise(...), static fn(string $function): bool => true, 'cli'))->install(); + $onInt = pcntl_signal_get_handler(\SIGINT); + $this->assertInstanceOf(\Closure::class, $onInt); + + $onInt(\SIGINT, null); + + $this->assertSame([\SIG_DFL], $seen, 'SIG_DFL was back when the drain started'); } #[RequiresPhpExtension('pcntl')] @@ -198,6 +237,21 @@ public function testAnIgnoredSignalStaysIgnored(): void $this->assertInstanceOf(\Closure::class, pcntl_signal_get_handler(\SIGINT)); } + /** Async signals are process wide: with nothing installed, AgentCat leaves the mode alone. */ + #[RequiresPhpExtension('pcntl')] + public function testWithBothSignalsIgnoredNothingChanges(): void + { + $this->resetHandlers(); + pcntl_signal(\SIGTERM, \SIG_IGN); + pcntl_signal(\SIGINT, \SIG_IGN); + + $this->drain()->install(); + + $this->assertSame(\SIG_IGN, pcntl_signal_get_handler(\SIGTERM)); + $this->assertSame(\SIG_IGN, pcntl_signal_get_handler(\SIGINT)); + $this->assertFalse(pcntl_async_signals()); + } + /** * The default `raise`, twice. A forked copy of this process gets SIGTERM and must end by it. This process gets * SIGWINCH, whose default disposition is "ignore": it shows the handler reset to `SIG_DFL` and survives. From 47ed2b56f1871536110a789339c9e545a056f8ed Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 16:34:02 +0000 Subject: [PATCH 08/17] test: prove tracking end to end over HTTP with a real mcp/sdk server --- .github/workflows/ci.yml | 2 +- composer-dependency-analyser.php | 4 +- phpunit.xml.dist | 3 + tests/E2E/Fixtures/http-server.php | 56 ++++ tests/E2E/HttpEndToEndTest.php | 383 +++++++++++++++++++++++++++ tests/E2E/Support/BuiltInServer.php | 149 +++++++++++ tests/E2E/Support/McpHttpClient.php | 173 ++++++++++++ tests/Support/McpSdk/HttpHarness.php | 30 ++- tests/bin/mcp-sdk-main.sh | 2 +- 9 files changed, 796 insertions(+), 6 deletions(-) create mode 100644 tests/E2E/Fixtures/http-server.php create mode 100644 tests/E2E/HttpEndToEndTest.php create mode 100644 tests/E2E/Support/BuiltInServer.php create mode 100644 tests/E2E/Support/McpHttpClient.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index df0ce53..9e6b8c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -147,7 +147,7 @@ jobs: - name: Install mcp/sdk main at the commit this release was verified against run: composer require --dev --no-interaction --update-with-all-dependencies "mcp/sdk:dev-main#82e19d114e95671e05e7df7909195f7ace03d90e as 0.9.0" - run: vendor/bin/phpstan analyse --no-progress --error-format=github - - run: vendor/bin/phpunit --testsuite Contract,Integration + - run: vendor/bin/phpunit --testsuite Contract,Integration,E2E all-green: name: All green diff --git a/composer-dependency-analyser.php b/composer-dependency-analyser.php index 5b046f2..4d2ca7a 100644 --- a/composer-dependency-analyser.php +++ b/composer-dependency-analyser.php @@ -17,4 +17,6 @@ // Psr\Container\ContainerInterface arrives with mcp/sdk; the adapter only passes it through. ->ignoreErrorsOnPackageAndPath('psr/container', __DIR__ . '/src/McpSdk', [ErrorType::SHADOW_DEPENDENCY]) // Psr\EventDispatcher\EventDispatcherInterface arrives with mcp/sdk; the test double stands in for a customer's dispatcher. - ->ignoreErrorsOnPackageAndPath('psr/event-dispatcher', __DIR__ . '/tests/Support/McpSdk/RecordingDispatcher.php', [ErrorType::SHADOW_DEPENDENCY]); + ->ignoreErrorsOnPackageAndPath('psr/event-dispatcher', __DIR__ . '/tests/Support/McpSdk/RecordingDispatcher.php', [ErrorType::SHADOW_DEPENDENCY]) + // Http\Discovery\Psr17Factory arrives with mcp/sdk; the HTTP fixture builds its request from the globals with it, like a customer. + ->ignoreErrorsOnPackageAndPath('php-http/discovery', __DIR__ . '/tests/E2E/Fixtures', [ErrorType::SHADOW_DEPENDENCY]); diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 2284557..38cd911 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -23,6 +23,9 @@ tests/Contract + + tests/E2E + tests/Architecture diff --git a/tests/E2E/Fixtures/http-server.php b/tests/E2E/Fixtures/http-server.php new file mode 100644 index 0000000..1336345 --- /dev/null +++ b/tests/E2E/Fixtures/http-server.php @@ -0,0 +1,56 @@ +setServerInfo('e2e-http-server', '3.1.4') + ->setSession(new FileSessionStore((string) getenv('AGENTCAT_E2E_SESSIONS'))) + ->addTool(static fn(string $msg): string => 'echo: ' . $msg, 'echo', description: 'Echo a message', inputSchema: ['type' => 'object', 'properties' => ['msg' => ['type' => 'string']], 'required' => ['msg']]) + ->addTool(static fn(string $msg): string => 'strict: ' . $msg, 'strict', description: 'Strict echo', inputSchema: ['type' => 'object', 'properties' => ['msg' => ['type' => 'string']], 'required' => ['msg'], 'additionalProperties' => false]) + ->addTool(static function (): string { + throw new \RuntimeException('e2e boom'); + }, 'boom', description: 'Always fails', inputSchema: ['type' => 'object', 'properties' => []]); + +AgentCat::track($builder, 'proj_' . str_repeat('e', 27), new Options( + identify: static function (ToolCall $call) use ($markers): Identity { + if (($call->headers['x-e2e-identify-sleep'] ?? '') === '1') { + usleep(1_000_000); + file_put_contents($markers . '/identify', sprintf('%.6F', microtime(true))); + } + + return new Identity('e2e-user', 'E2E User', ['plan' => 'pro']); + }, + apiBaseUrl: $collector, +)); + +$factory = new Psr17Factory(); +$response = $builder->build()->run(new StreamableHttpTransport($factory->createServerRequestFromGlobals(), $factory, $factory)); +$body = (string) $response->getBody(); +http_response_code($response->getStatusCode()); +foreach ($response->getHeaders() as $name => $values) { + foreach ($values as $value) { + header($name . ': ' . $value, false); + } +} +header('Content-Length: ' . strlen($body)); // the client can finish reading before the shutdown delivery runs +echo $body; +flush(); diff --git a/tests/E2E/HttpEndToEndTest.php b/tests/E2E/HttpEndToEndTest.php new file mode 100644 index 0000000..bbe5124 --- /dev/null +++ b/tests/E2E/HttpEndToEndTest.php @@ -0,0 +1,383 @@ + 'true', + 'HOME' => self::$dir . '/home', + 'AGENTCAT_E2E_SESSIONS' => self::$dir . '/sessions', + 'AGENTCAT_E2E_MARKERS' => self::$dir . '/markers', + ]); + } catch (\Throwable $e) { + self::tearDownAfterClass(); // PHPUnit skips it when this method fails + + throw $e; + } + } + + #[\Override] + public static function tearDownAfterClass(): void + { + self::$server?->stop(); + self::$server = null; + self::$collector?->stop(); + self::$collector = null; + self::remove(self::$dir); + } + + #[\Override] + protected function setUp(): void + { + $this->scenario = $this->collector()->scenario(); + } + + /** M6: one tracked call through the handshake, the session store and the shutdown delivery. */ + #[Group('ac-02-53')] + #[Group('ac-02-43')] + #[Group('ac-04-49')] + #[MaximumDuration(10000)] + public function testAHandshakeCallIsTrackedEndToEnd(): void + { + $client = $this->client(); + $client->initialize(); + $tools = $client->listTools()->tools ?? null; + $this->assertIsList($tools); + $names = array_map(static fn(mixed $tool): mixed => JsonAssert::at($tool, 'name'), $tools); + $this->assertSame(['echo', 'strict', 'boom', GetMoreTools::NAME], $names); + $this->assertSame(['msg', Constants::PARAM_SESSION_ID, Constants::PARAM_CONTEXT], $this->keys(JsonAssert::at($tools, 0, 'inputSchema', 'properties'))); + + $answer = $client->callTool('echo', ['msg' => 'hi', 'context' => 'why', 'session_id' => 'start'], $this->headers(['X-AgentCat-Health-Run' => 'S'])); + + $event = $this->event(); + $sessionId = JsonAssert::at($event, 'session_id'); + $this->assertIsString($sessionId); + $this->assertMatchesRegularExpression('/\Ases_[0-9A-Za-z]{27}\z/', $sessionId); + $this->assertSame([$this->issued($sessionId), 'echo: hi'], $this->texts(HttpHarness::result($answer['message']))); + $this->assertSame('echo', JsonAssert::at($event, 'resource_name')); + $this->assertSame('why', JsonAssert::at($event, 'user_intent')); + $this->assertSame('e2e-client', JsonAssert::at($event, 'client_name'), 'the handshake ladder through FileSessionStore'); + $this->assertSame('e2e-http-server', JsonAssert::at($event, 'server_name')); + $this->assertSame('3.1.4', JsonAssert::at($event, 'server_version')); + $this->assertSame('e2e-user', JsonAssert::at($event, 'identify_actor_given_id')); + $this->assertSame('{"plan":"pro"}', $this->json(JsonAssert::at($event, 'identify_data'))); + $this->assertSame('minted', JsonAssert::at($event, 'tags', Constants::TAG_SESSION_SOURCE)); + $this->assertSame( + '{"msg":"hi","context":"why","session_id":"start"}', + $this->json(JsonAssert::at($event, 'parameters', 'request', 'params', 'arguments')), + ); + $extra = JsonAssert::at($event, 'parameters', 'extra'); + $this->assertSame($client->sessionId(), JsonAssert::at($extra, 'sessionId')); + // ac-02-43, ac-04-49: the HTTP request, as TypeScript v1 records it (D34) + $requestInfo = JsonAssert::at($extra, 'requestInfo'); + $this->assertInstanceOf(\stdClass::class, $requestInfo); + $this->assertSame('S', JsonAssert::at($requestInfo, 'headers', 'x-agentcat-health-run')); + $this->assertSame($this->server()->baseUrl() . '/', JsonAssert::at($requestInfo, 'url')); + $this->assertObjectNotHasProperty('method', $requestInfo); + } + + #[Group('ac-02-20')] + #[MaximumDuration(10000)] + public function testEveryRequestRebuildsTheInjectionRegistries(): void + { + $client = $this->client(); + $client->initialize(); + $client->listTools(); + $before = $this->rebuildLines(); + + $client->callTool('echo', ['msg' => 'hi', 'session_id' => 'start'], $this->headers()); + + $this->event(); + $this->assertSame($before + 1, $this->rebuildLines(), 'the tools/list of the earlier request ran in another PHP request'); + } + + #[Group('ac-02-17')] + #[MaximumDuration(10000)] + public function testAStrictToolAcceptsTheInjectedParameters(): void + { + $client = $this->client(); + $client->initialize(); + + $answer = $client->callTool('strict', ['msg' => 'hi', 'session_id' => 'start', 'context' => 'why'], $this->headers()); + + $event = $this->event(); + $sessionId = JsonAssert::at($event, 'session_id'); + $this->assertIsString($sessionId); + $this->assertSame([$this->issued($sessionId), 'strict: hi'], $this->texts(HttpHarness::result($answer['message']))); + $this->assertObjectNotHasProperty('is_error', $event); + } + + /** + * Header conflict 6: the tool result does not wait for the hooks. The fixture's `identify` sleeps 1 s and then + * writes the marker; it runs in the shutdown delivery, after the response. + */ + #[Group('ac-01-37')] + #[Group('ac-02-48')] + #[Group('ac-03-14')] + #[MaximumDuration(10000)] + public function testTheResponseDoesNotWaitForTheHooks(): void + { + $client = $this->client(); + $client->initialize(); + $sentAt = microtime(true); + + $answer = $client->callTool('echo', ['msg' => 'hi', 'session_id' => 'start'], $this->headers(['X-E2E-Identify-Sleep' => '1'])); + + $this->assertLessThan(0.8, $answer['receivedAt'] - $sentAt, 'the response arrived before the hook ran'); + HttpHarness::result($answer['message']); + $event = $this->event(); + $this->assertSame('e2e-user', JsonAssert::at($event, 'identify_actor_given_id')); + $marker = file_get_contents(self::$dir . '/markers/identify'); + $this->assertIsString($marker); + $this->assertIsNumeric($marker); + $this->assertGreaterThan($answer['receivedAt'], (float) $marker, 'identify ran after the client had its response'); + } + + #[MaximumDuration(10000)] + public function testAModernEraCallIsTrackedWithoutAHandshake(): void + { + $client = $this->client(); + + $answer = $client->callToolModern('echo', ['msg' => 'm'], ['name' => 'modern-client', 'version' => '2.0.0'], $this->headers()); + + $event = $this->event(); + $sessionId = JsonAssert::at($event, 'session_id'); + $this->assertIsString($sessionId); + $this->assertSame([$this->issued($sessionId), 'echo: m'], $this->texts(HttpHarness::result($answer['message']))); + $this->assertSame('modern-client', JsonAssert::at($event, 'client_name')); + $this->assertSame(HttpHarness::MODERN_PROTOCOL_VERSION, JsonAssert::at($event, 'tags', Constants::TAG_PROTOCOL_VERSION)); + $extra = JsonAssert::at($event, 'parameters', 'extra'); + $this->assertInstanceOf(\stdClass::class, $extra); + $this->assertObjectNotHasProperty('sessionId', $extra); + } + + #[Group('ac-05-E5')] + #[Group('ac-02-25')] + #[MaximumDuration(10000)] + public function testAnUncaughtToolExceptionGivesTheLibrarysErrorAndAnEventWithTheException(): void + { + $client = $this->client(); + $client->initialize(); + + $answer = $client->callTool('boom', ['session_id' => 'start'], $this->headers()); + + $this->assertSame('{"code":-32603,"message":"Error while executing tool"}', $this->json(JsonAssert::at($answer['message'], 'error'))); + $event = $this->event(); + $this->assertTrue(JsonAssert::at($event, 'is_error')); + $this->assertSame('RuntimeException', JsonAssert::at($event, 'error', 'type')); + $this->assertSame('e2e boom', JsonAssert::at($event, 'error', 'message')); + $frames = JsonAssert::at($event, 'error', 'frames'); + $this->assertIsList($frames); + $files = array_map(static fn(mixed $frame): mixed => JsonAssert::at($frame, 'filename'), $frames); + $this->assertNotSame([], array_filter($files, static fn(mixed $file): bool => is_string($file) && str_ends_with($file, 'http-server.php')), $this->json($files)); + } + + #[Group('ac-02-40')] + #[MaximumDuration(10000)] + public function testGetMoreToolsIsAnsweredByAgentCat(): void + { + $client = $this->client(); + $client->initialize(); + + $answer = $client->callTool(GetMoreTools::NAME, ['context' => 'need X'], $this->headers()); + + $event = $this->event(); + $sessionId = JsonAssert::at($event, 'session_id'); + $this->assertIsString($sessionId); + $this->assertSame([$this->issued($sessionId), $this->cannedText()], $this->texts(HttpHarness::result($answer['message']))); + $this->assertSame(GetMoreTools::NAME, JsonAssert::at($event, 'resource_name')); + } + + /** + * Review Focus 5. Repeated Accept lines differ by PHP version (Ruling R7): `php -S` on PHP 8.3 keeps the first + * line, on PHP 8.4 and 8.5 it joins them with `, `. The fixture records the value the SAPI gave PHP, and the + * event must carry exactly that. + */ + #[MaximumDuration(10000)] + public function testOddHeadersReachTheEventAsStrings(): void + { + $client = $this->client(); + $client->initialize(); + $cookie = 'big=' . str_repeat('c', 8_996); + + $answer = $client->callTool('echo', ['msg' => 'hi', 'session_id' => 'start'], $this->headers([ + 'X-Odd' => "caf\xE9", + 'Accept' => ['application/json', 'text/event-stream'], + 'Cookie' => $cookie, + 'X-E2E-Record-Accept' => '1', + ])); + + $this->assertSame('echo: hi', JsonAssert::at(HttpHarness::result($answer['message']), 'content', 1, 'text')); + $headers = JsonAssert::at($this->event(), 'parameters', 'extra', 'requestInfo', 'headers'); + $this->assertSame("caf\u{FFFD}", JsonAssert::at($headers, 'x-odd')); + $this->assertSame(9_000, strlen($cookie)); + $this->assertSame($cookie, JsonAssert::at($headers, 'cookie')); + $accept = file_get_contents(self::$dir . '/markers/accept'); + $this->assertIsString($accept); + $this->assertStringStartsWith('application/json', $accept); + $this->assertSame($accept, JsonAssert::at($headers, 'accept')); + } + + private function client(): McpHttpClient + { + return new McpHttpClient($this->server()->baseUrl() . '/'); + } + + /** + * The headers of a tool call: this test's collector scenario, plus `$extra`. + * + * @param array> $extra + * + * @return array> + */ + private function headers(array $extra = []): array + { + return ['X-E2E-Collector' => $this->scenario, ...$extra]; + } + + /** Waits for the one event of this test's scenario and returns its decoded body. */ + private function event(): \stdClass + { + $deadline = microtime(true) + self::COLLECTOR_DEADLINE_SECONDS; + do { + $requests = $this->collector()->requests($this->scenario); + if ($requests !== []) { + break; + } + usleep(20_000); + } while (microtime(true) < $deadline); + $this->assertCount(1, $requests, 'one POST to the collector'); + $this->assertSame('POST', $requests[0]->method); + $body = $requests[0]->body; + $this->assertIsString($body); + $event = Json::decode($body); + $this->assertInstanceOf(\stdClass::class, $event, $body); + + return $event; + } + + /** How often the server's agentcat.log says that a request rebuilt the registries. */ + private function rebuildLines(): int + { + $log = file_get_contents(self::$dir . '/home/agentcat.log'); + $this->assertIsString($log); + + return substr_count($log, self::REBUILT); + } + + private function issued(string $sessionId): string + { + return implode("\n", [Constants::MINT_BACK_HEADER_ISSUED, 'session_id: ' . $sessionId, Constants::MINT_BACK_ISSUED_BODY]); + } + + private function cannedText(): string + { + $text = JsonAssert::at(Golden::json('handles/strings.json'), 'GET_MORE_TOOLS_RESULT', 'content', 0, 'text'); + $this->assertIsString($text); + + return $text; + } + + /** @return list the `text` of every content block */ + private function texts(\stdClass $result): array + { + $content = $result->content ?? null; + $this->assertIsList($content); + + return array_map(static fn(mixed $block): mixed => JsonAssert::at($block, 'text'), $content); + } + + /** @return list */ + private function keys(mixed $object): array + { + $this->assertInstanceOf(\stdClass::class, $object); + + return array_map(strval(...), array_keys(get_object_vars($object))); + } + + private function json(mixed $value): string + { + return json_encode($value, JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR); + } + + private function collector(): MockCollector + { + $this->assertInstanceOf(MockCollector::class, self::$collector); + + return self::$collector; + } + + private function server(): BuiltInServer + { + $this->assertInstanceOf(BuiltInServer::class, self::$server); + + return self::$server; + } + + /** The bootstrap removes only plain files in the test HOME (Ruling R8): remove this tree here. */ + private static function remove(string $path): void + { + if ($path === '' || (!is_dir($path) && !is_file($path))) { + return; + } + if (is_file($path) || is_link($path)) { + unlink($path); + + return; + } + $names = scandir($path); + foreach ($names === false ? [] : $names as $name) { + if ($name !== '.' && $name !== '..') { + self::remove($path . '/' . $name); + } + } + rmdir($path); + } +} diff --git a/tests/E2E/Support/BuiltInServer.php b/tests/E2E/Support/BuiltInServer.php new file mode 100644 index 0000000..55aae85 --- /dev/null +++ b/tests/E2E/Support/BuiltInServer.php @@ -0,0 +1,149 @@ + ` with 4 workers: an HTTP server whose every request is a fresh PHP request, + * like PHP-FPM. + * + * Test classes start one in `setUpBeforeClass()` and stop it in `tearDownAfterClass()`. Like `MockCollector`, the + * server runs under `tests/Integration/Support/watchdog.php`, which stops it when this process closes its stdin: on + * `stop()`, and also when the test process dies without running any cleanup. + */ +final class BuiltInServer +{ + private const float START_TIMEOUT_SECONDS = 5.0; + + private const float STOP_TIMEOUT_SECONDS = 1.0; + + /** A new port is tried when the server exits at once (another process took the free port first). */ + private const int START_ATTEMPTS = 3; + + private bool $stopped = false; + + private function __construct( + private readonly Process $process, + private readonly InputStream $input, + private readonly int $port, + ) {} + + public function __destruct() + { + $this->stop(); + } + + /** + * Starts the server and waits until it listens. + * + * @param string $router the router script that answers every request + * @param array $env environment variables of the server, on top of this process's + */ + public static function start(string $router, array $env): self + { + $errors = ''; + for ($attempt = 1; $attempt <= self::START_ATTEMPTS; ++$attempt) { + $server = self::launch($router, $env); + if ($server->waitUntilListening()) { + return $server; + } + $exitedAtOnce = !$server->process->isRunning(); + $errors .= $server->process->getErrorOutput() . $server->process->getOutput(); + $server->stop(); + if (!$exitedAtOnce) { + break; + } + } + + throw new \RuntimeException('The built-in server did not start: ' . $errors); + } + + /** `http://127.0.0.1:`, without a trailing slash. */ + public function baseUrl(): string + { + return 'http://127.0.0.1:' . $this->port; + } + + /** Closing stdin makes the watchdog stop the server; a kill is the last resort. */ + public function stop(): void + { + if ($this->stopped) { + return; + } + $this->stopped = true; + $this->input->close(); + $deadline = microtime(true) + self::STOP_TIMEOUT_SECONDS; + while ($this->process->isRunning() && microtime(true) < $deadline) { + usleep(10_000); + } + $this->process->stop(0); + } + + /** @param array $env */ + private static function launch(string $router, array $env): self + { + $port = self::freePort(); + $input = new InputStream(); + // php.ini-production sets output_buffering=4096, and PHP empties that output buffer only after the shutdown + // functions, so the response would wait for AgentCat's shutdown delivery. Without it, the router's flush() + // sends the body before the shutdown functions run. + $process = new Process( + [PHP_BINARY, __DIR__ . '/../../Integration/Support/watchdog.php', PHP_BINARY, '-d', 'output_buffering=0', '-S', '127.0.0.1:' . $port, $router], + null, + ['PHP_CLI_SERVER_WORKERS' => '4', ...$env], + $input, + null, + ); + $process->start(); + + return new self($process, $input, $port); + } + + private function waitUntilListening(): bool + { + $deadline = microtime(true) + self::START_TIMEOUT_SECONDS; + $banner = '(http://127.0.0.1:' . $this->port . ') started'; + set_error_handler(static fn(): bool => true); // connection refused warnings while the server boots + + try { + while ($this->process->isRunning()) { + // The banner proves that this server, not another process, owns the port. + if (str_contains($this->process->getErrorOutput() . $this->process->getOutput(), $banner)) { + $socket = stream_socket_client('tcp://127.0.0.1:' . $this->port, $errorCode, $errorMessage, 1.0); + if ($socket !== false) { + fclose($socket); + + return true; + } + } + if (microtime(true) >= $deadline) { + return false; + } + usleep(10_000); + } + + return false; + } finally { + restore_error_handler(); + } + } + + private static function freePort(): int + { + $server = stream_socket_server('tcp://127.0.0.1:0', $errorCode, $errorMessage); + if ($server === false) { + throw new \RuntimeException('No free port: ' . $errorMessage); + } + $name = stream_socket_get_name($server, false); + fclose($server); + if ($name === false || preg_match('/:(\d+)\z/', $name, $match) !== 1) { + throw new \RuntimeException('No free port'); + } + + return (int) $match[1]; + } +} diff --git a/tests/E2E/Support/McpHttpClient.php b/tests/E2E/Support/McpHttpClient.php new file mode 100644 index 0000000..6d946c7 --- /dev/null +++ b/tests/E2E/Support/McpHttpClient.php @@ -0,0 +1,173 @@ +sessionId; + } + + /** + * POSTs one JSON-RPC message, with the session headers once `initialize()` ran. A header in `$headers` replaces + * the default of the same name; a list value sends one header line per item. The response header names are in + * lower case, and repeated ones are joined with `, `. + * + * @param array $message + * @param array> $headers + * + * @return array{status: int, headers: array, body: string, receivedAt: float} + */ + public function post(array $message, array $headers = []): array + { + return $this->exchange($message, $headers, true); + } + + /** Sends `initialize` and `notifications/initialized`, and keeps the `Mcp-Session-Id`. */ + public function initialize(string $name = 'e2e-client', string $version = '1.0.0'): void + { + $result = HttpHarness::result($this->request('initialize', [ + 'protocolVersion' => self::HANDSHAKE_PROTOCOL_VERSION, + 'capabilities' => new \stdClass(), + 'clientInfo' => ['name' => $name, 'version' => $version], + ], [], true)['message']); + $this->protocolVersion = is_string($result->protocolVersion ?? null) ? $result->protocolVersion : self::HANDSHAKE_PROTOCOL_VERSION; + Assert::assertNotNull($this->sessionId, 'initialize issued no Mcp-Session-Id'); + + $response = $this->post(['jsonrpc' => '2.0', 'method' => 'notifications/initialized']); + Assert::assertSame(202, $response['status'], 'notifications/initialized'); + Assert::assertSame('', $response['body'], 'notifications/initialized'); + } + + /** The `result` of a `tools/list` request. */ + public function listTools(): \stdClass + { + return HttpHarness::result($this->request('tools/list', [], [], true)['message']); + } + + /** + * The whole answer to a `tools/call` request (`result` or `error`), and when it was read. + * + * @param array $arguments + * @param array> $headers + * + * @return array{message: \stdClass, receivedAt: float} + */ + public function callTool(string $name, array $arguments, array $headers = []): array + { + return $this->request('tools/call', ['name' => $name, 'arguments' => (object) $arguments], $headers, true); + } + + /** + * A 2026-07-28 `tools/call`: no session, the envelope and headers of `HttpHarness::modern()`. + * + * @param array $arguments + * @param array{name: string, version: string} $clientInfo + * @param array> $headers + * + * @return array{message: \stdClass, receivedAt: float} + */ + public function callToolModern(string $name, array $arguments, array $clientInfo, array $headers = []): array + { + $params = HttpHarness::modernParams(['name' => $name, 'arguments' => (object) $arguments], $clientInfo); + + return $this->request('tools/call', $params, [...HttpHarness::modernHeaders('tools/call', $name), ...$headers], false); + } + + /** + * @param array $params + * @param array> $headers + * + * @return array{message: \stdClass, receivedAt: float} + */ + private function request(string $method, array $params, array $headers, bool $withSession): array + { + $id = $this->nextId++; + $message = ['jsonrpc' => '2.0', 'id' => $id, 'method' => $method, 'params' => $params === [] ? new \stdClass() : $params]; + $response = $this->exchange($message, $headers, $withSession); + + return [ + 'message' => HttpHarness::decode($response['body'], $response['headers']['content-type'] ?? '', $id), + 'receivedAt' => $response['receivedAt'], + ]; + } + + /** + * @param array $message + * @param array> $headers + * + * @return array{status: int, headers: array, body: string, receivedAt: float} + */ + private function exchange(array $message, array $headers, bool $withSession): array + { + $lines = ['content-type' => ['Content-Type', ['application/json']], 'accept' => ['Accept', ['application/json, text/event-stream']]]; + if ($withSession && $this->sessionId !== null) { + $lines['mcp-session-id'] = ['Mcp-Session-Id', [$this->sessionId]]; + $lines['mcp-protocol-version'] = ['MCP-Protocol-Version', [$this->protocolVersion]]; + } + foreach ($headers as $name => $value) { + $lines[strtolower($name)] = [$name, is_string($value) ? [$value] : $value]; + } + $headerLines = ['Expect:']; // no 100-continue round trip + foreach ($lines as [$name, $values]) { + foreach ($values as $value) { + $headerLines[] = $name . ': ' . $value; + } + } + + $responseHeaders = []; + $handle = curl_init($this->url); + Assert::assertNotFalse($handle, 'curl_init'); + curl_setopt_array($handle, [ + CURLOPT_POST => true, + CURLOPT_POSTFIELDS => json_encode($message, JSON_THROW_ON_ERROR), + CURLOPT_HTTPHEADER => $headerLines, + CURLOPT_RETURNTRANSFER => true, + CURLOPT_TIMEOUT => self::TIMEOUT_SECONDS, + CURLOPT_HEADERFUNCTION => static function (\CurlHandle $handle, string $line) use (&$responseHeaders): int { + $parts = explode(':', $line, 2); + if (count($parts) === 2) { + $name = strtolower(trim($parts[0])); + $value = trim($parts[1]); + $responseHeaders[$name] = isset($responseHeaders[$name]) ? $responseHeaders[$name] . ', ' . $value : $value; + } + + return strlen($line); + }, + ]); + $body = curl_exec($handle); + $receivedAt = microtime(true); + Assert::assertIsString($body, 'curl: ' . curl_error($handle)); + $status = curl_getinfo($handle, CURLINFO_RESPONSE_CODE); + $session = $responseHeaders['mcp-session-id'] ?? ''; + if ($session !== '') { + $this->sessionId = $session; + } + + return ['status' => $status, 'headers' => $responseHeaders, 'body' => $body, 'receivedAt' => $receivedAt]; + } +} diff --git a/tests/Support/McpSdk/HttpHarness.php b/tests/Support/McpSdk/HttpHarness.php index e65c0b9..880492e 100644 --- a/tests/Support/McpSdk/HttpHarness.php +++ b/tests/Support/McpSdk/HttpHarness.php @@ -99,6 +99,19 @@ public function callTool(string $name, array $arguments, ?array $meta = null): \ * @param array{name: string, version: string}|null $clientInfo */ public function modern(string $method, array $params, ?string $name = null, ?array $clientInfo = self::MODERN_CLIENT): \stdClass + { + return $this->send($method, self::modernParams($params, $clientInfo), self::modernHeaders($method, $name), false); + } + + /** + * The params of a 2026-07-28 request: the protocol version, client capabilities and client info in `_meta`. + * + * @param array $params + * @param array{name: string, version: string}|null $clientInfo + * + * @return array + */ + public static function modernParams(array $params, ?array $clientInfo): array { $meta = is_array($params['_meta'] ?? null) ? $params['_meta'] : []; $meta['io.modelcontextprotocol/protocolVersion'] = self::MODERN_PROTOCOL_VERSION; @@ -107,12 +120,23 @@ public function modern(string $method, array $params, ?string $name = null, ?arr $meta['io.modelcontextprotocol/clientInfo'] = $clientInfo; } $params['_meta'] = $meta; + + return $params; + } + + /** + * The standard headers of a 2026-07-28 request. + * + * @return array + */ + public static function modernHeaders(string $method, ?string $name): array + { $headers = ['MCP-Protocol-Version' => self::MODERN_PROTOCOL_VERSION, 'Mcp-Method' => $method]; if ($name !== null) { $headers['Mcp-Name'] = $name; } - return $this->send($method, $params, $headers, false); + return $headers; } /** @@ -149,7 +173,7 @@ private function send(string $method, array $params, array $headers, bool $withS $message = ['jsonrpc' => '2.0', 'id' => $id, 'method' => $method, 'params' => $params === [] ? new \stdClass() : $params]; $response = $this->post($message, $headers, $withSession); - return $this->decode((string) $response->getBody(), $response->getHeaderLine('Content-Type'), $id); + return self::decode((string) $response->getBody(), $response->getHeaderLine('Content-Type'), $id); } /** @@ -180,7 +204,7 @@ private function post(array $message, array $headers, bool $withSession): Respon } /** A JSON body, or the `data:` line of an SSE body that answers request `$id`. */ - private function decode(string $body, string $contentType, int $id): \stdClass + public static function decode(string $body, string $contentType, int $id): \stdClass { if (str_starts_with($contentType, 'text/event-stream')) { foreach (explode("\n", $body) as $line) { diff --git a/tests/bin/mcp-sdk-main.sh b/tests/bin/mcp-sdk-main.sh index ec53ed5..029cd69 100755 --- a/tests/bin/mcp-sdk-main.sh +++ b/tests/bin/mcp-sdk-main.sh @@ -19,4 +19,4 @@ if [ -n "${MCP_SDK_REPO:-}" ]; then fi composer require --dev --no-interaction --update-with-all-dependencies "mcp/sdk:dev-main#82e19d114e95671e05e7df7909195f7ace03d90e as 0.9.0" vendor/bin/phpstan analyse --no-progress --memory-limit=1G -vendor/bin/phpunit --testsuite "${SUITES:-Contract,Integration}" +vendor/bin/phpunit --testsuite "${SUITES:-Contract,Integration,E2E}" From a71edbb0033aa8dfe3c12807a4a1c8f00a303fc0 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 16:50:10 +0000 Subject: [PATCH 09/17] test: give the HTTP end-to-end test room on slow runners and keep its server bounded and local --- tests/E2E/BuiltInServerTest.php | 101 +++++++++++++++++++++++++ tests/E2E/Fixtures/http-server.php | 2 +- tests/E2E/Fixtures/hung-server.php | 17 +++++ tests/E2E/HttpEndToEndTest.php | 68 +++++++++++++++-- tests/E2E/Support/BuiltInServer.php | 23 ++++-- tests/Integration/Support/watchdog.php | 17 ++++- 6 files changed, 213 insertions(+), 15 deletions(-) create mode 100644 tests/E2E/BuiltInServerTest.php create mode 100644 tests/E2E/Fixtures/hung-server.php diff --git a/tests/E2E/BuiltInServerTest.php b/tests/E2E/BuiltInServerTest.php new file mode 100644 index 0000000..ca644fa --- /dev/null +++ b/tests/E2E/BuiltInServerTest.php @@ -0,0 +1,101 @@ +assertIsString($home); + $this->dir = $home . '/e2e-server-' . bin2hex(random_bytes(6)); + $this->assertTrue(mkdir($this->dir, 0o700)); + } + + #[\Override] + protected function tearDown(): void + { + if (is_file($this->dir . '/pid')) { + unlink($this->dir . '/pid'); + } + if (is_dir($this->dir)) { + rmdir($this->dir); + } + } + + #[RequiresPhpExtension('pcntl')] + #[RequiresPhpExtension('posix')] + #[MaximumDuration(10000)] + public function testStopEndsAWorkerThatIgnoresSigterm(): void + { + $server = BuiltInServer::start(__DIR__ . '/Fixtures/hung-server.php', ['AGENTCAT_E2E_MARKERS' => $this->dir]); + $response = (new McpHttpClient($server->baseUrl() . '/'))->post(['jsonrpc' => '2.0', 'method' => 'ping']); + $this->assertSame('hanging', $response['body']); + $pid = file_get_contents($this->dir . '/pid'); + $this->assertIsString($pid); + $this->assertIsNumeric($pid); + $this->assertTrue($this->alive((int) $pid), 'the worker hangs after its response'); + + $server->stop(); + + $deadline = microtime(true) + self::DEADLINE_SECONDS; + while ($this->alive((int) $pid) && microtime(true) < $deadline) { + usleep(10_000); + } + $this->assertFalse($this->alive((int) $pid), 'stop() ended the hung worker'); + } + + #[MaximumDuration(10000)] + public function testTheWatchdogStopsItsCommandAfterTheTimeout(): void + { + $watchdog = new Process( + [PHP_BINARY, __DIR__ . '/../Integration/Support/watchdog.php', PHP_BINARY, '-r', 'sleep(30);'], + null, + ['AGENTCAT_WATCHDOG_TIMEOUT' => '0.2'], + new InputStream(), // stays open: only the timeout can stop the command + 10, + ); + $startedAt = microtime(true); + + $watchdog->start(); + $deadline = $startedAt + self::DEADLINE_SECONDS; + while ($watchdog->isRunning() && microtime(true) < $deadline) { + usleep(10_000); + } + + $this->assertFalse($watchdog->isRunning(), 'the watchdog ended after its timeout'); + $this->assertSame(0, $watchdog->getExitCode()); + } + + /** + * A zombie (exited, not yet reaped by its new parent) counts as ended. + * + * @phpstan-impure + */ + private function alive(int $pid): bool + { + if (!posix_kill($pid, 0)) { + return false; + } + $stat = @file_get_contents('/proc/' . $pid . '/stat'); + + return !is_string($stat) || preg_match('/\) Z /', $stat) !== 1; + } +} diff --git a/tests/E2E/Fixtures/http-server.php b/tests/E2E/Fixtures/http-server.php index 1336345..589dc19 100644 --- a/tests/E2E/Fixtures/http-server.php +++ b/tests/E2E/Fixtures/http-server.php @@ -33,7 +33,7 @@ AgentCat::track($builder, 'proj_' . str_repeat('e', 27), new Options( identify: static function (ToolCall $call) use ($markers): Identity { if (($call->headers['x-e2e-identify-sleep'] ?? '') === '1') { - usleep(1_000_000); + usleep(3_000_000); file_put_contents($markers . '/identify', sprintf('%.6F', microtime(true))); } diff --git a/tests/E2E/Fixtures/hung-server.php b/tests/E2E/Fixtures/hung-server.php new file mode 100644 index 0000000..c0d3f51 --- /dev/null +++ b/tests/E2E/Fixtures/hung-server.php @@ -0,0 +1,17 @@ + 'true', + 'AGENTCAT_API_URL' => self::$fallbackUrl, 'HOME' => self::$dir . '/home', 'AGENTCAT_E2E_SESSIONS' => self::$dir . '/sessions', 'AGENTCAT_E2E_MARKERS' => self::$dir . '/markers', @@ -155,13 +168,14 @@ public function testAStrictToolAcceptsTheInjectedParameters(): void } /** - * Header conflict 6: the tool result does not wait for the hooks. The fixture's `identify` sleeps 1 s and then - * writes the marker; it runs in the shutdown delivery, after the response. + * Header conflict 6: the tool result does not wait for the hooks. The fixture's `identify` sleeps 3 s and then + * writes the marker; it runs in the shutdown delivery, after the response. The 2 s bound leaves room for a slow + * runner; the marker proves the order. */ #[Group('ac-01-37')] #[Group('ac-02-48')] #[Group('ac-03-14')] - #[MaximumDuration(10000)] + #[MaximumDuration(15000)] public function testTheResponseDoesNotWaitForTheHooks(): void { $client = $this->client(); @@ -170,9 +184,9 @@ public function testTheResponseDoesNotWaitForTheHooks(): void $answer = $client->callTool('echo', ['msg' => 'hi', 'session_id' => 'start'], $this->headers(['X-E2E-Identify-Sleep' => '1'])); - $this->assertLessThan(0.8, $answer['receivedAt'] - $sentAt, 'the response arrived before the hook ran'); + $this->assertLessThan(2.0, $answer['receivedAt'] - $sentAt, 'the response arrived before the hook ran'); HttpHarness::result($answer['message']); - $event = $this->event(); + $event = $this->event(self::SLEEPING_HOOK_DEADLINE_SECONDS); $this->assertSame('e2e-user', JsonAssert::at($event, 'identify_actor_given_id')); $marker = file_get_contents(self::$dir . '/markers/identify'); $this->assertIsString($marker); @@ -265,6 +279,23 @@ public function testOddHeadersReachTheEventAsStrings(): void $this->assertSame($accept, JsonAssert::at($headers, 'accept')); } + /** A call without `X-E2E-Collector` falls back to `AGENTCAT_API_URL`, a closed local port: never the default API. */ + #[MaximumDuration(10000)] + public function testWithoutACollectorHeaderTheEventGoesToTheLocalFallback(): void + { + $client = $this->client(); + $client->initialize(); + $offset = strlen($this->log()); + + HttpHarness::result($client->callTool('echo', ['msg' => 'hi', 'session_id' => 'start'])['message']); + + $port = parse_url(self::$fallbackUrl, PHP_URL_PORT); + $this->assertIsInt($port); + $failure = $this->failedDelivery($offset); + $this->assertStringContainsString('127.0.0.1 port ' . $port, $failure, 'the delivery went to AGENTCAT_API_URL'); + $this->assertStringNotContainsString('agentcat.com', substr($this->log(), $offset)); + } + private function client(): McpHttpClient { return new McpHttpClient($this->server()->baseUrl() . '/'); @@ -283,9 +314,9 @@ private function headers(array $extra = []): array } /** Waits for the one event of this test's scenario and returns its decoded body. */ - private function event(): \stdClass + private function event(float $deadlineSeconds = self::COLLECTOR_DEADLINE_SECONDS): \stdClass { - $deadline = microtime(true) + self::COLLECTOR_DEADLINE_SECONDS; + $deadline = microtime(true) + $deadlineSeconds; do { $requests = $this->collector()->requests($this->scenario); if ($requests !== []) { @@ -305,11 +336,32 @@ private function event(): \stdClass /** How often the server's agentcat.log says that a request rebuilt the registries. */ private function rebuildLines(): int + { + return substr_count($this->log(), self::REBUILT); + } + + /** Waits for the first failed delivery that the server's agentcat.log reports after byte `$offset`. */ + private function failedDelivery(int $offset): string + { + $deadline = microtime(true) + self::LOG_DEADLINE_SECONDS; + do { + $written = substr($this->log(), $offset); + if (preg_match(self::FAILED_DELIVERY, $written, $match) === 1) { + return $match[0]; + } + usleep(20_000); + } while (microtime(true) < $deadline); + + self::fail('No failed delivery in agentcat.log: ' . $written); + } + + /** The server's agentcat.log so far. */ + private function log(): string { $log = file_get_contents(self::$dir . '/home/agentcat.log'); $this->assertIsString($log); - return substr_count($log, self::REBUILT); + return $log; } private function issued(string $sessionId): string diff --git a/tests/E2E/Support/BuiltInServer.php b/tests/E2E/Support/BuiltInServer.php index 55aae85..bef7864 100644 --- a/tests/E2E/Support/BuiltInServer.php +++ b/tests/E2E/Support/BuiltInServer.php @@ -12,8 +12,8 @@ * like PHP-FPM. * * Test classes start one in `setUpBeforeClass()` and stop it in `tearDownAfterClass()`. Like `MockCollector`, the - * server runs under `tests/Integration/Support/watchdog.php`, which stops it when this process closes its stdin: on - * `stop()`, and also when the test process dies without running any cleanup. + * server runs under `tests/Integration/Support/watchdog.php`, which stops it when this process closes its stdin, also + * when the test process dies without running any cleanup, and in any case after 120 s. */ final class BuiltInServer { @@ -21,6 +21,11 @@ final class BuiltInServer private const float STOP_TIMEOUT_SECONDS = 1.0; + /** Longer than any test class: the watchdog stops the server after that, even when this process hangs. */ + private const int LIFETIME_SECONDS = 120; + + private const int SIGKILL = 9; + /** A new port is tried when the server exits at once (another process took the free port first). */ private const int START_ATTEMPTS = 3; @@ -68,13 +73,21 @@ public function baseUrl(): string return 'http://127.0.0.1:' . $this->port; } - /** Closing stdin makes the watchdog stop the server; a kill is the last resort. */ + /** + * Kills the watchdog's process group: the watchdog, `php -S` and its workers, also a worker that ignores SIGTERM + * or hangs. The watchdog still runs at that moment, so the group ID is still its own. Without ext-posix, closing + * stdin makes the watchdog stop the server, and a kill of the watchdog is the last resort. + */ public function stop(): void { if ($this->stopped) { return; } $this->stopped = true; + $pid = $this->process->getPid(); + if ($pid !== null && $this->process->isRunning() && function_exists('posix_kill')) { + posix_kill(-$pid, self::SIGKILL); + } $this->input->close(); $deadline = microtime(true) + self::STOP_TIMEOUT_SECONDS; while ($this->process->isRunning() && microtime(true) < $deadline) { @@ -94,9 +107,9 @@ private static function launch(string $router, array $env): self $process = new Process( [PHP_BINARY, __DIR__ . '/../../Integration/Support/watchdog.php', PHP_BINARY, '-d', 'output_buffering=0', '-S', '127.0.0.1:' . $port, $router], null, - ['PHP_CLI_SERVER_WORKERS' => '4', ...$env], + ['PHP_CLI_SERVER_WORKERS' => '4', 'AGENTCAT_WATCHDOG_TIMEOUT' => (string) self::LIFETIME_SECONDS, ...$env], $input, - null, + self::LIFETIME_SECONDS, ); $process->start(); diff --git a/tests/Integration/Support/watchdog.php b/tests/Integration/Support/watchdog.php index 1ef54e2..8e2b179 100644 --- a/tests/Integration/Support/watchdog.php +++ b/tests/Integration/Support/watchdog.php @@ -5,7 +5,11 @@ // Runs the command given as arguments (the mock collector's `php -S`) until stdin reaches EOF, then stops it and // deletes $AGENTCAT_COLLECTOR_DIR. MockCollector::stop() closes stdin, and so does the death of the test process, // even by SIGKILL (an Infection timeout), so no collector outlives its test run. When the server exits first -// (port taken), this exits too. +// (port taken), this exits too. With $AGENTCAT_WATCHDOG_TIMEOUT (seconds), it also stops the command after that +// long, even when stdin stays open. +// +// With ext-posix, the watchdog leads its own process group, so the command and every child it forks (php -S workers) +// are in it too: posix_kill(-, 9) ends them all (BuiltInServer::stop()). $arguments = $_SERVER['argv'] ?? null; $command = is_array($arguments) ? array_values(array_filter(array_slice($arguments, 1), is_string(...))) : []; @@ -14,6 +18,12 @@ exit(2); } +if (function_exists('posix_setpgid')) { + posix_setpgid(0, 0); +} +$timeout = getenv('AGENTCAT_WATCHDOG_TIMEOUT'); +$deadline = is_string($timeout) && is_numeric($timeout) ? microtime(true) + (float) $timeout : INF; + $server = proc_open($command, [0 => ['pipe', 'r'], 1 => STDOUT, 2 => STDERR], $pipes); if ($server === false) { exit(1); @@ -21,6 +31,11 @@ stream_set_blocking(STDIN, false); while (proc_get_status($server)['running']) { + if (microtime(true) >= $deadline) { + proc_terminate($server); + + break; + } $read = [STDIN]; $write = null; $except = null; From a27b9cdd06e1640a745294fcf3c010078a780da1 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 17:17:57 +0000 Subject: [PATCH 10/17] test: prove tracking end to end over stdio, including EOF and SIGTERM delivery --- tests/E2E/Fixtures/stdio-server.php | 27 ++ tests/E2E/StdioEndToEndTest.php | 449 +++++++++++++++++++++++++++ tests/E2E/Support/StdioMcpClient.php | 156 ++++++++++ 3 files changed, 632 insertions(+) create mode 100644 tests/E2E/Fixtures/stdio-server.php create mode 100644 tests/E2E/StdioEndToEndTest.php create mode 100644 tests/E2E/Support/StdioMcpClient.php diff --git a/tests/E2E/Fixtures/stdio-server.php b/tests/E2E/Fixtures/stdio-server.php new file mode 100644 index 0000000..9cad99a --- /dev/null +++ b/tests/E2E/Fixtures/stdio-server.php @@ -0,0 +1,27 @@ + 'object', 'properties' => ['msg' => ['type' => 'string']], 'required' => ['msg']]; +$builder = Server::builder() + ->setServerInfo('e2e-stdio-server', '2.7.1') + ->addTool(static fn(string $msg): string => 'echo: ' . $msg, 'echo', description: 'Echo a message', inputSchema: $schema) + // Suspends the handler's fiber (a progress notification) in the middle of AgentCat's interceptor. + ->addTool(static function (string $msg, ClientGateway $gateway): string { + $gateway->progress(0.5, 1.0, 'half way'); + + return 'done: ' . $msg; + }, 'progress', description: 'Reports progress', inputSchema: $schema); + +$collector = getenv('AGENTCAT_E2E_COLLECTOR'); +AgentCat::track($builder, 'proj_' . str_repeat('s', 27), new Options(apiBaseUrl: is_string($collector) && $collector !== '' ? $collector : null)); + +exit($builder->build()->run(new StdioTransport())); diff --git a/tests/E2E/StdioEndToEndTest.php b/tests/E2E/StdioEndToEndTest.php new file mode 100644 index 0000000..cde2ded --- /dev/null +++ b/tests/E2E/StdioEndToEndTest.php @@ -0,0 +1,449 @@ +stop(); + self::$collector = null; + self::remove(self::$dir); + } + + #[\Override] + protected function tearDown(): void + { + $this->client?->process()->stop(0); + $this->client = null; + } + + /** M6: one stdio session through the handshake; both events reach the collector by the time stdin closes. */ + #[Group('ac-02-53')] + #[MaximumDuration(15000)] + public function testAHandshakeSessionIsTrackedAndDeliveredAtEof(): void + { + $client = $this->start(); + $this->handshake($client); + $tools = $this->resultOf($client, 'tools/list', [])->tools ?? null; + $this->assertIsList($tools); + $this->assertSame('echo', JsonAssert::at($tools, 0, 'name')); + + $first = $this->resultOf($client, 'tools/call', ['name' => 'echo', 'arguments' => ['msg' => 'one', 'session_id' => 'start']]); + $sessionId = $this->issuedSessionId($first); + $this->assertSame([$this->issued($sessionId), 'echo: one'], $this->texts($first)); + $second = $this->resultOf($client, 'tools/call', ['name' => 'echo', 'arguments' => ['msg' => 'two', 'session_id' => $sessionId]]); + $this->assertSame(['echo: two'], $this->texts($second), 'the issued session_id is recognized: no new block'); + + $this->assertExitsAtEof($client); + $events = $this->events(2); + foreach ($events as $event) { + $this->assertSame('e2e-stdio-server', JsonAssert::at($event, 'server_name')); + $this->assertSame('2.7.1', JsonAssert::at($event, 'server_version')); + $this->assertSame('stdio-client', JsonAssert::at($event, 'client_name')); + $this->assertSame('echo', JsonAssert::at($event, 'resource_name')); + $this->assertSame($sessionId, JsonAssert::at($event, 'session_id')); + } + $this->assertEqualsCanonicalizing(['one', 'two'], array_map( + static fn(\stdClass $event): mixed => JsonAssert::at($event, 'parameters', 'request', 'params', 'arguments', 'msg'), + $events, + )); + } + + /** No SDK byte on STDOUT or STDERR: the log lines go to `$HOME/agentcat.log`. */ + #[Group('ac-03-37')] + #[Group('ac-01-87')] + #[MaximumDuration(15000)] + public function testStdoutCarriesOnlyJsonRpcAndTheSdkLogsToItsFile(): void + { + $client = $this->start(); + $this->handshake($client); + $this->resultOf($client, 'tools/list', []); + $this->resultOf($client, 'tools/call', ['name' => 'echo', 'arguments' => ['msg' => 'hi', 'session_id' => 'start']]); + $this->resultOf($client, 'tools/call', ['name' => 'progress', 'arguments' => ['msg' => 'hi', 'session_id' => 'start'], '_meta' => ['progressToken' => 'p0']]); + + $this->assertExitsAtEof($client); + $this->events(2); + $this->assertStdoutIsOnlyJsonRpc($client); + $this->assertSame('', $client->process()->getErrorOutput()); + $log = $this->log(); + $this->assertStringContainsString('AgentCat setup started | project ' . self::PROJECT_ID . ' | sdk mcp-sdk/builder | signals ', $log); + $this->assertStringContainsString('AgentCat setup complete | project ' . self::PROJECT_ID . ' | tracing=true context=true reportMissing=true exporters=0', $log); + } + + /** The tool's progress notification suspends the handler's fiber in the middle of AgentCat's interceptor. */ + #[MaximumDuration(15000)] + public function testAFiberSuspendedInsideTheInterceptorIsAnsweredAndTrackedOnce(): void + { + $client = $this->start(); + $this->handshake($client); + $before = count($client->stdoutLines()); + + $result = $this->resultOf($client, 'tools/call', ['name' => 'progress', 'arguments' => ['msg' => 'x', 'session_id' => 'start'], '_meta' => ['progressToken' => 'p1']]); + + $sessionId = $this->issuedSessionId($result); + $this->assertSame([$this->issued($sessionId), 'done: x'], $this->texts($result)); + $messages = array_map(Json::decode(...), array_slice($client->stdoutLines(), $before)); + $this->assertCount(2, $messages, 'the notification, then the answer'); + $this->assertSame('notifications/progress', JsonAssert::at($messages[0], 'method')); + $this->assertSame('p1', JsonAssert::at($messages[0], 'params', 'progressToken')); + $this->assertSame($this->nextId - 1, JsonAssert::at($messages[1], 'id')); + + $this->assertExitsAtEof($client); + $event = $this->event(); + $this->assertSame('progress', JsonAssert::at($event, 'resource_name')); + $this->assertSame('done: x', JsonAssert::at($event, 'response', 'content', 0, 'text')); + $this->assertSame($sessionId, JsonAssert::at($event, 'session_id')); + } + + /** The pump at handler entry delivers an earlier call's event while stdin stays open. */ + #[MaximumDuration(15000)] + public function testAnEventIsDeliveredBetweenRequestsWhileStdinStaysOpen(): void + { + $client = $this->start(); + $this->handshake($client); + $sessionId = $this->issuedSessionId($this->resultOf($client, 'tools/call', ['name' => 'echo', 'arguments' => ['msg' => 'call 1', 'session_id' => 'start']])); + + $calls = 1; + $delivered = null; + while (!$delivered instanceof \stdClass && $calls < self::MAX_CALLS_BEFORE_DELIVERY) { + ++$calls; + $answer = $this->resultOf($client, 'tools/call', ['name' => 'echo', 'arguments' => ['msg' => 'call ' . $calls, 'session_id' => $sessionId]]); + $this->assertSame(['echo: call ' . $calls], $this->texts($answer)); + $delivered = $this->eventWithMessage('call 1', self::BETWEEN_CALLS_SECONDS); + } + + $this->assertInstanceOf(\stdClass::class, $delivered, sprintf('the first event reached the collector within %d calls', $calls)); + $this->assertTrue($client->process()->isRunning(), 'stdin is still open'); + $this->assertSame($sessionId, JsonAssert::at($delivered, 'session_id')); + $this->assertExitsAtEof($client); + $this->assertCount($calls, $this->events($calls), 'every call is delivered by the end'); + } + + /** + * Review Focus 3: stdin closes while the collector holds every delivery. The pump at each handler entry starts at + * most 5 deliveries, so 2 of the 7 events still wait when the 5 s shutdown budget ends. + */ + #[Group('ac-03-9')] + #[MaximumDuration(15000)] + public function testStdinClosingWhileTheCollectorIsSlowEndsWithinTheShutdownBudget(): void + { + $client = $this->start(['status' => 202, 'delayMs' => 8000]); + $this->handshake($client); + $sessionId = $this->issuedSessionId($this->resultOf($client, 'tools/call', ['name' => 'echo', 'arguments' => ['msg' => 'call 1', 'session_id' => 'start']])); + for ($call = 2; $call <= 7; ++$call) { + $this->resultOf($client, 'tools/call', ['name' => 'echo', 'arguments' => ['msg' => 'call ' . $call, 'session_id' => $sessionId]]); + } + + $this->assertExitsAtEof($client, self::SLOW_COLLECTOR_EXIT_SECONDS); + $log = $this->log(); + $this->assertSame(1, substr_count($log, 'Shutting down with '), $log); + $this->assertMatchesRegularExpression('/\] Shutting down with 2 events still in queue$/m', $log); + $this->assertStdoutIsOnlyJsonRpc($client); + $this->assertSame('', $client->process()->getErrorOutput()); + } + + /** SIGTERM with stdin open: the drain delivers the event, then the default disposition ends the process. */ + #[Group('ac-03-10')] + #[RequiresPhpExtension('pcntl')] + #[RequiresPhpExtension('posix')] + #[MaximumDuration(15000)] + public function testSigtermDeliversTheQueueAndThenEndsTheProcessBySignal(): void + { + $client = $this->start(); + $this->handshake($client); + $this->resultOf($client, 'tools/call', ['name' => 'echo', 'arguments' => ['msg' => 'hi', 'session_id' => 'start']]); + $this->assertSame([], $this->collector()->requests($this->scenario), 'nothing pumped the event yet'); + + $sentAt = microtime(true); + $client->signal(self::SIGTERM); + $code = $client->waitForExit(self::SIGTERM_EXIT_SECONDS); + + $this->assertNotNull($code, sprintf('the process ended within %.1f s', self::SIGTERM_EXIT_SECONDS)); + $this->assertTrue($client->process()->hasBeenSignaled(), sprintf('exit code %d after %.3f s', $code, microtime(true) - $sentAt)); + $this->assertSame(self::SIGTERM, $client->process()->getTermSignal()); + $event = $this->event(); + $this->assertSame('echo', JsonAssert::at($event, 'resource_name')); + $this->assertStdoutIsOnlyJsonRpc($client); + } + + /** mcp/sdk main serves the 2026-07-28 era over stdio when the first request carries its envelope. */ + #[MaximumDuration(15000)] + public function testAModernEraFirstRequestIsTrackedWithoutAHandshake(): void + { + if (!McpSdkVersion::isMain()) { + self::markTestSkipped('mcp/sdk 0.8.1 serves only the handshake era over stdio'); + } + $client = $this->start(); + + $result = $this->resultOf($client, 'tools/call', HttpHarness::modernParams( + ['name' => 'echo', 'arguments' => ['msg' => 'm']], + ['name' => 'modern-client', 'version' => '2.0.0'], + )); + + $sessionId = $this->issuedSessionId($result); + $this->assertSame([$this->issued($sessionId), 'echo: m'], $this->texts($result)); + $this->assertExitsAtEof($client); + $event = $this->event(); + $this->assertSame('modern-client', JsonAssert::at($event, 'client_name')); + $this->assertSame(HttpHarness::MODERN_PROTOCOL_VERSION, JsonAssert::at($event, 'tags', Constants::TAG_PROTOCOL_VERSION)); + $this->assertSame('echo', JsonAssert::at($event, 'resource_name')); + } + + /** + * Starts this test's server with its own collector scenario and HOME. `AGENTCAT_API_URL` is a closed local port, so + * an event without the scenario can never reach the AgentCat API. + * + * @param array{status: int, delayMs?: int, body?: string} $default the collector's answer to every event + */ + private function start(array $default = ['status' => 202, 'body' => '{"status":"accepted"}']): StdioMcpClient + { + $this->scenario = $this->collector()->scenario([], $default); + $this->home = self::$dir . '/home-' . bin2hex(random_bytes(6)); + $this->assertTrue(mkdir($this->home, 0o700)); + $this->client = StdioMcpClient::start(__DIR__ . '/Fixtures/stdio-server.php', [ + 'AGENTCAT_ENABLED' => 'true', + 'AGENTCAT_API_URL' => MockCollector::closedPortUrl(), + 'AGENTCAT_E2E_COLLECTOR' => $this->scenario, + 'HOME' => $this->home, + ]); + + return $this->client; + } + + /** `initialize` and `notifications/initialized`, as an MCP host opens a stdio session. */ + private function handshake(StdioMcpClient $client): void + { + $result = $this->resultOf($client, 'initialize', [ + 'protocolVersion' => self::HANDSHAKE_PROTOCOL_VERSION, + 'capabilities' => new \stdClass(), + 'clientInfo' => ['name' => 'stdio-client', 'version' => '1.0.0'], + ]); + $this->assertSame(self::HANDSHAKE_PROTOCOL_VERSION, JsonAssert::at($result, 'protocolVersion')); + $client->send(['jsonrpc' => '2.0', 'method' => 'notifications/initialized']); + } + + /** + * Sends one request and returns the `result` of its answer. + * + * @param array $params + */ + private function resultOf(StdioMcpClient $client, string $method, array $params): \stdClass + { + $id = $this->nextId++; + $client->send(['jsonrpc' => '2.0', 'id' => $id, 'method' => $method, 'params' => $params === [] ? new \stdClass() : $params]); + + return HttpHarness::result($client->receive($id)); + } + + /** Closes stdin and waits for exit code 0. */ + private function assertExitsAtEof(StdioMcpClient $client, float $timeoutSeconds = self::EOF_EXIT_SECONDS): void + { + $closedAt = microtime(true); + $client->closeInput(); + $code = $client->waitForExit($timeoutSeconds); + $this->assertSame(0, $code, sprintf('exit within %.1f s of stdin EOF (%.3f s); stderr: %s', $timeoutSeconds, microtime(true) - $closedAt, $client->process()->getErrorOutput())); + } + + /** Every byte on stdout belongs to a JSON-RPC message, one per line. */ + private function assertStdoutIsOnlyJsonRpc(StdioMcpClient $client): void + { + $lines = $client->stdoutLines(); + $this->assertNotSame([], $lines); + $this->assertSame(implode("\n", $lines) . "\n", $client->process()->getOutput(), 'stdout holds whole lines only'); + foreach ($lines as $line) { + $this->assertJson($line); + $message = Json::decode($line); + $this->assertInstanceOf(\stdClass::class, $message, $line); + $this->assertSame('2.0', $message->jsonrpc ?? null, $line); + } + } + + /** + * Waits until the collector holds `$count` events of this test's scenario and returns their decoded bodies. + * + * @return list<\stdClass> + */ + private function events(int $count): array + { + $deadline = microtime(true) + self::COLLECTOR_DEADLINE_SECONDS; + do { + $requests = $this->collector()->requests($this->scenario); + if (count($requests) >= $count) { + break; + } + usleep(20_000); + } while (microtime(true) < $deadline); + $this->assertCount($count, $requests, 'POSTs to the collector'); + + return array_map(function (\stdClass $request): \stdClass { + $this->assertSame('POST', $request->method); + $body = $request->body; + $this->assertIsString($body); + $event = Json::decode($body); + $this->assertInstanceOf(\stdClass::class, $event, $body); + + return $event; + }, $requests); + } + + /** Waits for the one event of this test's scenario and returns its decoded body. */ + private function event(): \stdClass + { + $event = $this->events(1)[0] ?? null; + $this->assertInstanceOf(\stdClass::class, $event); + + return $event; + } + + /** The collector's event whose call had `msg` `$msg`, waiting up to `$deadlineSeconds`; null if none came. */ + private function eventWithMessage(string $msg, float $deadlineSeconds): ?\stdClass + { + $deadline = microtime(true) + $deadlineSeconds; + do { + foreach ($this->collector()->requests($this->scenario) as $request) { + $event = is_string($request->body ?? null) ? Json::decode($request->body) : null; + if ($event instanceof \stdClass && JsonAssert::at($event, 'parameters', 'request', 'params', 'arguments', 'msg') === $msg) { + return $event; + } + } + usleep(20_000); + } while (microtime(true) < $deadline); + + return null; + } + + /** The server's agentcat.log. */ + private function log(): string + { + $log = file_get_contents($this->home . '/agentcat.log'); + $this->assertIsString($log); + + return $log; + } + + /** The `ses_` ID of the issued block at the start of an answer. */ + private function issuedSessionId(\stdClass $result): string + { + $text = JsonAssert::at($result, 'content', 0, 'text'); + $this->assertIsString($text); + $this->assertSame(1, preg_match('/^session_id: (ses_[0-9A-Za-z]{27})$/m', $text, $match), $text); + + return $match[1]; + } + + private function issued(string $sessionId): string + { + return implode("\n", [Constants::MINT_BACK_HEADER_ISSUED, 'session_id: ' . $sessionId, Constants::MINT_BACK_ISSUED_BODY]); + } + + /** @return list the `text` of every content block */ + private function texts(\stdClass $result): array + { + $content = $result->content ?? null; + $this->assertIsList($content); + + return array_map(static fn(mixed $block): mixed => JsonAssert::at($block, 'text'), $content); + } + + private function collector(): MockCollector + { + $this->assertInstanceOf(MockCollector::class, self::$collector); + + return self::$collector; + } + + /** The bootstrap removes only plain files in the test HOME (Ruling R8): remove this tree here. */ + private static function remove(string $path): void + { + if ($path === '' || (!is_dir($path) && !is_file($path))) { + return; + } + if (is_file($path) || is_link($path)) { + unlink($path); + + return; + } + $names = scandir($path); + foreach ($names === false ? [] : $names as $name) { + if ($name !== '.' && $name !== '..') { + self::remove($path . '/' . $name); + } + } + rmdir($path); + } +} diff --git a/tests/E2E/Support/StdioMcpClient.php b/tests/E2E/Support/StdioMcpClient.php new file mode 100644 index 0000000..a6a3227 --- /dev/null +++ b/tests/E2E/Support/StdioMcpClient.php @@ -0,0 +1,156 @@ +` with pipes, one JSON message per line on stdin, every + * stdout line kept for the test. + * + * The server has a 10 s timeout, checked whenever this client reads, so a hang fails the test instead of blocking it. + * The server also ends when this process dies: its stdin then reaches EOF. + */ +final class StdioMcpClient +{ + private const float TIMEOUT_SECONDS = 10.0; + + /** How long one poll of the pipes waits before the next one. */ + private const int POLL_MICROSECONDS = 2_000; + + /** @var list complete stdout lines, without their line feed */ + private array $lines = []; + + /** stdout bytes after the last line feed */ + private string $partial = ''; + + private function __construct( + private readonly Process $process, + private readonly InputStream $input, + ) {} + + /** + * Starts the server. + * + * @param string $script the server script + * @param array $env environment variables of the server, on top of this process's + */ + public static function start(string $script, array $env): self + { + $input = new InputStream(); + $process = new Process([PHP_BINARY, $script], null, $env, $input, self::TIMEOUT_SECONDS); + $process->start(); + + return new self($process, $input); + } + + /** + * Writes one message as one line on the server's stdin. + * + * @param array $message + */ + public function send(array $message): void + { + $this->input->write(json_encode($message, JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR) . "\n"); + $this->read(); + } + + /** Reads stdout until the answer (`result` or `error`) to request `$id` arrives; fails the test after `$timeout`. */ + public function receive(int|string $id, float $timeout = 5.0): \stdClass + { + $deadline = microtime(true) + $timeout; + while (true) { + $running = $this->process->isRunning(); // before the read: an ended server has nothing more to say + $this->read(); + foreach ($this->lines as $line) { + $message = $this->decode($line); + if ($message instanceof \stdClass && ($message->id ?? null) === $id + && (property_exists($message, 'result') || property_exists($message, 'error'))) { + return $message; + } + } + if (!$running || microtime(true) >= $deadline) { + Assert::fail(sprintf( + 'No answer to request %s within %.1f s. stdout: %s stderr: %s', + var_export($id, true), + $timeout, + implode("\n", $this->stdoutLines()), + $this->process->getErrorOutput(), + )); + } + usleep(self::POLL_MICROSECONDS); + } + } + + /** + * Every stdout line read so far, without its line feed; bytes after the last line feed come last. + * + * @return list + */ + public function stdoutLines(): array + { + return $this->partial === '' ? $this->lines : [...$this->lines, $this->partial]; + } + + /** Closes the server's stdin: the server reads EOF. */ + public function closeInput(): void + { + $this->input->close(); + $this->read(); + } + + /** Waits up to `$timeout` for the server to end; returns its exit code (128 + n after signal n), or null. */ + public function waitForExit(float $timeout): ?int + { + $deadline = microtime(true) + $timeout; + while ($this->process->isRunning()) { + $this->read(); + if (microtime(true) >= $deadline) { + return null; + } + usleep(self::POLL_MICROSECONDS); + } + $this->read(); + + return $this->process->getExitCode(); + } + + public function signal(int $signal): void + { + $this->process->signal($signal); + } + + public function process(): Process + { + return $this->process; + } + + /** Moves new stdout bytes into lines, writes pending stdin bytes, and stops the server after its timeout. */ + private function read(): void + { + if ($this->process->isRunning()) { + $this->process->checkTimeout(); + } + $chunk = $this->process->getIncrementalOutput(); + if ($chunk === '') { + return; + } + $parts = explode("\n", $this->partial . $chunk); + $this->partial = array_pop($parts); + array_push($this->lines, ...$parts); + } + + private function decode(string $line): mixed + { + try { + return Json::decode($line); + } catch (\JsonException) { + return null; // not JSON: the test's stdout check reports it + } + } +} From e6b554e71bd400c487801474d94a93b5f3951cc5 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 17:51:50 +0000 Subject: [PATCH 11/17] fix: send no duration for a float outside the int range PHP 8.5 warns when it casts such a float to int, and a CLI server prints that warning on STDOUT. A NaN, an infinity or an out-of-range float now leaves the duration out on PHP 8.3, 8.4 and 8.5 alike. --- src/Internal/Wire/WireMapper.php | 14 +++++++++++--- tests/Unit/Internal/Wire/WireMapperTest.php | 11 +++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Internal/Wire/WireMapper.php b/src/Internal/Wire/WireMapper.php index e48491b..01ca641 100644 --- a/src/Internal/Wire/WireMapper.php +++ b/src/Internal/Wire/WireMapper.php @@ -10,6 +10,9 @@ /** @internal */ final class WireMapper { + /** 2^63, the first float past `PHP_INT_MAX`; `-2^63` is `PHP_INT_MIN` itself. */ + private const float INT_RANGE_END = 9.223372036854775808E18; + /** * @throws \UnexpectedValueException when a free-form field is circular or nested too deeply */ @@ -83,12 +86,17 @@ private static function freeForm(mixed $value): ?\stdClass return is_array($wire) ? (object) $wire : null; } + /** + * A float duration is rounded. NaN, an infinity or a float outside the int range has no duration: casting it + * gives a meaningless int, and PHP 8.5 also prints a warning (on STDOUT in a CLI server). + */ private static function duration(int|float|null $duration): ?int { - if (is_float($duration)) { - return is_finite($duration) ? (int) round($duration) : null; + if (!is_float($duration)) { + return $duration; } + $rounded = round($duration); - return $duration; + return $rounded >= -self::INT_RANGE_END && $rounded < self::INT_RANGE_END ? (int) $rounded : null; // false for NaN } } diff --git a/tests/Unit/Internal/Wire/WireMapperTest.php b/tests/Unit/Internal/Wire/WireMapperTest.php index 534dfbd..8917af6 100644 --- a/tests/Unit/Internal/Wire/WireMapperTest.php +++ b/tests/Unit/Internal/Wire/WireMapperTest.php @@ -261,6 +261,17 @@ public function testDurationIsAnInteger(): void $this->assertFalse(property_exists($this->body($this->base()), 'duration')); } + /** PHP 8.5 warns when it casts a float outside the int range: such a duration is left out on every PHP version. */ + #[Group('ac-07-30')] + public function testADurationOutsideTheIntRangeIsLeftOut(): void + { + foreach ([1e19, -1e19, 9.223372036854775808E18, 1e30] as $duration) { + $this->assertFalse(property_exists($this->body(new Event(projectId: 'p', duration: $duration)), 'duration'), (string) $duration); + } + $this->assertSame(9_223_372_036_854_774_784, $this->body(new Event(projectId: 'p', duration: 9.223372036854774784E18))->duration, 'the last float below 2^63'); + $this->assertSame(PHP_INT_MIN, $this->body(new Event(projectId: 'p', duration: -9.223372036854775808E18))->duration, '-2^63 fits'); + } + #[Group('ac-07-31')] public function testMapperNeverShortensStrings(): void { From bcaa4a1ad7dbf5c55dae6d4e1cecea78f5833cc3 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 17:54:28 +0000 Subject: [PATCH 12/17] fix: dispatch nothing in a shutdown after the shared deadline A later shutdown() (the shutdown function after a signal drain that used the whole budget) still ran one pump: hooks for up to 5 waiting events and up to 5 transfers that the process abandons at once. Once the deadline of the first shutdown has passed, a later one only reports the events that still wait. --- src/Internal/Delivery/EventQueue.php | 19 +++++- .../Unit/Internal/Delivery/EventQueueTest.php | 64 +++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/src/Internal/Delivery/EventQueue.php b/src/Internal/Delivery/EventQueue.php index 9fd36c6..32fe5b9 100644 --- a/src/Internal/Delivery/EventQueue.php +++ b/src/Internal/Delivery/EventQueue.php @@ -154,13 +154,16 @@ public function flush(?float $timeoutSeconds = null): bool /** * Stops accepting events, then delivers for at most the shutdown timeout (TypeScript `destroy()`). The first call - * fixes the deadline; a later call delivers only until that same deadline. So a signal drain and the shutdown - * function after it, or a signal during the shutdown function's flush, share one budget. + * fixes the deadline; a later call delivers only until that same deadline, and dispatches nothing once it has + * passed. So a signal drain and the shutdown function after it, or a signal during the shutdown function's flush, + * share one budget. */ public function shutdown(): void { $this->accepting = false; - $this->deliverUntil(fn(): float => $this->shutdownDeadline ??= $this->transport->now() + $this->budget($this->shutdownTimeoutSeconds)); + if (!$this->shutdownDeadlinePassed()) { + $this->deliverUntil(fn(): float => $this->shutdownDeadline ??= $this->transport->now() + $this->budget($this->shutdownTimeoutSeconds)); + } if (!$this->waiting->isEmpty()) { Log::write(sprintf('Shutting down with %d events still in queue', $this->waiting->count())); } @@ -184,6 +187,16 @@ public function stats(): array return ['queueLength' => $this->waiting->count(), 'activeRequests' => $this->transport->inFlight()]; } + /** True when an earlier `shutdown()` fixed the deadline and that deadline has passed. */ + private function shutdownDeadlinePassed(): bool + { + try { + return $this->shutdownDeadline !== null && $this->transport->now() >= $this->shutdownDeadline; + } catch (\Throwable) { + return false; // the delivery loop logs the transport's failure + } + } + /** A NaN or negative timeout means "no time left". max() is no guard: max(0.0, NAN) is NaN on PHP 8.4+. */ private function budget(float $timeoutSeconds): float { diff --git a/tests/Unit/Internal/Delivery/EventQueueTest.php b/tests/Unit/Internal/Delivery/EventQueueTest.php index d9ffc8e..e996dcd 100644 --- a/tests/Unit/Internal/Delivery/EventQueueTest.php +++ b/tests/Unit/Internal/Delivery/EventQueueTest.php @@ -523,6 +523,70 @@ public function testLaterShutdownsShareTheFirstShutdownsBudget(): void $this->assertSame(array_fill(0, 3, 'Shutting down with 1 events still in queue'), $this->logger->messages()); } + /** The shutdown function after a signal drain that used the whole budget: no hook runs, no transfer starts. */ + #[Group('ac-03-9')] + public function testAShutdownAfterTheSharedDeadlineDispatchesNothing(): void + { + $transport = new RecordingTransport(complete: false); + $queue = new EventQueue(transport: $transport); + for ($i = 0; $i < 7; ++$i) { + $queue->add(new QueuedEvent($this->event())); + } + $queue->shutdown(); // 5 in delivery until the deadline, 2 waiting + $transport->complete = true; + $transport->pump(); // the 5 slots are free again + + $queue->finalShutdown(); + $queue->shutdown(); + + $this->assertCount(5, $transport->deliveries, 'nothing dispatched after the deadline'); + $this->assertSame(1, $transport->waits); + $this->assertSame(array_fill(0, 3, 'Shutting down with 2 events still in queue'), $this->logger->messages()); + } + + /** A later shutdown before the deadline still delivers, until that deadline. */ + #[Group('ac-03-9')] + public function testAShutdownBeforeTheSharedDeadlineStillDelivers(): void + { + $transport = new RecordingTransport(); + $queue = new EventQueue(transport: $transport); + $breakTransport = static function () use ($transport): void { + $transport->failure = new \RuntimeException('down'); + }; + $queue->add(new QueuedEvent($this->event('first'), pending: [$breakTransport])); + $queue->add(new QueuedEvent($this->event('second'))); + $queue->shutdown(); // fixes the deadline at 5 s; the first hook breaks the transport at 0 s + $transport->failure = null; + + $queue->shutdown(); + + $this->assertEqualsWithDelta(0.0, $transport->now(), PHP_FLOAT_EPSILON, 'still before the deadline'); + $this->assertSame(['second'], $this->resourceNames($transport)); + $this->assertSame(['queueLength' => 0, 'activeRequests' => 0], $queue->stats()); + } + + public function testALaterShutdownWithAFailingClockNeverThrows(): void + { + $transport = new RecordingTransport(); + $queue = new EventQueue(transport: $transport); + $breakTransport = static function () use ($transport): void { + $transport->failure = new \RuntimeException('down'); + }; + $queue->add(new QueuedEvent($this->event('first'), pending: [$breakTransport])); + $queue->add(new QueuedEvent($this->event('second'))); + $queue->shutdown(); + $this->logger->records = []; + + $queue->shutdown(); // the deadline check cannot read the clock: the delivery loop logs the failure + + $this->assertSame([ + self::GAVE_UP . 'RuntimeException: down', + self::GAVE_UP . 'RuntimeException: down', + 'Shutting down with 1 events still in queue', + ], $this->logger->messages()); + $this->assertSame([], $transport->deliveries); + } + public function testShutdownDeliversEverything(): void { $transport = new RecordingTransport(); From 9b4bebbcf3e943cca837306259fde19659cbaabf Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 17:54:42 +0000 Subject: [PATCH 13/17] docs: say that the signal drain sees only an ignore set in PHP An ignore inherited from the parent process reads as SIG_DFL in PHP, so AgentCat installs its drain for that signal. No behavior change. --- src/Internal/Delivery/SignalDrain.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Internal/Delivery/SignalDrain.php b/src/Internal/Delivery/SignalDrain.php index e7d1e2e..bae755a 100644 --- a/src/Internal/Delivery/SignalDrain.php +++ b/src/Internal/Delivery/SignalDrain.php @@ -10,8 +10,9 @@ * * The global queue calls `install()` when it gets an event, so nothing happens at autoload. It installs once, only * under the CLI SAPI with ext-pcntl, and keeps the handler each signal had before. A signal that the application - * ignores (`SIG_IGN`) stays ignored. When it installs a handler, `install()` also turns on - * `pcntl_async_signals(true)`, which is process wide: from then on every PHP signal handler of the process runs + * ignores in PHP (`pcntl_signal(…, SIG_IGN)`) stays ignored; an ignore inherited from the parent process is not + * visible to PHP, so AgentCat installs its drain for that signal. When it installs a handler, `install()` also turns + * on `pcntl_async_signals(true)`, which is process wide: from then on every PHP signal handler of the process runs * between two opcodes, not at `pcntl_signal_dispatch()` or a tick. * * Each handler runs once (TypeScript `process.once`). It first gives the signal back to the earlier handler, so a From 38f362b42bd9e3f9ead64ecb0b7a6f4a422904d3 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 17:55:05 +0000 Subject: [PATCH 14/17] test: drop the ac-01-69 tag from the fake's test-mode tests AC-69 is "a high-level server whose low-level server is tracked is accepted". These tests cover D31, D44 and ruling R4, which have no acceptance criterion; PublishCustomEventTest keeps the AC-69 tag. --- tests/Unit/Testing/AgentCatFakeTest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/Unit/Testing/AgentCatFakeTest.php b/tests/Unit/Testing/AgentCatFakeTest.php index 6818caa..d41ce5d 100644 --- a/tests/Unit/Testing/AgentCatFakeTest.php +++ b/tests/Unit/Testing/AgentCatFakeTest.php @@ -22,7 +22,6 @@ use AgentCat\ToolCall; use Mcp\Server\Builder; use PHPUnit\Framework\Assert; -use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; /** Customer style: everything goes through `AgentCat`. */ @@ -52,7 +51,7 @@ protected function tearDown(): void $this->environment->restore(); } - #[Group('ac-01-69')] + /** D31, Review Focus 1: PHPUnit is loaded and `AGENTCAT_ENABLED` is unset. */ public function testWithoutFakeTheSdkStaysOffInATestSuite(): void { $builder = TrackingFixture::builder(); @@ -144,7 +143,7 @@ public function testACustomEventIsNotAToolCall(): void ); } - #[Group('ac-01-69')] + /** D31: the fake records and sends nothing. */ public function testNothingGoesToTheNetwork(): void { putenv('AGENTCAT_API_URL=' . MockCollector::closedPortUrl()); @@ -170,7 +169,7 @@ public function testTelemetryOnlyServersAreRecordedToo(): void $fake->assertTrackedCount(1); } - #[Group('ac-01-69')] + /** Ruling R4: `AGENTCAT_ENABLED=false` (D44) does not turn the fake off. */ public function testTheFakeRecordsWhateverAgentcatEnabledSays(): void { putenv('AGENTCAT_ENABLED=false'); From a4c381a0aa27466dc35c1984521b4984273e2e44 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 17:58:24 +0000 Subject: [PATCH 15/17] fix: keep AgentCat's log in the fake while a fake is the shared client Under AgentCat::fake(), track() and publishCustomEvent() wrote to $HOME/agentcat.log on every developer machine and CI runner, and a Builder's PSR-3 logger tracked under the fake took every later AgentCat line of the suite. While a fake's client is the shared client, AgentCat now logs to that fake only (AgentCatFake::logs()) and adopts no Builder logger. AgentCat::setClient() that replaces it brings back the routing from before the first fake(). --- src/AgentCat.php | 13 ++- src/Internal/Logging/Log.php | 26 ++++++ src/Testing/AgentCatFake.php | 21 ++++- src/Testing/Internal/FakeLog.php | 52 ++++++++++++ tests/Unit/Internal/Logging/LogTest.php | 49 +++++++++++ tests/Unit/Testing/AgentCatFakeTest.php | 107 +++++++++++++++++++++++- 6 files changed, 265 insertions(+), 3 deletions(-) create mode 100644 src/Testing/Internal/FakeLog.php diff --git a/src/AgentCat.php b/src/AgentCat.php index d521dfc..ddc2a36 100644 --- a/src/AgentCat.php +++ b/src/AgentCat.php @@ -4,7 +4,9 @@ namespace AgentCat; +use AgentCat\Internal\Logging\Log; use AgentCat\Testing\AgentCatFake; +use AgentCat\Testing\Internal\FakeLog; /** * The AgentCat entry point: `AgentCat::track($builder, 'proj_…')` before `$builder->build()`. @@ -102,6 +104,9 @@ public static function shutdown(): void * Starts recording for your test suite: installs a client that records every event and sends nothing, and * returns the fake to assert on. Restore the real client with `AgentCat::setClient(null)` in `tearDown()`. * + * While the fake is the shared client, AgentCat logs to the fake's `logs()` only, and adopts no Builder's PSR-3 + * logger. `setClient()` brings back the log routing from before the first `fake()`. + * * Call it before the server is tracked: a Builder tracked earlier keeps the client that tracked it. * * The fake records whatever `AGENTCAT_ENABLED` says, so a CI that sets `AGENTCAT_ENABLED=false` still works. Test @@ -123,10 +128,16 @@ public static function client(): Client /** * Replaces the shared client, for example with one from your dependency-injection container. Null goes back to - * a new default client. + * a new default client. Replacing the client of a fake brings back the log routing from before the first + * `fake()`. */ public static function setClient(?Client $client): void { self::$client = $client; + Log::endDiversion(); + $fakeLog = $client instanceof Client ? FakeLog::of($client) : null; + if ($fakeLog instanceof FakeLog) { + Log::divert($fakeLog); // a fake's client: AgentCat logs to the fake only + } } } diff --git a/src/Internal/Logging/Log.php b/src/Internal/Logging/Log.php index 4535dde..893ab87 100644 --- a/src/Internal/Logging/Log.php +++ b/src/Internal/Logging/Log.php @@ -15,6 +15,9 @@ final class Log /** True once `setLogger()` or `adopt()` chose the logger; the lazy file fallback does not count. */ private static bool $chosen = false; + /** @var array{logger: ?LoggerInterface, chosen: bool}|null the routing `divert()` replaced; null without one */ + private static ?array $beforeDiversion = null; + /** A non-null logger wins over every later `adopt()`; null goes back to the file fallback. */ public static function setLogger(?LoggerInterface $logger): void { @@ -35,11 +38,34 @@ public static function adopt(LoggerInterface $logger): void self::$chosen = true; } + /** + * Sends every later line to `$logger` only, until `endDiversion()`; `adopt()` changes nothing meanwhile. A second + * `divert()` changes the target and keeps the routing from before the first one. `AgentCat::fake()` uses it. + */ + public static function divert(LoggerInterface $logger): void + { + self::$beforeDiversion ??= ['logger' => self::$logger, 'chosen' => self::$chosen]; + self::$logger = $logger; + self::$chosen = true; + } + + /** Puts back the routing from before the first `divert()`: the logger, and whether `adopt()` can choose one. */ + public static function endDiversion(): void + { + if (self::$beforeDiversion === null) { + return; + } + self::$logger = self::$beforeDiversion['logger']; + self::$chosen = self::$beforeDiversion['chosen']; + self::$beforeDiversion = null; + } + /** Tests only. */ public static function reset(): void { self::$logger = null; self::$chosen = false; + self::$beforeDiversion = null; } /** Never throws. The level comes from the line prefix only. */ diff --git a/src/Testing/AgentCatFake.php b/src/Testing/AgentCatFake.php index afc5eff..2fd8ddb 100644 --- a/src/Testing/AgentCatFake.php +++ b/src/Testing/AgentCatFake.php @@ -9,13 +9,15 @@ use AgentCat\Internal\ClientSeams; use AgentCat\Internal\Constants; use AgentCat\Internal\Delivery\EventQueue; +use AgentCat\Testing\Internal\FakeLog; use AgentCat\Testing\Internal\InstantTransport; use AgentCat\Testing\Internal\RecordingSink; use PHPUnit\Framework\Assert; /** * The test double of AgentCat for your own test suite: `AgentCat::fake()` returns one. It records every event - * (after hooks, redaction and truncation) and sends nothing over the network. + * (after hooks, redaction and truncation) and sends nothing over the network. While it is the shared client, AgentCat + * logs to its `logs()` only: not to `$HOME/agentcat.log`, and not to the PSR-3 logger of a Builder. * * Assertions use PHPUnit when it is loaded and throw `\AssertionError` otherwise. */ @@ -25,6 +27,8 @@ private Client $client; + private FakeLog $log; + public function __construct() { $this->sink = new RecordingSink(); @@ -32,6 +36,7 @@ public function __construct() new EventQueue(transport: new InstantTransport(), exporters: $this->sink), static fn(): bool => true, )); + $this->log = FakeLog::attach($this->client); } /** The client that records. `AgentCat::fake()` already installed it with `AgentCat::setClient()`. */ @@ -53,6 +58,20 @@ public function events(): array return $this->sink->events; } + /** + * The messages AgentCat logged while this fake was the shared client, in order and without the timestamp of the + * log file: useful to see hook errors and setup warnings in a test. Calling it runs the hooks that are still + * waiting, so their errors are in the list. + * + * @return list + */ + public function logs(): array + { + $this->client->flush(); + + return $this->log->messages; + } + /** * Asserts that at least one tool call (`mcp:tools/call`) was tracked, and one matches `$filter` when given. * diff --git a/src/Testing/Internal/FakeLog.php b/src/Testing/Internal/FakeLog.php new file mode 100644 index 0000000..61ff2bf --- /dev/null +++ b/src/Testing/Internal/FakeLog.php @@ -0,0 +1,52 @@ +|null */ + private static ?\WeakMap $byClient = null; + + /** @var list */ + public array $messages = []; + + /** A new, empty log for the fake that owns `$client`. */ + public static function attach(Client $client): self + { + return self::byClient()[$client] = new self(); + } + + /** The log of a fake's client; null for any other client. */ + public static function of(Client $client): ?self + { + return self::byClient()[$client] ?? null; + } + + /** @param mixed[] $context */ + #[\Override] + public function log($level, string|\Stringable $message, array $context = []): void + { + $this->messages[] = (string) $message; + } + + /** @return \WeakMap */ + private static function byClient(): \WeakMap + { + return self::$byClient ??= new \WeakMap(); + } +} diff --git a/tests/Unit/Internal/Logging/LogTest.php b/tests/Unit/Internal/Logging/LogTest.php index 3e6f9f6..5224192 100644 --- a/tests/Unit/Internal/Logging/LogTest.php +++ b/tests/Unit/Internal/Logging/LogTest.php @@ -163,6 +163,55 @@ public function testResetForgetsTheAdoptedLogger(): void $this->assertSame(['hello'], $second->messages()); } + public function testADiversionTakesEveryLineAndBlocksAdoption(): void + { + $set = new MemoryLogger(); + Log::setLogger($set); + $first = new MemoryLogger(); + $second = new MemoryLogger(); + + Log::divert($first); + Log::write('one'); + Log::divert($second); + Log::adopt(new MemoryLogger()); + Log::write('two'); + Log::endDiversion(); + Log::write('three'); + + $this->assertSame(['one'], $first->messages()); + $this->assertSame(['two'], $second->messages()); + $this->assertSame(['three'], $set->messages(), 'the routing from before the first diversion'); + } + + public function testEndingADiversionReopensAdoption(): void + { + Log::divert(new MemoryLogger()); + Log::endDiversion(); + Log::endDiversion(); // without a diversion: nothing changes + $adopted = new MemoryLogger(); + + Log::adopt($adopted); + Log::write('hello'); + + $this->assertSame(['hello'], $adopted->messages()); + } + + public function testResetForgetsADiversion(): void + { + $set = new MemoryLogger(); + Log::setLogger($set); + Log::divert(new MemoryLogger()); + Log::reset(); + $adopted = new MemoryLogger(); + Log::adopt($adopted); + + Log::endDiversion(); + Log::write('hello'); + + $this->assertSame(['hello'], $adopted->messages(), 'nothing to end after reset()'); + $this->assertSame([], $set->messages()); + } + public function testTheFileFallbackDoesNotStopALaterAdoption(): void { $home = sys_get_temp_dir() . '/agentcat-logtest-' . bin2hex(random_bytes(6)); diff --git a/tests/Unit/Testing/AgentCatFakeTest.php b/tests/Unit/Testing/AgentCatFakeTest.php index d41ce5d..4014a0d 100644 --- a/tests/Unit/Testing/AgentCatFakeTest.php +++ b/tests/Unit/Testing/AgentCatFakeTest.php @@ -5,9 +5,11 @@ namespace AgentCat\Tests\Unit\Testing; use AgentCat\AgentCat; +use AgentCat\Client; use AgentCat\CustomEvent; use AgentCat\Event; use AgentCat\Identity; +use AgentCat\Internal\ClientSeams; use AgentCat\Internal\Delivery\EventQueue; use AgentCat\Internal\Engine\TrackedServers; use AgentCat\Internal\Logging\Log; @@ -18,6 +20,7 @@ use AgentCat\Tests\Support\McpSdk\Tools; use AgentCat\Tests\Support\McpSdk\TrackingFixture; use AgentCat\Tests\Support\MemoryLogger; +use AgentCat\Tests\Support\RecordingTransport; use AgentCat\Tests\Support\Thrown; use AgentCat\ToolCall; use Mcp\Server\Builder; @@ -33,12 +36,18 @@ final class AgentCatFakeTest extends TestCase private MemoryLogger $logger; + /** The test's own `$HOME`, from `freshHome()`; removed in `tearDown()`. */ + private ?string $freshHome = null; + + private string $home = ''; + #[\Override] protected function setUp(): void { $this->environment = Environment::clear('AGENTCAT_ENABLED', 'AGENTCAT_API_URL', 'MCPCAT_API_URL', 'AGENTCAT_PROJECT_ID'); $this->logger = new MemoryLogger(); Log::setLogger($this->logger); + $this->home = $this->home(); } #[\Override] @@ -49,6 +58,13 @@ protected function tearDown(): void EventQueue::setGlobal(null); Log::reset(); $this->environment->restore(); + putenv('HOME=' . $this->home); + if ($this->freshHome !== null) { + if (is_file($this->freshHome . '/agentcat.log')) { + unlink($this->freshHome . '/agentcat.log'); + } + rmdir($this->freshHome); + } } /** D31, Review Focus 1: PHPUnit is loaded and `AGENTCAT_ENABLED` is unset. */ @@ -151,10 +167,89 @@ public function testNothingGoesToTheNetwork(): void $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); $fake->assertTrackedCount(1); - $this->assertSame([], array_filter($this->logger->messages(), static fn(string $m): bool => str_contains($m, 'Failed to send event'))); + $this->assertNotSame([], $fake->logs()); + $this->assertSame([], array_filter($fake->logs(), static fn(string $m): bool => str_contains($m, 'Failed to send event'))); putenv('AGENTCAT_API_URL'); } + /** A customer suite chose no logger: under the fake, nothing goes to `$HOME/agentcat.log`. */ + public function testAFakeOnlyTestWritesNoLogFile(): void + { + Log::reset(); + $home = $this->freshHome(); + $fake = AgentCat::fake(); + + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + AgentCat::publishCustomEvent('s-1', self::PROJECT, new CustomEvent(message: 'm')); + + $fake->assertTrackedCount(1); + $this->assertFileDoesNotExist($home . '/agentcat.log'); + $this->assertSame([ + 'AgentCat setup started | project ' . self::PROJECT . ' | sdk mcp-sdk/builder | signals mcpSdkBuilder', + 'AgentCat setup complete | project ' . self::PROJECT . ' | tracing=true context=true reportMissing=true exporters=0', + 'Rebuilt injection registries on demand (tools/call before tools/list on this instance)', + "Published custom event for session s-1 with type 'agentcat:custom'", + ], $fake->logs()); + } + + public function testLogsHoldTheLinesOfTheTimeTheFakeWasTheSharedClient(): void + { + Log::write('before'); + $first = AgentCat::fake(); + Log::write('first'); + $second = AgentCat::fake(); + Log::write('second'); + AgentCat::setClient(null); + Log::write('after'); + + $this->assertSame(['first'], $first->logs()); + $this->assertSame(['second'], $second->logs(), 'a second fake starts its own logs'); + $this->assertSame(['before', 'after'], $this->logger->messages(), 'the routing from before the first fake is back'); + } + + /** D38: the first Builder logger adopted gets every later line, but not while a fake is the shared client. */ + public function testABuildersLoggerTrackedUnderTheFakeGetsNoAgentCatLine(): void + { + Log::reset(); + $home = $this->freshHome(); + $customers = new MemoryLogger(); + $fake = AgentCat::fake(); + $builder = TrackingFixture::builder()->setLogger($customers); + + AgentCat::track($builder, self::PROJECT); + AgentCat::publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + AgentCat::setClient(null); + + $this->assertSame([], $customers->messages()); + $this->assertCount(3, $fake->logs()); + $later = new MemoryLogger(); + Log::adopt($later); + Log::write('after the fake'); + $this->assertSame(['after the fake'], $later->messages(), 'adoption resumes after the fake'); + $this->assertFileDoesNotExist($home . '/agentcat.log'); + } + + public function testReplacingTheFakeByAnotherClientRestoresTheRouting(): void + { + $fake = AgentCat::fake(); + AgentCat::setClient(new Client(ClientSeams::of(new EventQueue(transport: new RecordingTransport()), static fn(): bool => true))); + + Log::write('after'); + + $this->assertSame([], $fake->logs()); + $this->assertSame(['after'], $this->logger->messages()); + } + + public function testLogsRunTheWaitingHooksFirst(): void + { + $fake = AgentCat::fake(); + $harness = $this->trackedHarness(new Options(eventTags: static fn(): never => throw new \RuntimeException('tags down'))); + + $harness->callTool('echo', ['msg' => 'hi']); + + $this->assertContains('eventTags callback error: RuntimeException: tags down', $fake->logs()); + } + public function testTelemetryOnlyServersAreRecordedToo(): void { $fake = AgentCat::fake(); @@ -252,6 +347,16 @@ private function failureOf(\Closure $code): string return $failure->getMessage(); } + /** Points `$HOME` at a new, empty directory for this test. */ + private function freshHome(): string + { + $this->freshHome = $this->home . '/fake-' . bin2hex(random_bytes(4)); + mkdir($this->freshHome); + putenv('HOME=' . $this->freshHome); + + return $this->freshHome; + } + private function home(): string { $home = getenv('HOME'); From daf426f3bb1db041edc7787d7180aceeb743fae8 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 18:01:18 +0000 Subject: [PATCH 16/17] fix: log instead of throwing for a server tracked while AgentCat was off In a test environment, a server that the application tracks in setUp(), before the test calls AgentCat::fake(), is not tracked at all. Code under test that then published a custom event for it got the TypeScript "Server is not tracked" exception. track() now records, process wide, that it ran while AgentCat was off; publishCustomEvent() then logs a warning that says to call fake() first (the failed-track line keeps precedence), and assertTracked()/assertTrackedCount() add the same hint when no tool call was recorded. The fake() PHPDoc says to call it before the application builds its servers. --- src/AgentCat.php | 12 ++-- src/Client.php | 14 ++++- src/Internal/Engine/TrackedServers.php | 28 ++++++++- src/Testing/AgentCatFake.php | 19 +++++- tests/Integration/ClientTrackTest.php | 2 + tests/Integration/PublishCustomEventTest.php | 32 ++++++++++ tests/Unit/AgentCatTest.php | 2 + tests/Unit/ClientTest.php | 2 + .../Internal/Engine/TrackedServersTest.php | 14 +++++ tests/Unit/Testing/AgentCatFakeTest.php | 62 +++++++++++++++++++ 10 files changed, 174 insertions(+), 13 deletions(-) diff --git a/src/AgentCat.php b/src/AgentCat.php index ddc2a36..00492b2 100644 --- a/src/AgentCat.php +++ b/src/AgentCat.php @@ -59,9 +59,9 @@ public static function track(object $server, ?string $projectId = null, ?Options * wins over it. No server or SDK fields and no hooks apply. * * The event is queued, not sent: AgentCat delivers it like any other event. Apart from the two errors below, it - * never throws: a failure is logged. When a `track()` in this process could not track a server, a server that is - * not tracked logs a warning instead of the second error. In test environments it validates `$projectId` and then - * does nothing, like `track()` (see `AGENTCAT_ENABLED` there). + * never throws: a failure is logged. When a `track()` in this process could not track a server, or ran while + * AgentCat was off, a server that is not tracked logs a warning instead of the second error. In test environments + * it validates `$projectId` and then does nothing, like `track()` (see `AGENTCAT_ENABLED` there). * * In a file without `declare(strict_types=1)`, PHP turns an int, float or bool first argument into a session ID * string. @@ -104,11 +104,13 @@ public static function shutdown(): void * Starts recording for your test suite: installs a client that records every event and sends nothing, and * returns the fake to assert on. Restore the real client with `AgentCat::setClient(null)` in `tearDown()`. * + * Call it first, before the application builds its servers. In a test environment, a server tracked before + * `fake()` is not tracked at all: no tool call of it is recorded. With `AGENTCAT_ENABLED=true`, a server tracked + * before `fake()` keeps the client that tracked it. + * * While the fake is the shared client, AgentCat logs to the fake's `logs()` only, and adopts no Builder's PSR-3 * logger. `setClient()` brings back the log routing from before the first `fake()`. * - * Call it before the server is tracked: a Builder tracked earlier keeps the client that tracked it. - * * The fake records whatever `AGENTCAT_ENABLED` says, so a CI that sets `AGENTCAT_ENABLED=false` still works. Test * environments without `fake()` stay silent: `track()` does nothing there. */ diff --git a/src/Client.php b/src/Client.php index a67d9f7..66d83cb 100644 --- a/src/Client.php +++ b/src/Client.php @@ -76,6 +76,8 @@ public function track(object $server, ?string $projectId = null, ?Options $optio { try { if (!($this->enabled)()) { + TrackedServers::markSkipped(); // a fake() after this finds no tracked server: it must not throw then + return $server; // D44 AGENTCAT_ENABLED, else D31: nothing in test environments unless fake() } $options ??= new Options(); @@ -143,9 +145,9 @@ public function track(object $server, ?string $projectId = null, ?Options $optio * wins over it. No server or SDK fields and no hooks apply. * * The event is queued, not sent: AgentCat delivers it like any other event. Apart from the two errors below, it - * never throws: a failure is logged. When a `track()` in this process could not track a server, a server that is - * not tracked logs a warning instead of the second error. In test environments it validates `$projectId` and then - * does nothing, like `track()` (see `AGENTCAT_ENABLED` there). + * never throws: a failure is logged. When a `track()` in this process could not track a server, or ran while + * AgentCat was off, a server that is not tracked logs a warning instead of the second error. In test environments + * it validates `$projectId` and then does nothing, like `track()` (see `AGENTCAT_ENABLED` there). * * In a file without `declare(strict_types=1)`, PHP turns an int, float or bool first argument into a session ID * string. @@ -178,6 +180,12 @@ public function publishCustomEvent(object|string $serverOrSessionId, ?string $pr return; } + if (TrackedServers::skipped()) { + // a test that tracked the server before fake(): its code must not get the exception + Log::write('Warning: Failed to publish custom event - the server is not tracked (track() ran while AgentCat was off; call AgentCat::fake() before track())'); + + return; + } throw new \InvalidArgumentException('Server is not tracked. Please call agentcat.track() first or provide a session ID string.'); } diff --git a/src/Internal/Engine/TrackedServers.php b/src/Internal/Engine/TrackedServers.php index ee1db45..24588e5 100644 --- a/src/Internal/Engine/TrackedServers.php +++ b/src/Internal/Engine/TrackedServers.php @@ -6,8 +6,8 @@ /** * Every server tracked in this process, whichever `Client` tracked it. The first `track()` of a server owns it, and - * a later one from any client changes nothing. An entry goes away with its server. A process-wide flag also records - * that a server could not be tracked or read. + * a later one from any client changes nothing. An entry goes away with its server. Two process-wide flags also record + * that a server could not be tracked or read, and that `track()` ran while AgentCat was off. * * @internal */ @@ -18,6 +18,8 @@ final class TrackedServers private static bool $incomplete = false; + private static bool $skipped = false; + /** The tracking state of the server, or null when no client tracks it. */ public static function get(object $server): ?TrackedServer { @@ -50,6 +52,28 @@ public static function resetIncomplete(): void self::$incomplete = false; } + /** + * Records that a `track()` returned without tracking because AgentCat was off (D31, D44). A server tracked like + * that is not tracked at all: once `AgentCat::fake()` turns a client on, `publishCustomEvent()` logs for it + * instead of throwing, and the fake's assertions say why no tool call was recorded. + */ + public static function markSkipped(): void + { + self::$skipped = true; + } + + /** True once `markSkipped()` ran in this process. */ + public static function skipped(): bool + { + return self::$skipped; + } + + /** Tests only: forgets `markSkipped()`. The tracked servers stay. */ + public static function resetSkipped(): void + { + self::$skipped = false; + } + /** @return \WeakMap */ private static function servers(): \WeakMap { diff --git a/src/Testing/AgentCatFake.php b/src/Testing/AgentCatFake.php index 2fd8ddb..4ed911e 100644 --- a/src/Testing/AgentCatFake.php +++ b/src/Testing/AgentCatFake.php @@ -9,6 +9,7 @@ use AgentCat\Internal\ClientSeams; use AgentCat\Internal\Constants; use AgentCat\Internal\Delivery\EventQueue; +use AgentCat\Internal\Engine\TrackedServers; use AgentCat\Testing\Internal\FakeLog; use AgentCat\Testing\Internal\InstantTransport; use AgentCat\Testing\Internal\RecordingSink; @@ -79,7 +80,8 @@ public function logs(): array */ public function assertTracked(?\Closure $filter = null): void { - $this->check($this->matching($this->ofType(Constants::TOOLS_CALL_EVENT_TYPE), $filter) !== [], 'Failed asserting that a tool call matching the filter was tracked.'); + $toolCalls = $this->ofType(Constants::TOOLS_CALL_EVENT_TYPE); + $this->check($this->matching($toolCalls, $filter) !== [], $this->hinted('Failed asserting that a tool call matching the filter was tracked.', $toolCalls)); } /** Asserts that no event of any type was recorded. */ @@ -92,8 +94,9 @@ public function assertNothingTracked(): void /** Asserts the number of tracked tool calls (`mcp:tools/call`); custom events do not count. */ public function assertTrackedCount(int $count): void { - $actual = count($this->ofType(Constants::TOOLS_CALL_EVENT_TYPE)); - $this->check($actual === $count, sprintf('Failed asserting that %d tool call(s) were tracked: %d were.', $count, $actual)); + $toolCalls = $this->ofType(Constants::TOOLS_CALL_EVENT_TYPE); + $actual = count($toolCalls); + $this->check($actual === $count, $this->hinted(sprintf('Failed asserting that %d tool call(s) were tracked: %d were.', $count, $actual), $toolCalls)); } /** @@ -106,6 +109,16 @@ public function assertCustomEventPublished(?\Closure $filter = null): void $this->check($this->matching($this->ofType(Constants::CUSTOM_EVENT_TYPE), $filter) !== [], 'Failed asserting that a custom event matching the filter was published.'); } + /** + * Adds why no tool call may have been recorded: a server tracked while AgentCat was off is not tracked at all. + * + * @param array $toolCalls + */ + private function hinted(string $message, array $toolCalls): string + { + return $toolCalls === [] && TrackedServers::skipped() ? $message . ' A server was tracked while AgentCat was off: call AgentCat::fake() before track().' : $message; + } + /** @return array */ private function ofType(string $eventType): array { diff --git a/tests/Integration/ClientTrackTest.php b/tests/Integration/ClientTrackTest.php index 822b7f2..9d8f3c4 100644 --- a/tests/Integration/ClientTrackTest.php +++ b/tests/Integration/ClientTrackTest.php @@ -69,6 +69,7 @@ protected function setUp(): void { $this->environment = Environment::clear('AGENTCAT_PROJECT_ID', 'AGENTCAT_API_URL', 'MCPCAT_API_URL', 'AGENTCAT_ENABLED', 'APP_ENV'); TrackedServers::resetIncomplete(); + TrackedServers::resetSkipped(); $this->logger = new MemoryLogger(); Log::setLogger($this->logger); $this->transport = new RecordingTransport(); @@ -81,6 +82,7 @@ protected function setUp(): void protected function tearDown(): void { TrackedServers::resetIncomplete(); // a failed track() marks the process + TrackedServers::resetSkipped(); // so does a track() while AgentCat is off Log::reset(); $this->environment->restore(); } diff --git a/tests/Integration/PublishCustomEventTest.php b/tests/Integration/PublishCustomEventTest.php index e487c23..459d33f 100644 --- a/tests/Integration/PublishCustomEventTest.php +++ b/tests/Integration/PublishCustomEventTest.php @@ -46,6 +46,8 @@ final class PublishCustomEventTest extends TestCase private const string NOT_TRACKED_AFTER_FAILURE = 'Warning: Failed to publish custom event - the server is not tracked (see the earlier AgentCat log line)'; + private const string NOT_TRACKED_WHILE_OFF = 'Warning: Failed to publish custom event - the server is not tracked (track() ran while AgentCat was off; call AgentCat::fake() before track())'; + private Environment $environment; private MemoryLogger $logger; @@ -69,12 +71,14 @@ protected function setUp(): void $this->queue = new EventQueue(transport: $this->transport, exporters: $this->exporter); $this->client = new Client(ClientSeams::of($this->queue, static fn(): bool => true)); TrackedServers::resetIncomplete(); // no failed track() before this test + TrackedServers::resetSkipped(); // and no track() while AgentCat was off } #[\Override] protected function tearDown(): void { TrackedServers::resetIncomplete(); + TrackedServers::resetSkipped(); Log::reset(); $this->environment->restore(); } @@ -498,6 +502,34 @@ public function testAfterAFailedTrackAnUntrackedServerLogsInsteadOfThrowing(\Clo $this->assertSame(self::NO_PROJECT, $error->getMessage(), 'the project ID is still checked first'); } + /** A server tracked while AgentCat was off (test mode, D31) was not tracked at all: an enabled client logs. */ + public function testAfterATrackWhileOffAnUntrackedServerLogsInsteadOfThrowing(): void + { + $off = new Client(ClientSeams::of($this->queue, static fn(): bool => false)); + $builder = TrackingFixture::builder(); + $off->track($builder, self::PROJECT); + + $this->client->publishCustomEvent($builder, self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + $this->client->publishCustomEvent($builder->build(), self::PROJECT, new CustomEvent(sessionId: 'ses_1')); + + $this->assertNothingQueued(); + $line = ['level' => LogLevel::WARNING, 'message' => self::NOT_TRACKED_WHILE_OFF]; + $this->assertSame([$line, $line], $this->logger->records, 'one line per call'); + $error = Thrown::by(\InvalidArgumentException::class, fn() => $this->client->publishCustomEvent($builder, null)); + $this->assertSame(self::NO_PROJECT, $error->getMessage(), 'the project ID is still checked first'); + } + + public function testTheFailedTrackLineWinsOverTheTrackWhileOffLine(): void + { + (new Client(ClientSeams::of($this->queue, static fn(): bool => false)))->track(TrackingFixture::builder(), self::PROJECT); + $this->client->track(new \stdClass(), self::PROJECT); // fails: marks the process incomplete + $this->logger->records = []; + + $this->client->publishCustomEvent(TrackingFixture::builder(), self::PROJECT); + + $this->assertSame([self::NOT_TRACKED_AFTER_FAILURE], $this->logger->messages()); + } + #[Group('ac-01-68')] public function testAfterASuccessfulTrackAnUntrackedServerStillThrows(): void { diff --git a/tests/Unit/AgentCatTest.php b/tests/Unit/AgentCatTest.php index 8f04c3f..1893b81 100644 --- a/tests/Unit/AgentCatTest.php +++ b/tests/Unit/AgentCatTest.php @@ -45,12 +45,14 @@ protected function setUp(): void $this->queue = new EventQueue(transport: $this->transport); $this->client = new Client(ClientSeams::of($this->queue, static fn(): bool => true)); TrackedServers::resetIncomplete(); + TrackedServers::resetSkipped(); } #[\Override] protected function tearDown(): void { TrackedServers::resetIncomplete(); // a failed track() marks the process + TrackedServers::resetSkipped(); // so does a track() while AgentCat is off AgentCat::setClient(null); EventQueue::setGlobal(null); // N9: the default client of setClient(null) builds the global queue Log::reset(); diff --git a/tests/Unit/ClientTest.php b/tests/Unit/ClientTest.php index 2e6862b..0653908 100644 --- a/tests/Unit/ClientTest.php +++ b/tests/Unit/ClientTest.php @@ -7,6 +7,7 @@ use AgentCat\Client; use AgentCat\Event; use AgentCat\Internal\Delivery\EventQueue; +use AgentCat\Internal\Engine\TrackedServers; use AgentCat\Internal\Logging\Log; use AgentCat\Internal\Pipeline\QueuedEvent; use AgentCat\Tests\Support\Autowire; @@ -37,6 +38,7 @@ protected function setUp(): void #[\Override] protected function tearDown(): void { + TrackedServers::resetSkipped(); // a track() while AgentCat is off marks the process EventQueue::setGlobal(null); Log::reset(); $this->environment->restore(); diff --git a/tests/Unit/Internal/Engine/TrackedServersTest.php b/tests/Unit/Internal/Engine/TrackedServersTest.php index 0bc4088..5352654 100644 --- a/tests/Unit/Internal/Engine/TrackedServersTest.php +++ b/tests/Unit/Internal/Engine/TrackedServersTest.php @@ -18,6 +18,7 @@ final class TrackedServersTest extends TestCase protected function tearDown(): void { TrackedServers::resetIncomplete(); + TrackedServers::resetSkipped(); } public function testAServerHasTheStateItWasAddedWith(): void @@ -61,6 +62,19 @@ public function testTheIncompleteFlagIsProcessWideUntilReset(): void $this->assertSame($state, TrackedServers::get($server), 'the reset keeps the tracked servers'); } + public function testTheSkippedFlagIsProcessWideUntilReset(): void + { + TrackedServers::resetSkipped(); + $this->assertFalse(TrackedServers::skipped()); + + TrackedServers::markSkipped(); + + $this->assertTrue(TrackedServers::skipped()); + $this->assertFalse(TrackedServers::incomplete(), 'a separate flag'); + TrackedServers::resetSkipped(); + $this->assertFalse(TrackedServers::skipped()); + } + private function state(object $server): TrackedServer { return new TrackedServer('proj', new Options(), static fn(): array => [$server::class, null], new Publisher(new EventQueue(transport: new RecordingTransport()))); diff --git a/tests/Unit/Testing/AgentCatFakeTest.php b/tests/Unit/Testing/AgentCatFakeTest.php index 4014a0d..61e671a 100644 --- a/tests/Unit/Testing/AgentCatFakeTest.php +++ b/tests/Unit/Testing/AgentCatFakeTest.php @@ -32,6 +32,12 @@ final class AgentCatFakeTest extends TestCase { private const string PROJECT = 'proj_abcdefghijklmnopqrstuvwxyz0'; + private const string NOT_TRACKED = 'Server is not tracked. Please call agentcat.track() first or provide a session ID string.'; + + private const string NOT_TRACKED_WHILE_OFF = 'Warning: Failed to publish custom event - the server is not tracked (track() ran while AgentCat was off; call AgentCat::fake() before track())'; + + private const string TRACKED_WHILE_OFF_HINT = 'A server was tracked while AgentCat was off: call AgentCat::fake() before track().'; + private Environment $environment; private MemoryLogger $logger; @@ -48,12 +54,15 @@ protected function setUp(): void $this->logger = new MemoryLogger(); Log::setLogger($this->logger); $this->home = $this->home(); + TrackedServers::resetIncomplete(); + TrackedServers::resetSkipped(); } #[\Override] protected function tearDown(): void { TrackedServers::resetIncomplete(); + TrackedServers::resetSkipped(); // the SDK is off in this suite: AgentCat::track() without fake() marks the process AgentCat::setClient(null); EventQueue::setGlobal(null); Log::reset(); @@ -81,6 +90,59 @@ public function testWithoutFakeTheSdkStaysOffInATestSuite(): void $this->assertFileDoesNotExist($this->home() . '/agentcat.log'); } + /** The application's factory tracks its server in `setUp()`, while AgentCat is off; the test body calls `fake()`. */ + public function testAServerTrackedBeforeTheFakeLogsInsteadOfThrowing(): void + { + $builder = TrackingFixture::builder(); + Tools::echo($builder); + AgentCat::track($builder, self::PROJECT); + $server = $builder->build(); + $fake = AgentCat::fake(); + + AgentCat::publishCustomEvent($builder, self::PROJECT, new CustomEvent(message: 'm')); + AgentCat::publishCustomEvent($server, self::PROJECT, new CustomEvent(message: 'm')); + + $this->assertSame([self::NOT_TRACKED_WHILE_OFF, self::NOT_TRACKED_WHILE_OFF], $fake->logs()); + $this->assertSame([], $fake->events()); + $this->assertSame(['queueLength' => 0, 'activeRequests' => 0], EventQueue::global()->stats()); + } + + public function testAssertionsWithoutAToolCallHintAtAServerTrackedBeforeTheFake(): void + { + AgentCat::track(TrackingFixture::builder(), self::PROJECT); + $fake = AgentCat::fake(); + + $this->assertSame( + 'Failed asserting that a tool call matching the filter was tracked. ' . self::TRACKED_WHILE_OFF_HINT, + $this->failureOf(static fn() => $fake->assertTracked(static fn(Event $e): bool => true)), + ); + $this->assertSame( + 'Failed asserting that 1 tool call(s) were tracked: 0 were. ' . self::TRACKED_WHILE_OFF_HINT, + $this->failureOf(static fn() => $fake->assertTrackedCount(1)), + ); + + $this->trackedHarness()->callTool('echo', ['msg' => 'hi']); + + $this->assertSame( + 'Failed asserting that a tool call matching the filter was tracked.', + $this->failureOf(static fn() => $fake->assertTracked(static fn(Event $e): bool => false)), + 'a tool call was recorded: no hint', + ); + $this->assertSame( + 'Failed asserting that 2 tool call(s) were tracked: 1 were.', + $this->failureOf(static fn() => $fake->assertTrackedCount(2)), + ); + } + + public function testWithoutATrackWhileOffAnUntrackedBuilderStillThrows(): void + { + AgentCat::fake(); + + $error = Thrown::by(\InvalidArgumentException::class, static fn() => AgentCat::publishCustomEvent(TrackingFixture::builder(), self::PROJECT)); + + $this->assertSame(self::NOT_TRACKED, $error->getMessage()); + } + public function testFakeInstallsItsClient(): void { $fake = AgentCat::fake(); From 88d0b2c28cab3c432a0e0854d8850ee721c0de9e Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 17 Oct 2026 18:12:53 +0000 Subject: [PATCH 17/17] test: pin the fake log's client registry and its string messages --- tests/Unit/Testing/Internal/FakeLogTest.php | 48 +++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 tests/Unit/Testing/Internal/FakeLogTest.php diff --git a/tests/Unit/Testing/Internal/FakeLogTest.php b/tests/Unit/Testing/Internal/FakeLogTest.php new file mode 100644 index 0000000..20c55b6 --- /dev/null +++ b/tests/Unit/Testing/Internal/FakeLogTest.php @@ -0,0 +1,48 @@ +client(); + $log = FakeLog::attach($client); + + $this->assertSame($log, FakeLog::of($client)); + $this->assertNull(FakeLog::of($this->client()), 'a client of no fake'); + $this->assertNotSame($log, FakeLog::attach($this->client())); + } + + public function testItKeepsEachMessageAsAStringWithoutItsLevel(): void + { + $log = FakeLog::attach($this->client()); + $stringable = new class implements \Stringable { + #[\Override] + public function __toString(): string + { + return 'from a Stringable'; + } + }; + + $log->log(LogLevel::INFO, 'first'); + $log->warning($stringable); + + $this->assertSame(['first', 'from a Stringable'], $log->messages); + } + + private function client(): Client + { + return new Client(ClientSeams::of(new EventQueue(transport: new RecordingTransport()), static fn(): bool => true)); + } +}