diff --git a/docs/reference/api.md b/docs/reference/api.md index 967485fd..72af8600 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -1125,16 +1125,18 @@ provided (non-null) in the body are updated. "mergeTimeoutMinutes": 60, "minModelScore": 70, "requiredCapabilities": ["sensitive"], - "dependsOn": ["", "..."] + "dependsOn": ["", "..."], + "agentClassId": "optional agent class id" } ``` * Returns `200 OK` with the updated work item record. -* Returns `409 Conflict` for non-`dependsOn` fields when the item is not in `Queued` state (in-flight items are read-only). `dependsOn` is allowed on any **non-terminal** state and returns `409` only on terminal items. +* Returns `409 Conflict` for non-`dependsOn` fields when the item is not in `Queued` state (in-flight items are read-only). `dependsOn`, the audit-budget fields, and `agentClassId` are allowed on any **non-terminal** state and return `409` only on terminal items. `agentClassId` additionally returns `409` while a worker holds the item, so the edit cannot race the dispatch path. * Validation rules for `title`, `prompt`, and `agent` are identical to `POST /workitems`. * `workTimeoutMinutes` is clamped to `[1, 480]`, `mergeTimeoutMinutes` to `[1, 240]`, `minModelScore` to `[0, 200]` — out-of-range values pin to the boundary rather than 400, matching the creation surface. This lets an operator bulk-PATCH the queue after a defaults bump without special-casing stray inputs. * `requiredCapabilities` is the explicit clearance/trust gate (see [agent-classes.md](../concepts/agent-classes.md#capability-gate)). Tags are trimmed, de-duplicated case-insensitively, and validated for length (≤64 chars) and count (≤16 entries). Sending the field replaces the existing list; omit it to leave the list unchanged. * `dependsOn` is replace-set semantics: the array overwrites the item's full dependency list. Each entry is a GUID, a namespaced `ns:value` externalId, or a bare externalId (must be unambiguous within the project). Capped at 100 entries; the create-time validation (existence, self-loop, cycle) re-runs against the proposed graph and `400`s on rejection. Pass `[]` to clear all deps. Persisted via a partial UPDATE that does not stomp `state` / `startedAt`, and a `work_item.dependencies_changed` audit-log entry records the pre/post sets. +* `agentClassId` moves the item to a different agent class without re-running completed phases — e.g. a `WorkComplete` item parked behind an auditor class whose members are all unavailable. The id must name a class in the live router catalog (unknown ids `400`); the value is persisted via a terminal-guarded partial UPDATE alongside a `work_item.agent_class_changed` audit-log entry recording the old and new class. * Priority is not editable here — use `PATCH /workitems/{id}/priority` (works on any non-terminal state, uses a TOCTOU-safe partial UPDATE). ### `PATCH /workitems/{id}/priority` diff --git a/src/CodeyBox.Api/WorkItemEndpoints.cs b/src/CodeyBox.Api/WorkItemEndpoints.cs index 71621b5d..877b9017 100644 --- a/src/CodeyBox.Api/WorkItemEndpoints.cs +++ b/src/CodeyBox.Api/WorkItemEndpoints.cs @@ -1396,11 +1396,17 @@ private static async Task RecoverAsync( /// Partially update a work item's editable fields. Most fields (title, /// prompt, agent, work/merge timeouts, min model score, required /// capabilities) are Queued-only — they affect a running pipeline so the - /// endpoint rejects 409 once dispatch starts. - /// and the audit-budget fields are the exceptions: they are allowed on any - /// non-terminal state (Queued / Working / Auditing / …), persisted via + /// endpoint rejects 409 once dispatch starts. , + /// the audit-budget fields, and + /// are the exceptions: they are allowed on any non-terminal state + /// (Queued / Working / Auditing / WorkComplete / …), persisted via /// partial UPDATEs that do not stomp state and friends. /// + /// AgentClassId is refused with 409 while a worker holds the item (the + /// dispatch path may be resolving the class concurrently) and with 409 on + /// terminal items; unknown class ids are rejected with 400 against the + /// live router catalog. + /// /// Timeout / score fields are clamped using the same bounds as creation — /// out-of-range values do not error, they pin to the boundary so an /// operator-led bulk-PATCH of a queue after a defaults bump never 400s. @@ -1418,6 +1424,8 @@ private static async Task PatchWorkItemAsync( IProjectRepository projects, IAgentRegistry agents, IKnobRegistry knobs, + IWorkerRegistry registry, + AgentClassRouter router, CancellationToken ct) { var (item, err) = await ResolveWorkItemAsync(id, store, ct); @@ -1443,6 +1451,7 @@ body.Title is not null || body.RequiredCapabilities is not null; var auditBudgetPatch = body.AuditMaxIterations is not null || body.AuditComplexity is not null; + var agentClassPatch = body.AgentClassId is not null; // ── State pre-checks: surface 409 before any write ──────────────────── // DependsOn is allowed on any non-terminal state — adding a dependency @@ -1458,6 +1467,16 @@ body.Title is not null { error = $"cannot edit audit budget of work item in terminal state '{item.State}'", }); + // AgentClassId is allowed on any non-terminal state — the motivating + // case is a WorkComplete item parked behind an auditor class whose + // members are all unavailable (a Queued-only restriction would not + // solve it). Terminal items are closed; worker-held items are refused + // below rather than racing the dispatch path. + if (agentClassPatch && WorkItemDependencies.TerminalStates.Contains(item!.State)) + return Results.Conflict(new + { + error = $"cannot edit agent class of work item in terminal state '{item.State}'", + }); if (queuedOnlyPatch && item!.State != WorkItemState.Queued) return Results.Conflict(new { @@ -1482,6 +1501,57 @@ body.Title is not null normalisedPatchKnobs = normalisedKnobs!; } + // ── AgentClassId validation (no writes yet, may 400) ───────────────── + // Same bounds as the create path (≤200 chars), plus an existence check + // against the live router catalog: an unknown class would otherwise + // fall through to direct agent pick at dispatch and silently strand + // the item outside the class the operator intended. + string? newAgentClassId = null; + string? oldAgentClassId = item!.AgentClassId; + if (agentClassPatch) + { + var trimmed = body.AgentClassId!.Trim(); + if (trimmed.Length == 0) + return Results.BadRequest(new { error = "agentClassId must not be empty" }); + if (trimmed.Length > 200) + return Results.BadRequest(new { error = "agentClassId must be <= 200 chars" }); + var knownClasses = router.ClassIds; + if (!knownClasses.Contains(trimmed, StringComparer.OrdinalIgnoreCase)) + return Results.BadRequest(new + { + error = $"unknown agent class '{trimmed}'", + available = knownClasses.OrderBy(c => c, StringComparer.OrdinalIgnoreCase), + }); + newAgentClassId = trimmed; + + // Refuse while a worker holds the item rather than racing the + // dispatch path, which may be resolving the class concurrently. + // Checked last (closest to the write) to minimise the check/write + // gap; the store's terminal guard below still fails closed on a + // concurrent transition. + try + { + var idStr = item.Id.ToString(); + var workers = await registry.ListAsync(ct); + foreach (var worker in workers) + { + if (string.Equals(worker.CurrentWorkItemId, idStr, StringComparison.OrdinalIgnoreCase)) + return Results.Conflict(new + { + error = $"cannot change agent class while worker '{worker.WorkerId}' holds work item '{id}'", + }); + } + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } + catch (Exception ex) + { + return Results.Conflict(new + { + error = $"cannot change agent class of work item '{id}': failed to inspect worker bindings: {ex.Message}", + }); + } + } + var updated = item!; var now = DateTimeOffset.UtcNow; var queuedUpdateExpectedUpdatedAt = item!.UpdatedAt; @@ -1578,6 +1648,9 @@ body.Title is not null updated = updated with { AuditComplexity = normalised, UpdatedAt = now }; } + if (newAgentClassId is not null) + updated = updated with { AgentClassId = newAgentClassId, UpdatedAt = now }; + IReadOnlyList oldDependsOn = updated.DependsOn; if (depsPatch) updated = updated with { DependsOn = newDependsOn!, UpdatedAt = now }; @@ -1646,6 +1719,33 @@ body.Title is not null break; } } + // AgentClassId on a Queued item with other queued edits rides the + // guarded row UPDATE above (its SQL carries agent_class_id); every + // other case — notably non-Queued items, where the guarded write is + // unavailable — goes through the terminal-guarded partial UPDATE so + // pipeline-owned columns are never stomped. + if (agentClassPatch && !needsQueuedRowUpdate) + { + var classResult = await store.UpdateAgentClassAsync( + updated.Id, + newAgentClassId, + now, + ct); + switch (classResult.Outcome) + { + case AgentClassUpdateOutcome.NotFound: + return Results.NotFound(new { error = $"work item '{id}' no longer exists" }); + case AgentClassUpdateOutcome.TerminalState: + return Results.Conflict(new + { + error = $"work item transitioned to terminal state '{classResult.Item!.State}' before agent class could be updated", + }); + case AgentClassUpdateOutcome.Updated: + oldAgentClassId = classResult.OldAgentClassId ?? oldAgentClassId; + updated = classResult.Item ?? updated with { AgentClassId = newAgentClassId, UpdatedAt = now }; + break; + } + } if (depsPatch && !queuedOnlyPatch) { var depResult = await store.UpdateDependsOnAsync(updated.Id, newDependsOn!, now, ct); @@ -1679,6 +1779,8 @@ body.Title is not null } if (depsPatch) AuditLog.WorkItemDependenciesChanged(updated.Id, oldDependsOn, newDependsOn!); + if (agentClassPatch) + AuditLog.WorkItemAgentClassChanged(updated.Id, oldAgentClassId, newAgentClassId); var statesById = new Dictionary(); var depExternalIds = new Dictionary(); @@ -3113,7 +3215,13 @@ public sealed record PatchWorkItemRequest( // Replace-set knob edit (queued-only, like Title/Agent). Sending a non-null // map replaces the entire stored map. Unknown keys and invalid values are // rejected with 400. Send an empty map to clear all per-item overrides. - IReadOnlyDictionary? Knobs = null); + IReadOnlyDictionary? Knobs = null, + // Agent-class reassignment. Allowed on any non-terminal item with no + // worker bound to it (409 while a worker holds the item or the item is + // terminal). The id must name a class in the live router catalog — + // unknown ids are rejected with 400, mirroring the create-time bounds. + // A work_item.agent_class_changed audit entry records the old/new values. + string? AgentClassId = null); public sealed record PatchPriorityRequest(int Priority); diff --git a/src/CodeyBox.Core/AuditLog.cs b/src/CodeyBox.Core/AuditLog.cs index a14a1b41..bdcd284d 100644 --- a/src/CodeyBox.Core/AuditLog.cs +++ b/src/CodeyBox.Core/AuditLog.cs @@ -226,6 +226,25 @@ public static void WorkItemDependenciesChanged( string.Join(",", oldDependsOn.Select(d => d.ToString())), string.Join(",", newDependsOn.Select(d => d.ToString()))); + /// + /// Distinct audit event for post-hoc agent-class edits via PATCH /workitems/{id}. + /// Records the previous and new class ids explicitly so the audit trail + /// captures the routing mutation; 's flags-only + /// shape would otherwise erase it. Null on either side means the item + /// carries no class (direct agent routing): the empty-string sentinel is + /// used because Serilog drops null properties, and readers rely on the + /// property being present to distinguish "no class" from "schema changed". + /// + public static void WorkItemAgentClassChanged( + WorkItemId id, + string? oldAgentClassId, + string? newAgentClassId) => + Audit("work_item.agent_class_changed") + .Information("Work item {WorkItemId} agent class changed: {OldAgentClassId} → {NewAgentClassId}", + id.ToString(), + oldAgentClassId ?? "", + newAgentClassId ?? ""); + public static void WorkItemReordered(int count) => Audit("work_item.reordered") .Information("Queue reordered: {Count} items repositioned", count); diff --git a/src/CodeyBox.Core/IWorkItemStore.cs b/src/CodeyBox.Core/IWorkItemStore.cs index fcbc224b..269e85e3 100644 --- a/src/CodeyBox.Core/IWorkItemStore.cs +++ b/src/CodeyBox.Core/IWorkItemStore.cs @@ -100,6 +100,32 @@ public enum AuditBudgetUpdateOutcome /// public readonly record struct AuditBudgetUpdateResult(AuditBudgetUpdateOutcome Outcome, WorkItem? Item); +/// +/// Outcome of . +/// +public enum AgentClassUpdateOutcome +{ + /// The row was updated and the new agent class id is persisted. + Updated, + /// The row no longer exists. + NotFound, + /// The row exists but is in a terminal state; no write was issued. + TerminalState, +} + +/// +/// Result returned by . +/// is populated on +/// and on so callers can +/// return the current state to the client; null on . +/// is the pre-update class id, captured so the +/// caller can emit a meaningful audit-log entry without re-reading the row. +/// +public readonly record struct AgentClassUpdateResult( + AgentClassUpdateOutcome Outcome, + WorkItem? Item, + string? OldAgentClassId); + /// /// Snapshot of a single dispatched iteration. /// is the value of at the moment the iteration @@ -239,6 +265,30 @@ Task UpdateAuditBudgetAsync( DateTimeOffset updatedAt, CancellationToken ct = default); + /// + /// Partial UPDATE that touches only the agent_class_id column and + /// updated_at for the row identified by . + /// Used by PATCH /workitems/{id} when an operator moves an item to a + /// different agent class — e.g. a WorkComplete item parked behind + /// an auditor class whose members are all unavailable. The full-row + /// would otherwise stomp state, + /// started_at, and friends when applied to an in-flight item. + /// + /// Returns when the row + /// is in a terminal state — class edits cannot affect closed work. + /// + /// The default implementation throws; persistent stores must override it + /// with a terminal-guarded partial UPDATE like + /// . + /// + Task UpdateAgentClassAsync( + WorkItemId id, + string? agentClassId, + DateTimeOffset updatedAt, + CancellationToken ct = default) + => throw new NotSupportedException( + "This work item store must implement guarded agent-class replacement before it can accept agent class edits."); + /// /// Partial UPDATE that touches only the per-item knob map and /// updated_at, guarded by both persisted state and the exact diff --git a/src/CodeyBox.Orchestrator/SqliteWorkItemStore.cs b/src/CodeyBox.Orchestrator/SqliteWorkItemStore.cs index 2b968212..5192384f 100644 --- a/src/CodeyBox.Orchestrator/SqliteWorkItemStore.cs +++ b/src/CodeyBox.Orchestrator/SqliteWorkItemStore.cs @@ -2218,6 +2218,61 @@ UPDATE work_items SET } } + public async Task UpdateAgentClassAsync( + WorkItemId id, + string? agentClassId, + DateTimeOffset updatedAt, + CancellationToken ct = default) + { + await _writeLock.WaitAsync(ct); + try + { + WorkItem? current; + using (var read = _conn.CreateCommand()) + { + read.CommandText = "SELECT * FROM work_items WHERE id = $id;"; + read.Parameters.AddWithValue("$id", id.ToString()); + using var reader = await read.ExecuteReaderAsync(ct); + current = await reader.ReadAsync(ct) ? Read(reader) : null; + } + + if (current is null) + return new AgentClassUpdateResult(AgentClassUpdateOutcome.NotFound, null, null); + + current = current with { ExternalIds = await LoadExternalIdsForAsync(current.Id, _conn, ct) }; + + if (WorkItemDependencies.TerminalStates.Contains(current.State)) + return new AgentClassUpdateResult(AgentClassUpdateOutcome.TerminalState, current, current.AgentClassId); + + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + UPDATE work_items SET + agent_class_id = $agent_class_id, + updated_at = $updated_at + WHERE id = $id; + """; + cmd.Parameters.AddWithValue("$agent_class_id", (object?)agentClassId ?? DBNull.Value); + cmd.Parameters.AddWithValue("$updated_at", updatedAt.ToString("O")); + cmd.Parameters.AddWithValue("$id", id.ToString()); + await cmd.ExecuteNonQueryAsync(ct); + + var updated = current with + { + AgentClassId = agentClassId, + UpdatedAt = updatedAt, + }; + return new AgentClassUpdateResult(AgentClassUpdateOutcome.Updated, updated, current.AgentClassId); + } + catch (SqliteException sqlex) when (sqlex.SqliteErrorCode == SQLITE_FULL) + { + throw HandleDiskFull("UpdateAgentClassAsync", sqlex); + } + finally + { + _writeLock.Release(); + } + } + public async Task TryReplaceKnobsIfStateAndUpdatedAtAsync( WorkItemId id, IReadOnlyDictionary knobs, diff --git a/tests/CodeyBox.Tests/PatchWorkItemAgentClassTests.cs b/tests/CodeyBox.Tests/PatchWorkItemAgentClassTests.cs new file mode 100644 index 00000000..0c89ec1f --- /dev/null +++ b/tests/CodeyBox.Tests/PatchWorkItemAgentClassTests.cs @@ -0,0 +1,289 @@ +using System.Net; +using System.Net.Http.Json; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using CodeyBox.Core; +using CodeyBox.Orchestrator; +using Serilog; +using Serilog.Events; + +namespace CodeyBox.Tests; + +/// +/// Tests for agentClassId on PATCH /workitems/{id}: an operator can move +/// an item to a different agent class on any non-terminal state with no worker +/// bound (notably WorkComplete items parked behind an auditor class whose +/// members are all unavailable), unknown ids 400, worker-held items 409, and +/// the change is recorded in the audit log with old and new values. +/// +public sealed class PatchWorkItemAgentClassTests : IDisposable +{ + private const string OldClass = "old-class"; + private const string NewClass = "new-class"; + + private readonly WorkItemApiFactory _factory = new(); + private readonly List _disposables = new(); + private readonly TestSink _sink = new(); + + public void Dispose() + { + foreach (var d in _disposables) + d.Dispose(); + _factory.Dispose(); + } + + private static WorkItem Sample(WorkItemState state, string? agentClassId) => new() + { + Id = WorkItemId.New(), + ProjectId = new ProjectId("proj"), + Title = "original title", + Prompt = "original prompt", + Agent = AgentKind.Codex, + AgentClassId = agentClassId, + State = state, + }; + + private static Project SampleProject() => new() + { + Id = new ProjectId("proj"), + DisplayName = "Test Project", + RepositoryUrl = "https://github.com/test/repo", + }; + + private (HttpClient Client, IServiceProvider Services) CreateClassClient() + { + var customised = _factory.WithWebHostBuilder(builder => + { + builder.ConfigureAppConfiguration((_, cfg) => + { + cfg.AddInMemoryCollection(new Dictionary + { + ["CodeyBox:AgentClasses:0:Id"] = OldClass, + ["CodeyBox:AgentClasses:0:DisplayName"] = "Old", + ["CodeyBox:AgentClasses:0:Members:0:Agent"] = "codex", + ["CodeyBox:AgentClasses:0:Members:0:Billing"] = "Subscription", + ["CodeyBox:AgentClasses:0:Members:0:QualityScore"] = "100", + ["CodeyBox:AgentClasses:1:Id"] = NewClass, + ["CodeyBox:AgentClasses:1:DisplayName"] = "New", + ["CodeyBox:AgentClasses:1:Members:0:Agent"] = "claude", + ["CodeyBox:AgentClasses:1:Members:0:Billing"] = "Subscription", + ["CodeyBox:AgentClasses:1:Members:0:QualityScore"] = "100", + }); + }); + }); + _disposables.Add(customised); + var client = customised.CreateClient(); + _disposables.Add(client); + return (client, customised.Services); + } + + private sealed class FixedQuotaProbe(AgentKind kind, double availablePct) : IAgentQuotaProbe + { + public AgentKind Kind { get; } = kind; + + public Task GetAvailabilityAsync(AgentMembership member, CancellationToken ct) + => Task.FromResult(new AgentQuotaSnapshot { AvailablePct = availablePct }); + } + + // Local router over the same two-class catalog the server is configured + // with. The server's own router is not used here: in this environment its + // availability gate benches the members (no installed CLIs/credentials), + // which would make the routing assertion about the environment rather + // than about the persisted class id. + private static AgentClassRouter BuildRouter() => new( + [ + new AgentClass + { + Id = OldClass, + DisplayName = "Old", + Members = [new() { Agent = AgentKind.Codex, Billing = AgentBilling.Subscription, QualityScore = 100 }], + }, + new AgentClass + { + Id = NewClass, + DisplayName = "New", + Members = [new() { Agent = AgentKind.Claude, Billing = AgentBilling.Subscription, QualityScore = 100 }], + }, + ], + [new FixedQuotaProbe(AgentKind.Codex, 80), new FixedQuotaProbe(AgentKind.Claude, 80)], + new QuotaRouterOptions { MinQuotaPct = 10.0, QuotaRecheckInterval = TimeSpan.FromMinutes(5) }, + NullLogger.Instance); + + [Fact] + public async Task PatchAgentClass_OnWorkCompleteWithoutWorker_RouterConsidersNewClassMembers() + { + var (client, _) = CreateClassClient(); + var item = Sample(WorkItemState.WorkComplete, OldClass); + await _factory.Store.CreateAsync(item); + + var router = BuildRouter(); + var project = SampleProject(); + + var before = await router.ResolveAsync(item, project, CancellationToken.None); + Assert.NotNull(before.Chosen); + Assert.Equal(AgentKind.Codex, before.Chosen!.Agent); + + var response = await client.PatchAsJsonAsync( + $"/workitems/{item.Id}", + new { agentClassId = NewClass }); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var stored = await _factory.Store.GetAsync(item.Id); + Assert.Equal(NewClass, stored!.AgentClassId); + Assert.Equal(WorkItemState.WorkComplete, stored.State); + + var after = await router.ResolveAsync(stored, project, CancellationToken.None); + Assert.NotNull(after.Chosen); + Assert.Equal(AgentKind.Claude, after.Chosen!.Agent); + } + + [Fact] + public async Task PatchAgentClass_UnknownClass_Returns400AndLeavesItemUnchanged() + { + var (client, _) = CreateClassClient(); + var item = Sample(WorkItemState.Queued, OldClass); + await _factory.Store.CreateAsync(item); + + var response = await client.PatchAsJsonAsync( + $"/workitems/{item.Id}", + new { agentClassId = "no-such-class" }); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + Assert.Contains("unknown agent class", body); + + var stored = await _factory.Store.GetAsync(item.Id); + Assert.Equal(OldClass, stored!.AgentClassId); + Assert.Equal(WorkItemState.Queued, stored.State); + } + + [Fact] + public async Task PatchAgentClass_WhenWorkerBound_Returns409AndLeavesItemUnchanged() + { + var (client, services) = CreateClassClient(); + var item = Sample(WorkItemState.WorkComplete, OldClass); + await _factory.Store.CreateAsync(item); + + var registry = services.GetRequiredService(); + var now = DateTimeOffset.UtcNow; + await registry.RegisterAsync(new WorkerRegistration + { + WorkerId = "worker-1", + HostName = "test-host", + ProcessId = 1234, + StartedAt = now, + LastHeartbeatAt = now, + CurrentWorkItemId = item.Id.ToString(), + }); + + var response = await client.PatchAsJsonAsync( + $"/workitems/{item.Id}", + new { agentClassId = NewClass }); + + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + Assert.Contains("holds", body); + + var stored = await _factory.Store.GetAsync(item.Id); + Assert.Equal(OldClass, stored!.AgentClassId); + Assert.Equal(WorkItemState.WorkComplete, stored.State); + } + + [Fact] + public async Task PatchAgentClass_EmitsAuditEventWithOldAndNewClass() + { + var (client, _) = CreateClassClient(); + + // The endpoint emits through the process-global Serilog logger (the + // test's AsyncLocal scoped logger does not flow to the TestServer + // pipeline), so swap the global for a sink-backed logger around each + // PATCH. Each attempt uses a fresh item and matches on its id, so a + // concurrent host boot in another collection reassigning Log.Logger + // mid-call retries instead of flaking. + using var testLogger = new LoggerConfiguration() + .Enrich.FromLogContext() + .WriteTo.Sink(_sink) + .CreateLogger(); + + LogEvent? found = null; + for (var attempt = 0; attempt < 5 && found is null; attempt++) + { + _sink.Clear(); + var item = Sample(WorkItemState.WorkComplete, OldClass); + await _factory.Store.CreateAsync(item); + + var previous = Log.Logger; + Log.Logger = testLogger; + try + { + var response = await client.PatchAsJsonAsync( + $"/workitems/{item.Id}", + new { agentClassId = NewClass }); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + finally + { + Log.Logger = previous; + } + + found = _sink.Events.FirstOrDefault(e => + Scalar(e, "EventName") == "work_item.agent_class_changed" + && Scalar(e, "WorkItemId") == item.Id.ToString()); + } + + Assert.True(found is not null, "expected work_item.agent_class_changed in the audit log"); + Assert.Equal(OldClass, Scalar(found!, "OldAgentClassId")); + Assert.Equal(NewClass, Scalar(found, "NewAgentClassId")); + } + + [Fact] + public async Task PatchAgentClass_OnQueuedItemWithOtherFields_PersistsBoth() + { + // The Queued guarded row UPDATE carries agent_class_id, so a combined + // PATCH must persist the class alongside the other queued edits. + var (client, _) = CreateClassClient(); + var item = Sample(WorkItemState.Queued, OldClass); + await _factory.Store.CreateAsync(item); + + var response = await client.PatchAsJsonAsync( + $"/workitems/{item.Id}", + new { title = "new title", agentClassId = NewClass }); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var stored = await _factory.Store.GetAsync(item.Id); + Assert.Equal("new title", stored!.Title); + Assert.Equal(NewClass, stored.AgentClassId); + Assert.Equal(WorkItemState.Queued, stored.State); + } + + [Fact] + public async Task PatchAgentClass_OnTerminalItem_Returns409AndLeavesItemUnchanged() + { + var (client, _) = CreateClassClient(); + var item = Sample(WorkItemState.Done, OldClass); + await _factory.Store.CreateAsync(item); + + var response = await client.PatchAsJsonAsync( + $"/workitems/{item.Id}", + new { agentClassId = NewClass }); + + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); + + var stored = await _factory.Store.GetAsync(item.Id); + Assert.Equal(OldClass, stored!.AgentClassId); + Assert.Equal(WorkItemState.Done, stored.State); + } + + private static T? Scalar(LogEvent evt, string key) + { + if (!evt.Properties.TryGetValue(key, out var prop) || prop is not ScalarValue sv) + return default; + if (sv.Value is T t) + return t; + return default; + } +}