diff --git a/Mcp/Tools/Agent/State/StateSet.php b/Mcp/Tools/Agent/State/StateSet.php index 702edb3..f7c6b1d 100644 --- a/Mcp/Tools/Agent/State/StateSet.php +++ b/Mcp/Tools/Agent/State/StateSet.php @@ -7,7 +7,7 @@ namespace Core\Mod\Agentic\Mcp\Tools\Agent\State; use Core\Mcp\Dependencies\ToolDependency; use Core\Mod\Agentic\Mcp\Tools\Agent\AgentTool; use Core\Mod\Agentic\Models\AgentPlan; -use Core\Mod\Agentic\Models\AgentWorkspaceState; +use Core\Mod\Agentic\Models\WorkspaceState; /** * Set a workspace state value. @@ -93,7 +93,7 @@ class StateSet extends AgentTool return $this->error("Plan not found: {$planSlug}"); } - $state = AgentWorkspaceState::updateOrCreate( + $state = WorkspaceState::updateOrCreate( [ 'agent_plan_id' => $plan->id, 'key' => $key, diff --git a/Models/AgentPlan.php b/Models/AgentPlan.php index 8605b41..36ccf80 100644 --- a/Models/AgentPlan.php +++ b/Models/AgentPlan.php @@ -103,7 +103,7 @@ class AgentPlan extends Model public function states(): HasMany { - return $this->hasMany(AgentWorkspaceState::class); + return $this->hasMany(WorkspaceState::class); } public function templateVersion(): BelongsTo @@ -240,7 +240,7 @@ class AgentPlan extends Model return $state?->value; } - public function setState(string $key, mixed $value, string $type = 'json', ?string $description = null): AgentWorkspaceState + public function setState(string $key, mixed $value, string $type = 'json', ?string $description = null): WorkspaceState { return $this->states()->updateOrCreate( ['key' => $key], diff --git a/Models/AgentWorkspaceState.php b/Models/AgentWorkspaceState.php deleted file mode 100644 index d1db1ce..0000000 --- a/Models/AgentWorkspaceState.php +++ /dev/null @@ -1,116 +0,0 @@ - 'array', - ]; - - // Type constants - public const TYPE_JSON = 'json'; - - public const TYPE_MARKDOWN = 'markdown'; - - public const TYPE_CODE = 'code'; - - public const TYPE_REFERENCE = 'reference'; - - // Relationships - public function plan(): BelongsTo - { - return $this->belongsTo(AgentPlan::class, 'agent_plan_id'); - } - - // Scopes - public function scopeForPlan(Builder $query, AgentPlan|int $plan): Builder - { - $planId = $plan instanceof AgentPlan ? $plan->id : $plan; - - return $query->where('agent_plan_id', $planId); - } - - public function scopeOfType(Builder $query, string $type): Builder - { - return $query->where('type', $type); - } - - // Helpers - public function isJson(): bool - { - return $this->type === self::TYPE_JSON; - } - - public function isMarkdown(): bool - { - return $this->type === self::TYPE_MARKDOWN; - } - - public function isCode(): bool - { - return $this->type === self::TYPE_CODE; - } - - public function isReference(): bool - { - return $this->type === self::TYPE_REFERENCE; - } - - public function getValue(): mixed - { - return $this->value; - } - - public function getFormattedValue(): string - { - if ($this->isMarkdown() || $this->isCode()) { - return is_string($this->value) ? $this->value : json_encode($this->value, JSON_PRETTY_PRINT); - } - - return json_encode($this->value, JSON_PRETTY_PRINT); - } - - // Output - public function toMcpContext(): array - { - return [ - 'key' => $this->key, - 'type' => $this->type, - 'description' => $this->description, - 'value' => $this->value, - 'updated_at' => $this->updated_at?->toIso8601String(), - ]; - } -} diff --git a/Models/WorkspaceState.php b/Models/WorkspaceState.php index 1000dd8..091fa36 100644 --- a/Models/WorkspaceState.php +++ b/Models/WorkspaceState.php @@ -12,12 +12,25 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; /** * Workspace State Model * - * Key-value state storage for agent plans with typed content. + * Persistent key-value state storage for agent plans. + * Stores typed values shared across agent sessions within a plan, + * enabling context sharing and state recovery. + * + * @property int $id + * @property int $agent_plan_id + * @property string $key + * @property array $value + * @property string $type + * @property string|null $description + * @property \Carbon\Carbon|null $created_at + * @property \Carbon\Carbon|null $updated_at */ class WorkspaceState extends Model { use HasFactory; + protected $table = 'agent_workspace_states'; + public const TYPE_JSON = 'json'; public const TYPE_MARKDOWN = 'markdown'; @@ -31,34 +44,27 @@ class WorkspaceState extends Model 'key', 'value', 'type', - 'metadata', + 'description', ]; protected $casts = [ - 'metadata' => 'array', + 'value' => 'array', ]; - protected $attributes = [ - 'type' => self::TYPE_JSON, - 'metadata' => '{}', - ]; + // Relationships public function plan(): BelongsTo { return $this->belongsTo(AgentPlan::class, 'agent_plan_id'); } - /** - * Get typed value. - */ - public function getTypedValue(): mixed - { - return match ($this->type) { - self::TYPE_JSON => json_decode($this->value, true), - default => $this->value, - }; - } + // Scopes + public function scopeForPlan($query, AgentPlan|int $plan): mixed + { + $planId = $plan instanceof AgentPlan ? $plan->id : $plan; + +<<<<<<< HEAD /** * Set typed value. */ @@ -145,4 +151,71 @@ class WorkspaceState extends Model { return $query->where('type', $type); } + + // Type helpers + + public function isJson(): bool + { + return $this->type === self::TYPE_JSON; + } + + public function isMarkdown(): bool + { + return $this->type === self::TYPE_MARKDOWN; + } + + public function isCode(): bool + { + return $this->type === self::TYPE_CODE; + } + + public function isReference(): bool + { + return $this->type === self::TYPE_REFERENCE; + } + + public function getFormattedValue(): string + { + if ($this->isMarkdown() || $this->isCode()) { + return is_string($this->value) ? $this->value : json_encode($this->value, JSON_PRETTY_PRINT); + } + + return json_encode($this->value, JSON_PRETTY_PRINT); + } + + // Static helpers + + /** + * Get a state value for a plan, returning $default if not set. + */ + public static function getValue(AgentPlan $plan, string $key, mixed $default = null): mixed + { + $state = static::where('agent_plan_id', $plan->id)->where('key', $key)->first(); + + return $state !== null ? $state->value : $default; + } + + /** + * Set (upsert) a state value for a plan. + */ + public static function setValue(AgentPlan $plan, string $key, mixed $value, string $type = self::TYPE_JSON): self + { + return static::updateOrCreate( + ['agent_plan_id' => $plan->id, 'key' => $key], + ['value' => $value, 'type' => $type] + ); + } + + // MCP output + + public function toMcpContext(): array + { + return [ + 'key' => $this->key, + 'type' => $this->type, + 'description' => $this->description, + 'value' => $this->value, + 'updated_at' => $this->updated_at?->toIso8601String(), + ]; + } } diff --git a/TODO.md b/TODO.md index a8daa6c..d96acfe 100644 --- a/TODO.md +++ b/TODO.md @@ -134,10 +134,11 @@ Production-quality task list for the AI agent orchestration package. ### Code Quality -- [ ] **CQ-001: Duplicate state models (WorkspaceState vs AgentWorkspaceState)** - - Files: `Models/WorkspaceState.php`, `Models/AgentWorkspaceState.php` - - Issue: Two similar models for same purpose - - Fix: Consolidate into single model, or clarify distinct purposes +- [x] **CQ-001: Duplicate state models (WorkspaceState vs AgentWorkspaceState)** (FIXED 2026-02-23) + - Deleted `Models/AgentWorkspaceState.php` (unused legacy port) + - Consolidated into `Models/WorkspaceState.php` backed by `agent_workspace_states` table + - Updated `AgentPlan`, `StateSet`, `SecurityTest` to use `WorkspaceState` + - Added `WorkspaceStateTest` covering model behaviour and static helpers - [x] **CQ-002: ApiKeyManager uses Core\Api\ApiKey, not AgentApiKey** (FIXED 2026-02-23) - Location: `View/Modal/Admin/ApiKeyManager.php` diff --git a/tests/Feature/SecurityTest.php b/tests/Feature/SecurityTest.php index a4e1814..ba56601 100644 --- a/tests/Feature/SecurityTest.php +++ b/tests/Feature/SecurityTest.php @@ -10,7 +10,7 @@ use Core\Mod\Agentic\Mcp\Tools\Agent\State\StateGet; use Core\Mod\Agentic\Mcp\Tools\Agent\State\StateList; use Core\Mod\Agentic\Mcp\Tools\Agent\State\StateSet; use Core\Mod\Agentic\Models\AgentPlan; -use Core\Mod\Agentic\Models\AgentWorkspaceState; +use Core\Mod\Agentic\Models\WorkspaceState; use Core\Mod\Agentic\Models\Task; use Core\Tenant\Models\Workspace; use Illuminate\Foundation\Testing\RefreshDatabase; @@ -100,7 +100,7 @@ class SecurityTest extends TestCase 'workspace_id' => $this->workspace->id, ]); - AgentWorkspaceState::create([ + WorkspaceState::create([ 'agent_plan_id' => $plan->id, 'key' => 'test_key', 'value' => ['data' => 'secret'], @@ -122,7 +122,7 @@ class SecurityTest extends TestCase 'workspace_id' => $this->otherWorkspace->id, ]); - AgentWorkspaceState::create([ + WorkspaceState::create([ 'agent_plan_id' => $otherPlan->id, 'key' => 'secret_key', 'value' => ['data' => 'sensitive'], @@ -144,7 +144,7 @@ class SecurityTest extends TestCase 'workspace_id' => $this->workspace->id, ]); - AgentWorkspaceState::create([ + WorkspaceState::create([ 'agent_plan_id' => $plan->id, 'key' => 'test_key', 'value' => ['data' => 'allowed'], @@ -186,7 +186,7 @@ class SecurityTest extends TestCase 'workspace_id' => $this->otherWorkspace->id, ]); - AgentWorkspaceState::create([ + WorkspaceState::create([ 'agent_plan_id' => $otherPlan->id, 'key' => 'secret_key', 'value' => ['data' => 'sensitive'], diff --git a/tests/Feature/WorkspaceStateTest.php b/tests/Feature/WorkspaceStateTest.php new file mode 100644 index 0000000..6456498 --- /dev/null +++ b/tests/Feature/WorkspaceStateTest.php @@ -0,0 +1,270 @@ +workspace = Workspace::factory()->create(); + $this->plan = AgentPlan::factory()->create([ + 'workspace_id' => $this->workspace->id, + ]); + } + + // ========================================================================= + // Table and fillable + // ========================================================================= + + public function test_it_uses_agent_workspace_states_table(): void + { + $state = WorkspaceState::create([ + 'agent_plan_id' => $this->plan->id, + 'key' => 'test_key', + 'value' => ['data' => 'value'], + 'type' => WorkspaceState::TYPE_JSON, + ]); + + $this->assertDatabaseHas('agent_workspace_states', [ + 'id' => $state->id, + 'key' => 'test_key', + ]); + } + + public function test_it_casts_value_as_array(): void + { + $state = WorkspaceState::create([ + 'agent_plan_id' => $this->plan->id, + 'key' => 'array_key', + 'value' => ['foo' => 'bar', 'count' => 42], + ]); + + $fresh = $state->fresh(); + $this->assertIsArray($fresh->value); + $this->assertEquals('bar', $fresh->value['foo']); + $this->assertEquals(42, $fresh->value['count']); + } + + // ========================================================================= + // Type constants and helpers + // ========================================================================= + + public function test_type_constants_are_defined(): void + { + $this->assertEquals('json', WorkspaceState::TYPE_JSON); + $this->assertEquals('markdown', WorkspaceState::TYPE_MARKDOWN); + $this->assertEquals('code', WorkspaceState::TYPE_CODE); + $this->assertEquals('reference', WorkspaceState::TYPE_REFERENCE); + } + + public function test_isJson_returns_true_for_json_type(): void + { + $state = WorkspaceState::create([ + 'agent_plan_id' => $this->plan->id, + 'key' => 'json_key', + 'value' => ['x' => 1], + 'type' => WorkspaceState::TYPE_JSON, + ]); + + $this->assertTrue($state->isJson()); + $this->assertFalse($state->isMarkdown()); + $this->assertFalse($state->isCode()); + $this->assertFalse($state->isReference()); + } + + public function test_isMarkdown_returns_true_for_markdown_type(): void + { + $state = WorkspaceState::create([ + 'agent_plan_id' => $this->plan->id, + 'key' => 'md_key', + 'value' => null, + 'type' => WorkspaceState::TYPE_MARKDOWN, + ]); + + $this->assertTrue($state->isMarkdown()); + $this->assertFalse($state->isJson()); + } + + public function test_getFormattedValue_returns_json_string(): void + { + $state = WorkspaceState::create([ + 'agent_plan_id' => $this->plan->id, + 'key' => 'fmt_key', + 'value' => ['a' => 1], + 'type' => WorkspaceState::TYPE_JSON, + ]); + + $formatted = $state->getFormattedValue(); + $this->assertIsString($formatted); + $this->assertStringContainsString('"a"', $formatted); + } + + // ========================================================================= + // Relationship + // ========================================================================= + + public function test_it_belongs_to_plan(): void + { + $state = WorkspaceState::create([ + 'agent_plan_id' => $this->plan->id, + 'key' => 'rel_key', + 'value' => [], + ]); + + $this->assertEquals($this->plan->id, $state->plan->id); + } + + public function test_plan_has_many_states(): void + { + WorkspaceState::create(['agent_plan_id' => $this->plan->id, 'key' => 'k1', 'value' => []]); + WorkspaceState::create(['agent_plan_id' => $this->plan->id, 'key' => 'k2', 'value' => []]); + + $this->assertCount(2, $this->plan->states); + } + + // ========================================================================= + // Scopes + // ========================================================================= + + public function test_scopeForPlan_filters_by_plan_id(): void + { + $otherPlan = AgentPlan::factory()->create(['workspace_id' => $this->workspace->id]); + + WorkspaceState::create(['agent_plan_id' => $this->plan->id, 'key' => 'mine', 'value' => []]); + WorkspaceState::create(['agent_plan_id' => $otherPlan->id, 'key' => 'theirs', 'value' => []]); + + $results = WorkspaceState::forPlan($this->plan)->get(); + + $this->assertCount(1, $results); + $this->assertEquals('mine', $results->first()->key); + } + + public function test_scopeForPlan_accepts_int(): void + { + WorkspaceState::create(['agent_plan_id' => $this->plan->id, 'key' => 'int_scope', 'value' => []]); + + $results = WorkspaceState::forPlan($this->plan->id)->get(); + + $this->assertCount(1, $results); + } + + public function test_scopeOfType_filters_by_type(): void + { + WorkspaceState::create(['agent_plan_id' => $this->plan->id, 'key' => 'j', 'value' => [], 'type' => WorkspaceState::TYPE_JSON]); + WorkspaceState::create(['agent_plan_id' => $this->plan->id, 'key' => 'm', 'value' => null, 'type' => WorkspaceState::TYPE_MARKDOWN]); + + $jsonStates = WorkspaceState::ofType(WorkspaceState::TYPE_JSON)->get(); + + $this->assertCount(1, $jsonStates); + $this->assertEquals('j', $jsonStates->first()->key); + } + + // ========================================================================= + // Static helpers + // ========================================================================= + + public function test_getValue_returns_stored_value(): void + { + WorkspaceState::create([ + 'agent_plan_id' => $this->plan->id, + 'key' => 'endpoints', + 'value' => ['count' => 12], + ]); + + $value = WorkspaceState::getValue($this->plan, 'endpoints'); + + $this->assertEquals(['count' => 12], $value); + } + + public function test_getValue_returns_default_when_key_missing(): void + { + $value = WorkspaceState::getValue($this->plan, 'nonexistent', 'default_val'); + + $this->assertEquals('default_val', $value); + } + + public function test_setValue_creates_new_state(): void + { + $state = WorkspaceState::setValue($this->plan, 'api_findings', ['endpoints' => 5]); + + $this->assertDatabaseHas('agent_workspace_states', [ + 'agent_plan_id' => $this->plan->id, + 'key' => 'api_findings', + ]); + $this->assertEquals(['endpoints' => 5], $state->value); + } + + public function test_setValue_updates_existing_state(): void + { + WorkspaceState::setValue($this->plan, 'counter', ['n' => 1]); + WorkspaceState::setValue($this->plan, 'counter', ['n' => 2]); + + $this->assertDatabaseCount('agent_workspace_states', 1); + $this->assertEquals(['n' => 2], WorkspaceState::getValue($this->plan, 'counter')); + } + + // ========================================================================= + // MCP output + // ========================================================================= + + public function test_toMcpContext_returns_expected_keys(): void + { + $state = WorkspaceState::create([ + 'agent_plan_id' => $this->plan->id, + 'key' => 'mcp_key', + 'value' => ['x' => 99], + 'type' => WorkspaceState::TYPE_JSON, + 'description' => 'Test state entry', + ]); + + $context = $state->toMcpContext(); + + $this->assertArrayHasKey('key', $context); + $this->assertArrayHasKey('type', $context); + $this->assertArrayHasKey('description', $context); + $this->assertArrayHasKey('value', $context); + $this->assertArrayHasKey('updated_at', $context); + $this->assertEquals('mcp_key', $context['key']); + $this->assertEquals('Test state entry', $context['description']); + } + + // ========================================================================= + // Plan setState() integration + // ========================================================================= + + public function test_plan_setState_creates_workspace_state(): void + { + $state = $this->plan->setState('progress', ['done' => 3, 'total' => 10]); + + $this->assertInstanceOf(WorkspaceState::class, $state); + $this->assertEquals('progress', $state->key); + $this->assertEquals(['done' => 3, 'total' => 10], $state->value); + } + + public function test_plan_getState_retrieves_value(): void + { + $this->plan->setState('status_data', ['phase' => 'analysis']); + + $value = $this->plan->getState('status_data'); + + $this->assertEquals(['phase' => 'analysis'], $value); + } +}