refactor: AbilityRegistrar nimmt AbilityGuard-Liste und ToolRefusal statt DiviToolGate/DiviLocked; DiviToolGate implementiert AbilityGuard, DiviLocked ToolRefusal; Test mit Fake-Guard und Fake-Refusal ohne Divi

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Jan-Niclas Loosen
2026-10-18 21:47:26 +02:00
co-authored by Claude Sonnet 5.5
parent a7a9c09de8
commit abeb4b0eed
8 changed files with 257 additions and 26 deletions
+35
View File
@@ -0,0 +1,35 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Abilities;
use WP_Error;
defined('ABSPATH') || exit;
/**
* A cross-cutting rule the AbilityRegistrar applies to every ability it registers: it may refuse a call before the
* ability runs, add notes to the result and extend the output schema accordingly (e.g. the Divi release lock).
* The registrar knows only this interface; which guards exist is decided in Plugin (composition root).
*/
interface AbilityGuard
{
/**
* The refusal for this call before execute() runs, or null to let it through.
*
* @param array<string, mixed> $input
*/
public function refusal(Ability $ability, array $input): ?WP_Error;
/** The result of a finished call, possibly with added warnings. */
public function annotate(Ability $ability, mixed $result): mixed;
/**
* The output schema, possibly extended (e.g. by a warnings property).
*
* @param array<string, mixed>|null $schema
* @return array<string, mixed>|null
*/
public function outputSchema(Ability $ability, ?array $schema): ?array;
}
+41 -16
View File
@@ -5,8 +5,6 @@ declare(strict_types=1);
namespace LIT\MCP\Abilities; namespace LIT\MCP\Abilities;
use LIT\MCP\Access; use LIT\MCP\Access;
use LIT\MCP\Integrations\Divi\Release\DiviLocked;
use LIT\MCP\Integrations\Divi\Release\DiviToolGate;
use LIT\MCP\Integrations\IntegrationRegistry; use LIT\MCP\Integrations\IntegrationRegistry;
use LIT\MCP\Log; use LIT\MCP\Log;
use LIT\MCP\Server\RequestContext; use LIT\MCP\Server\RequestContext;
@@ -17,16 +15,20 @@ defined('ABSPATH') || exit;
/** /**
* Registers all abilities of available integrations with the Abilities API and wraps them * Registers all abilities of available integrations with the Abilities API and wraps them
* with the role gate, the Divi release lock (DiviToolGate) and neutral error handling. * with the role gate, the AbilityGuards it is given (e.g. the Divi release lock) and neutral error handling.
* It knows no integration: guards and ToolRefusal exceptions are the only seams.
*/ */
final class AbilityRegistrar final class AbilityRegistrar
{ {
public const CATEGORY = 'lit-mcp'; public const CATEGORY = 'lit-mcp';
/**
* @param list<AbilityGuard> $guards Applied in order: the first refusal wins, annotate() and outputSchema() chain.
*/
public function __construct( public function __construct(
private IntegrationRegistry $registry, private IntegrationRegistry $registry,
private ?ExposurePolicy $policy = null, private ?ExposurePolicy $policy = null,
private ?DiviToolGate $divi = null private array $guards = []
) { ) {
} }
@@ -55,7 +57,7 @@ final class AbilityRegistrar
/** @return array<string, mixed> */ /** @return array<string, mixed> */
public function argsFor(Ability $ability): array public function argsFor(Ability $ability): array
{ {
$divi = $this->divi ?? new DiviToolGate($this->registry->groupMap(...)); $guards = $this->guards;
$args = [ $args = [
'label' => $ability->label(), 'label' => $ability->label(),
'description' => $ability->description(), 'description' => $ability->description(),
@@ -77,14 +79,22 @@ final class AbilityRegistrar
return Errors::internal(); return Errors::internal();
} }
}, },
'execute_callback' => static function (mixed $input = null) use ($ability, $divi): mixed { 'execute_callback' => static function (mixed $input = null) use ($ability, $guards): mixed {
$refusal = $divi->refusal($ability); $input = is_array($input) ? $input : [];
if ($refusal !== null) { foreach ($guards as $guard) {
return $refusal; $refusal = $guard->refusal($ability, $input);
if ($refusal !== null) {
return $refusal;
}
} }
try { try {
return $divi->annotate($ability, $ability->execute(is_array($input) ? $input : [])); $result = $ability->execute($input);
foreach ($guards as $guard) {
$result = $guard->annotate($ability, $result);
}
return $result;
} catch (Throwable $e) { } catch (Throwable $e) {
Log::failure($ability->name(), $e); Log::failure($ability->name(), $e);
@@ -104,7 +114,10 @@ final class AbilityRegistrar
], ],
]; ];
$output = $divi->outputSchema($ability, $ability->outputSchema()); $output = $ability->outputSchema();
foreach ($guards as $guard) {
$output = $guard->outputSchema($ability, $output);
}
if ($output !== null) { if ($output !== null) {
$args['output_schema'] = $output; $args['output_schema'] = $output;
} }
@@ -113,19 +126,31 @@ final class AbilityRegistrar
} }
/** /**
* A partial write stays partial_write; otherwise a Divi release lock anywhere in the cause chain is its refusal * A partial write stays partial_write; otherwise the first ToolRefusal anywhere in the cause chain is the answer
* (spec §5.5); a rolled-back write is write_failed; anything else internal_error. * (e.g. the Divi release lock, spec §5.5); a rolled-back write is write_failed; anything else internal_error.
*/ */
private static function errorFor(Throwable $e): WP_Error private static function errorFor(Throwable $e): WP_Error
{ {
if ($e instanceof WriteFailed && ! $e->rolledBack) { if ($e instanceof WriteFailed && ! $e->rolledBack) {
return $e->toError(); return $e->toError();
} }
$locked = DiviLocked::in($e); $refusal = self::refusalIn($e);
if ($locked !== null) { if ($refusal !== null) {
return $locked->toError(); return $refusal->toError();
} }
return $e instanceof WriteFailed ? $e->toError() : Errors::internal(); return $e instanceof WriteFailed ? $e->toError() : Errors::internal();
} }
/** The first ToolRefusal in $e's chain of previous exceptions (callers wrap failures in WriteFailed). */
private static function refusalIn(Throwable $e): ?ToolRefusal
{
for ($cause = $e; $cause !== null; $cause = $cause->getPrevious()) {
if ($cause instanceof ToolRefusal) {
return $cause;
}
}
return null;
}
} }
+20
View File
@@ -0,0 +1,20 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Abilities;
use WP_Error;
defined('ABSPATH') || exit;
/**
* Implemented by exceptions that stand for a deliberate refusal instead of a failure (e.g. DiviLocked): thrown
* anywhere below an ability's execute(), also wrapped as the cause of a WriteFailed, the AbilityRegistrar answers
* with toError() instead of internal_error.
*/
interface ToolRefusal
{
/** The error the client gets for this refusal. */
public function toError(): WP_Error;
}
+2 -1
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace LIT\MCP\Integrations\Divi\Release; namespace LIT\MCP\Integrations\Divi\Release;
use LIT\MCP\Abilities\ToolRefusal;
use RuntimeException; use RuntimeException;
use Throwable; use Throwable;
use WP_Error; use WP_Error;
@@ -11,7 +12,7 @@ use WP_Error;
defined('ABSPATH') || exit; defined('ABSPATH') || exit;
/** A write refused because the installed Divi is not released (spec §5.5); AbilityRegistrar returns toError(). */ /** A write refused because the installed Divi is not released (spec §5.5); AbilityRegistrar returns toError(). */
final class DiviLocked extends RuntimeException final class DiviLocked extends RuntimeException implements ToolRefusal
{ {
public function __construct(public readonly ReleaseStatus $status) public function __construct(public readonly ReleaseStatus $status)
{ {
@@ -6,6 +6,7 @@ namespace LIT\MCP\Integrations\Divi\Release;
use Closure; use Closure;
use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Ability;
use LIT\MCP\Abilities\AbilityGuard;
use LIT\MCP\ChangeLog\ChangeLog; use LIT\MCP\ChangeLog\ChangeLog;
use LIT\MCP\Integrations\Divi\Library\LibraryAccess; use LIT\MCP\Integrations\Divi\Library\LibraryAccess;
use LIT\MCP\Integrations\Divi\ThemeBuilder\ThemeBuilderAccess; use LIT\MCP\Integrations\Divi\ThemeBuilder\ThemeBuilderAccess;
@@ -17,9 +18,9 @@ defined('ABSPATH') || exit;
* The central release lock of the Divi tools (spec §5.5): a writing tool of the groups Divi, Divi-Bibliothek or Divi * The central release lock of the Divi tools (spec §5.5): a writing tool of the groups Divi, Divi-Bibliothek or Divi
* Theme Builder is refused before it runs while Divi is not released (logged with result "refused"); a reading tool * Theme Builder is refused before it runs while Divi is not released (logged with result "refused"); a reading tool
* runs and gets the notice in warnings. Other tools that reach Divi (lit/content-create with divi) meet the adapter * runs and gets the notice in warnings. Other tools that reach Divi (lit/content-create with divi) meet the adapter
* lock (ReleaseGate::assertWritable()). * lock (ReleaseGate::assertWritable()). The AbilityRegistrar receives it as an AbilityGuard from Plugin.
*/ */
final class DiviToolGate final class DiviToolGate implements AbilityGuard
{ {
public const GROUPS = ['divi', LibraryAccess::GROUP, ThemeBuilderAccess::GROUP]; public const GROUPS = ['divi', LibraryAccess::GROUP, ThemeBuilderAccess::GROUP];
@@ -39,7 +40,7 @@ final class DiviToolGate
return in_array(($this->groups)()[$ability->name()] ?? '', self::GROUPS, true); return in_array(($this->groups)()[$ability->name()] ?? '', self::GROUPS, true);
} }
public function refusal(Ability $ability): ?WP_Error public function refusal(Ability $ability, array $input): ?WP_Error
{ {
if (! $this->covers($ability) || self::readsOnly($ability)) { if (! $this->covers($ability) || self::readsOnly($ability)) {
return null; return null;
+5 -1
View File
@@ -144,7 +144,11 @@ final class Plugin
DiviCheckCommand::register($diviCheck); DiviCheckCommand::register($diviCheck);
$policy = ExposurePolicy::fromOption()->withGroups($registry->groupMap(...)); $policy = ExposurePolicy::fromOption()->withGroups($registry->groupMap(...));
(new AbilityRegistrar($registry, $policy, new DiviToolGate($registry->groupMap(...), $changeLog)))->register(); (new AbilityRegistrar(
$registry,
$policy,
[new DiviToolGate($registry->groupMap(...), $changeLog)]
))->register();
(new ServerRegistrar( (new ServerRegistrar(
$registry, $registry,
new InstructionsBuilder($registry, static fn (): array => $schema->postTypeNames(), $policy), new InstructionsBuilder($registry, static fn (): array => $schema->postTypeNames(), $policy),
+146 -1
View File
@@ -6,8 +6,11 @@ namespace LIT\MCP\Tests\Unit\Abilities;
use Brain\Monkey\Functions; use Brain\Monkey\Functions;
use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Ability;
use LIT\MCP\Abilities\AbilityGuard;
use LIT\MCP\Abilities\AbilityRegistrar; use LIT\MCP\Abilities\AbilityRegistrar;
use LIT\MCP\Abilities\Errors;
use LIT\MCP\Abilities\ExposurePolicy; use LIT\MCP\Abilities\ExposurePolicy;
use LIT\MCP\Abilities\ToolRefusal;
use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\Abilities\WriteFailed;
use LIT\MCP\Integrations\Divi\Release\DiviLocked; use LIT\MCP\Integrations\Divi\Release\DiviLocked;
use LIT\MCP\Integrations\Divi\Release\DiviToolGate; use LIT\MCP\Integrations\Divi\Release\DiviToolGate;
@@ -20,6 +23,7 @@ use LIT\MCP\Server\RequestContext;
use LIT\MCP\Tests\Unit\TestCase; use LIT\MCP\Tests\Unit\TestCase;
use Mockery; use Mockery;
use RuntimeException; use RuntimeException;
use Throwable;
use WP_Error; use WP_Error;
final class AbilityRegistrarTest extends TestCase final class AbilityRegistrarTest extends TestCase
@@ -56,7 +60,7 @@ final class AbilityRegistrarTest extends TestCase
$integration->allows('abilities')->andReturn([$ability]); $integration->allows('abilities')->andReturn([$ability]);
$registry = new IntegrationRegistry($integration); $registry = new IntegrationRegistry($integration);
return new AbilityRegistrar($registry, null, new DiviToolGate($registry->groupMap(...))); return new AbilityRegistrar($registry, null, [new DiviToolGate($registry->groupMap(...))]);
} }
private function ability(bool $readonly = true): Mockery\MockInterface private function ability(bool $readonly = true): Mockery\MockInterface
@@ -247,6 +251,147 @@ final class AbilityRegistrarTest extends TestCase
self::assertToolError($this->executeLogged($ability), 'partial_write'); self::assertToolError($this->executeLogged($ability), 'partial_write');
} }
/** @param list<AbilityGuard> $guards */
private function guardedRegistrar(Ability $ability, array $guards): AbilityRegistrar
{
$integration = Mockery::mock(Integration::class);
$integration->allows('id')->andReturn('core');
$integration->allows('isAvailable')->andReturn(true);
$integration->allows('abilities')->andReturn([$ability]);
return new AbilityRegistrar(new IntegrationRegistry($integration), null, $guards);
}
/**
* A guard without any Divi knowledge: refuses with $refusal (or lets through), appends $tag to the result's
* "trail" and to the schema's "trail" property; $seen collects the inputs it was asked about.
*/
private function guard(string $tag, ?WP_Error $refusal = null, array &$seen = []): AbilityGuard
{
return new class ($tag, $refusal, $seen) implements AbilityGuard {
public function __construct(private string $tag, private ?WP_Error $refusal, private array &$seen)
{
}
public function refusal(Ability $ability, array $input): ?WP_Error
{
$this->seen[] = $input;
return $this->refusal;
}
public function annotate(Ability $ability, mixed $result): mixed
{
$result['trail'] = [...($result['trail'] ?? []), $this->tag];
return $result;
}
public function outputSchema(Ability $ability, ?array $schema): ?array
{
$schema['properties']['trail'][] = $this->tag;
return $schema;
}
};
}
private function refusalException(WP_Error $error, ?Throwable $previous = null): RuntimeException
{
return new class ($error, $previous) extends RuntimeException implements ToolRefusal {
public function __construct(private WP_Error $error, ?Throwable $previous)
{
parent::__construct('refused', 0, $previous);
}
public function toError(): WP_Error
{
return $this->error;
}
};
}
public function testWithoutGuardsNothingIsCheckedOrAdded(): void
{
$ability = $this->ability(false);
$ability->expects('execute')->with(['a' => 1])->andReturn(['ok' => true]);
$args = $this->registrarFor($ability)->argsFor($ability);
self::assertSame(['ok' => true], ($args['execute_callback'])(['a' => 1]));
self::assertSame(['type' => 'object'], $args['output_schema']);
}
public function testAGuardRefusesBeforeTheAbilityRuns(): void
{
$seen = [];
$ability = $this->ability(false);
$ability->expects('execute')->never();
$registrar = $this->guardedRegistrar($ability, [$this->guard('a', Errors::forbidden('Nein.'), $seen)]);
$result = ($registrar->argsFor($ability)['execute_callback'])(['x' => 1]);
self::assertToolError($result, 'forbidden');
self::assertSame([['x' => 1]], $seen);
}
public function testTheFirstRefusalWinsAndLaterGuardsAreNotAsked(): void
{
$seenSecond = [];
$ability = $this->ability(false);
$ability->expects('execute')->never();
$registrar = $this->guardedRegistrar($ability, [
$this->guard('a'),
$this->guard('b', Errors::notFound('Weg.')),
$this->guard('c', Errors::forbidden('Nein.'), $seenSecond),
]);
self::assertToolError(($registrar->argsFor($ability)['execute_callback'])([]), 'not_found');
self::assertSame([], $seenSecond);
}
public function testGuardsAnnotateTheResultAndExtendTheSchemaInOrder(): void
{
$ability = $this->ability(false);
$ability->allows('execute')->andReturn(['ok' => true]);
$registrar = $this->guardedRegistrar($ability, [$this->guard('a'), $this->guard('b')]);
$args = $registrar->argsFor($ability);
self::assertSame(['ok' => true, 'trail' => ['a', 'b']], ($args['execute_callback'])([]));
self::assertSame(['a', 'b'], $args['output_schema']['properties']['trail']);
}
public function testAToolRefusalThrownByAnAbilityIsItsErrorWithoutDivi(): void
{
$ability = $this->ability();
$ability->allows('execute')->andThrow($this->refusalException(Errors::forbidden('Gesperrt.')));
self::assertToolError($this->executeLogged($ability), 'forbidden');
}
public function testAToolRefusalInsideARolledBackWriteIsItsErrorButPartialStaysPartial(): void
{
$refusal = $this->refusalException(Errors::notFound('Weg.'));
$rolledBack = $this->ability();
$rolledBack->allows('execute')->andThrow(WriteFailed::rolledBack('Nicht gespeichert.', $refusal));
$partial = $this->ability();
$partial->allows('execute')->andThrow(WriteFailed::partial('Teilweise gespeichert.', $refusal));
self::assertToolError($this->executeLogged($rolledBack), 'not_found');
self::assertToolError($this->executeLogged($partial), 'partial_write');
}
public function testTheFirstRefusalInTheCauseChainWins(): void
{
$inner = $this->refusalException(Errors::notFound('Innen.'));
$outer = $this->refusalException(Errors::forbidden('Außen.'), $inner);
$ability = $this->ability();
$ability->allows('execute')->andThrow(WriteFailed::rolledBack('Nicht gespeichert.', $outer));
self::assertToolError($this->executeLogged($ability), 'forbidden');
}
/** /**
* Runs the execute callback with error_log() redirected (a PHP internal, not stubbable – like the tests above). * Runs the execute callback with error_log() redirected (a PHP internal, not stubbable – like the tests above).
*/ */
@@ -47,7 +47,7 @@ final class DiviToolGateTest extends TestCase
{ {
$gate = $this->gate(FakeChangeLog::forbidden()); $gate = $this->gate(FakeChangeLog::forbidden());
self::assertNull($gate->refusal($this->ability('lit/divi-insert', false))); self::assertNull($gate->refusal($this->ability('lit/divi-insert', false), []));
self::assertSame(['ok' => true], $gate->annotate($this->ability('lit/divi-outline', true), ['ok' => true])); self::assertSame(['ok' => true], $gate->annotate($this->ability('lit/divi-outline', true), ['ok' => true]));
} }
@@ -58,10 +58,10 @@ final class DiviToolGateTest extends TestCase
$gate = $this->gate($log); $gate = $this->gate($log);
foreach (['lit/divi-insert', 'lit/library-save', 'lit/tb-create-template'] as $name) { foreach (['lit/divi-insert', 'lit/library-save', 'lit/tb-create-template'] as $name) {
self::assertToolError($gate->refusal($this->ability($name, false)), 'divi_untested', $name); self::assertToolError($gate->refusal($this->ability($name, false), []), 'divi_untested', $name);
} }
self::assertNull($gate->refusal($this->ability('lit/content-create', false)), 'not a Divi tool (adapter lock)'); self::assertNull($gate->refusal($this->ability('lit/content-create', false), []), 'not a Divi tool (adapter lock)');
self::assertNull($gate->refusal($this->ability('lit/divi-outline', true)), 'reading stays allowed'); self::assertNull($gate->refusal($this->ability('lit/divi-outline', true), []), 'reading stays allowed');
self::assertSame(['refused', 'refused', 'refused'], array_column($log->records, 'result')); self::assertSame(['refused', 'refused', 'refused'], array_column($log->records, 'result'));
self::assertSame('Abgelehnt: Divi 5.3.0 ist nicht freigegeben.', $log->records[0]['summary']); self::assertSame('Abgelehnt: Divi 5.3.0 ist nicht freigegeben.', $log->records[0]['summary']);
} }