Skip to content

Streamline Registry Notifications and Add Discovery Suppression Support #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 13, 2025
Merged

Streamline Registry Notifications and Add Discovery Suppression Support #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e2cc08
fix(registry): Prevent excessive list_changed notifications
CodeWithKyrian Jun 13, 2025
069e909
fix(tests): Update notifier tests for clarity and functionality
CodeWithKyrian Jun 13, 2025
cd008aa
refactor: Rename notification methods for consistency and clarity
CodeWithKyrian Jun 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 82 additions & 50 deletions src/Registry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,8 @@ class Registry

private bool $discoveredElementsLoaded = false;

/** @var callable|null */
private $notifyToolsChanged = null;
private bool $notificationsEnabled = true;

/** @var callable|null */
private $notifyResourcesChanged = null;

/** @var callable|null */
private $notifyPromptsChanged = null;

public function __construct(
LoggerInterface $logger,
Expand All @@ -73,7 +67,6 @@ public function __construct(
$this->clientStateManager = $clientStateManager;

$this->initializeCollections();
$this->initializeDefaultNotifiers();

if ($this->cache) {
$this->loadDiscoveredElementsFromCache();
Expand Down Expand Up @@ -115,54 +108,93 @@ private function initializeCollections(): void
$this->manualTemplateUris = [];
}

private function initializeDefaultNotifiers(): void
public function enableNotifications(): void
{
$this->notifyToolsChanged = function () {
if ($this->clientStateManager) {
$notification = Notification::make('notifications/tools/list_changed');
$framedMessage = json_encode($notification->toArray(), JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . "\n";
if ($framedMessage !== false) {
$this->clientStateManager->queueMessageForAll($framedMessage);
}
}
};

$this->notifyResourcesChanged = function () {
if ($this->clientStateManager) {
$notification = Notification::make('notifications/resources/list_changed');
$framedMessage = json_encode($notification->toArray(), JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . "\n";
if ($framedMessage !== false) {
$this->clientStateManager->queueMessageForAll($framedMessage);
}
}
};

$this->notifyPromptsChanged = function () {
if ($this->clientStateManager) {
$notification = Notification::make('notifications/prompts/list_changed');
$framedMessage = json_encode($notification->toArray(), JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . "\n";
if ($framedMessage !== false) {
$this->clientStateManager->queueMessageForAll($framedMessage);
}
}
};
$this->notificationsEnabled = true;
}

public function setToolsChangedNotifier(?callable $notifier): void
public function disableNotifications(): void
{
$this->notifyToolsChanged = $notifier;
$this->notificationsEnabled = false;
}

public function setResourcesChangedNotifier(?callable $notifier): void
public function notifyToolsListChanged(): void
{
$this->notifyResourcesChanged = $notifier;
if (!$this->notificationsEnabled || !$this->clientStateManager) {
return;
}
$notification = Notification::make('notifications/tools/list_changed');

$framedMessage = json_encode($notification, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . "\n";
if ($framedMessage === false || $framedMessage === "\n") {
$this->logger->error('Failed to encode notification for queuing.', ['method' => $notification->method]);
return;
}
$this->clientStateManager->queueMessageForAll($framedMessage);
}

public function setPromptsChangedNotifier(?callable $notifier): void
public function notifyResourcesListChanged(): void
{
$this->notifyPromptsChanged = $notifier;
if (!$this->notificationsEnabled || !$this->clientStateManager) {
return;
}
$notification = Notification::make('notifications/resources/list_changed');

$framedMessage = json_encode($notification, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . "\n";
if ($framedMessage === false || $framedMessage === "\n") {
$this->logger->error('Failed to encode notification for queuing.', ['method' => $notification->method]);
return;
}
$this->clientStateManager->queueMessageForAll($framedMessage);
}

public function notifyPromptsListChanged(): void
{
if (!$this->notificationsEnabled || !$this->clientStateManager) {
return;
}
$notification = Notification::make('notifications/prompts/list_changed');

$framedMessage = json_encode($notification, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . "\n";
if ($framedMessage === false || $framedMessage === "\n") {
$this->logger->error('Failed to encode notification for queuing.', ['method' => $notification->method]);
return;
}
$this->clientStateManager->queueMessageForAll($framedMessage);
}

public function notifyResourceUpdated(string $uri): void
{
if (!$this->notificationsEnabled || !$this->clientStateManager) {
return;
}

$subscribers = $this->clientStateManager->getResourceSubscribers($uri);
if (empty($subscribers)) {
return;
}
$notification = Notification::make('notifications/resources/updated', ['uri' => $uri]);

$framedMessage = json_encode($notification, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) . "\n";
if ($framedMessage === false || $framedMessage === "\n") {
$this->logger->error('Failed to encode resource/updated notification.', ['uri' => $uri]);
return;
}

foreach ($subscribers as $clientId) {
$this->clientStateManager->queueMessage($clientId, $framedMessage);
}
}

/** @deprecated */
public function setToolsChangedNotifier(?callable $notifier): void {}

/** @deprecated */
public function setResourcesChangedNotifier(?callable $notifier): void {}

/** @deprecated */
public function setPromptsChangedNotifier(?callable $notifier): void {}

public function registerTool(ToolDefinition $tool, bool $isManual = false): void
{
$toolName = $tool->getName();
Expand All @@ -187,8 +219,8 @@ public function registerTool(ToolDefinition $tool, bool $isManual = false): void
unset($this->manualToolNames[$toolName]);
}

if (! $exists && $this->notifyToolsChanged) {
($this->notifyToolsChanged)($tool);
if (! $exists) {
$this->notifyToolsListChanged();
}
}

Expand All @@ -214,8 +246,8 @@ public function registerResource(ResourceDefinition $resource, bool $isManual =
unset($this->manualResourceUris[$uri]);
}

if (! $exists && $this->notifyResourcesChanged) {
($this->notifyResourcesChanged)();
if (! $exists) {
$this->notifyResourcesListChanged();
}
}

Expand Down Expand Up @@ -265,8 +297,8 @@ public function registerPrompt(PromptDefinition $prompt, bool $isManual = false)
unset($this->manualPromptNames[$promptName]);
}

if (! $exists && $this->notifyPromptsChanged) {
($this->notifyPromptsChanged)();
if (! $exists) {
$this->notifyPromptsListChanged();
}
}

Expand Down
14 changes: 4 additions & 10 deletions tests/Unit/RegistryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ function getRegistryProperty(Registry $reg, string $propName)

// --- Notifier Tests ---

it('default notifiers send messages via ClientStateManager', function () {
it('sends notifications when tools, resources, and prompts are registered', function () {
// Arrange
$tool = createTestTool('notify-tool');
$resource = createTestResource('notify://res');
Expand All @@ -392,18 +392,12 @@ function getRegistryProperty(Registry $reg, string $propName)
$this->registry->registerPrompt($prompt);
});

it('custom notifiers can be set and are called', function () {
it('does not send notifications when notifications are disabled', function () {
// Arrange
$toolNotifierCalled = false;
$this->registry->setToolsChangedNotifier(function () use (&$toolNotifierCalled) {
$toolNotifierCalled = true;
});
$this->registry->disableNotifications();

$this->clientStateManager->shouldNotReceive('queueMessageForAll');

// Act
$this->registry->registerTool(createTestTool('custom-notify'));

// Assert
expect($toolNotifierCalled)->toBeTrue();
$this->registry->registerTool(createTestTool('notify-tool'));
});