refactor: consolidate duplicate state models into WorkspaceState (#18)

- Delete Models/AgentWorkspaceState.php (legacy port, no backing table)
- Rewrite Models/WorkspaceState.php as the single canonical state model
  backed by agent_workspace_states table with array value cast,
  type helpers, scopeForPlan/scopeOfType, static getValue/setValue,
  and toMcpContext() for MCP tool output
- Update AgentPlan::states() relation and setState() return type
- Update StateSet MCP tool import
- Update SecurityTest to use WorkspaceState
- Add WorkspaceStateTest covering table, casts, type helpers, scopes,
  static helpers, toMcpContext, and AgentPlan integration
- Mark CQ-001 done in TODO.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

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,

Models/AgentPlan.php

@@ -95,7 +95,7 @@ class AgentPlan extends Model
 
     public function states(): HasMany
     {
-        return $this->hasMany(AgentWorkspaceState::class);
+        return $this->hasMany(WorkspaceState::class);
     }
 
     // Scopes
@@ -227,7 +227,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],

Models/AgentWorkspaceState.php

@@ -1,115 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Core\Mod\Agentic\Models;
-
-use Illuminate\Database\Eloquent\Model;
-use Illuminate\Database\Eloquent\Relations\BelongsTo;
-
-/**
- * Agent Workspace State - shared context between sessions within a plan.
- *
- * Stores key-value data that persists across agent sessions,
- * 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 AgentWorkspaceState extends Model
-{
-    protected $table = 'agent_workspace_states';
-
-    protected $fillable = [
-        'agent_plan_id',
-        'key',
-        'value',
-        'type',
-        'description',
-    ];
-
-    protected $casts = [
-        'value' => '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($query, AgentPlan|int $plan)
-    {
-        $planId = $plan instanceof AgentPlan ? $plan->id : $plan;
-
-        return $query->where('agent_plan_id', $planId);
-    }
-
-    public function scopeOfType($query, string $type)
-    {
-        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(),
-        ];
-    }
-}

Models/WorkspaceState.php

@@ -11,12 +11,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';
@@ -30,118 +43,98 @@ 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
+    // Scopes
+
+    public function scopeForPlan($query, AgentPlan|int $plan): mixed
     {
-        return match ($this->type) {
-            self::TYPE_JSON => json_decode($this->value, true),
-            default => $this->value,
-        };
-    }
+        $planId = $plan instanceof AgentPlan ? $plan->id : $plan;
 
-    /**
-     * Set typed value.
-     */
-    public function setTypedValue(mixed $value): void
-    {
-        $storedValue = match ($this->type) {
-            self::TYPE_JSON => json_encode($value),
-            default => (string) $value,
-        };
-
-        $this->update(['value' => $storedValue]);
-    }
-
-    /**
-     * Get or create state for a plan.
-     */
-    public static function getOrCreate(AgentPlan $plan, string $key, mixed $default = null, string $type = self::TYPE_JSON): self
-    {
-        $state = static::where('agent_plan_id', $plan->id)
-            ->where('key', $key)
-            ->first();
-
-        if (! $state) {
-            $value = match ($type) {
-                self::TYPE_JSON => json_encode($default),
-                default => (string) ($default ?? ''),
-            };
-
-            $state = static::create([
-                'agent_plan_id' => $plan->id,
-                'key' => $key,
-                'value' => $value,
-                'type' => $type,
-            ]);
-        }
-
-        return $state;
-    }
-
-    /**
-     * Set state value for a plan.
-     */
-    public static function set(AgentPlan $plan, string $key, mixed $value, string $type = self::TYPE_JSON): self
-    {
-        $storedValue = match ($type) {
-            self::TYPE_JSON => json_encode($value),
-            default => (string) $value,
-        };
-
-        return static::updateOrCreate(
-            ['agent_plan_id' => $plan->id, 'key' => $key],
-            ['value' => $storedValue, 'type' => $type]
-        );
-    }
-
-    /**
-     * Get state value for a plan.
-     */
-    public static function get(AgentPlan $plan, string $key, mixed $default = null): mixed
-    {
-        $state = static::where('agent_plan_id', $plan->id)
-            ->where('key', $key)
-            ->first();
-
-        if (! $state) {
-            return $default;
-        }
-
-        return $state->getTypedValue();
-    }
-
-    /**
-     * Scope: for plan.
-     */
-    public function scopeForPlan($query, int $planId)
-    {
         return $query->where('agent_plan_id', $planId);
     }
 
-    /**
-     * Scope: by type.
-     */
-    public function scopeByType($query, string $type)
+    public function scopeOfType($query, string $type): mixed
     {
         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(),
+        ];
+    }
 }

TODO.md

@@ -131,10 +131,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
 
 - [ ] **CQ-002: ApiKeyManager uses Core\Api\ApiKey, not AgentApiKey**
   - Location: `View/Modal/Admin/ApiKeyManager.php`

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'],

tests/Feature/WorkspaceStateTest.php

@@ -0,0 +1,270 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Core\Mod\Agentic\Tests\Feature;
+
+use Core\Mod\Agentic\Models\AgentPlan;
+use Core\Mod\Agentic\Models\WorkspaceState;
+use Core\Tenant\Models\Workspace;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Tests\TestCase;
+
+/**
+ * Tests for WorkspaceState model (consolidated from WorkspaceState + AgentWorkspaceState).
+ */
+class WorkspaceStateTest extends TestCase
+{
+    use RefreshDatabase;
+
+    private Workspace $workspace;
+
+    private AgentPlan $plan;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->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);
+    }
+}