From 418e261686ee750d37bb70ea35b6a5278aa31fb3 Mon Sep 17 00:00:00 2001 From: Adam Frisby Date: Mon, 14 Sep 2026 07:20:08 +0000 Subject: [PATCH] Allow AgentClassId on PATCH /workitems/{id} Moves an item to a different agent class on any non-terminal state with no worker bound (notably WorkComplete items parked behind an unavailable auditor class), validated against the live router catalog (unknown ids 400), refused 409 on terminal items and while a worker holds the item. Persisted via a terminal-guarded partial UPDATE (UpdateAgentClassAsync) or riding the Queued guarded row write; emits work_item.agent_class_changed with old/new values. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox --- docs/reference/api.md | 6 +- src/CodeyBox.Api/WorkItemEndpoints.cs | 116 ++++++- src/CodeyBox.Core/AuditLog.cs | 19 ++ src/CodeyBox.Core/IWorkItemStore.cs | 50 +++ .../SqliteWorkItemStore.cs | 55 ++++ .../PatchWorkItemAgentClassTests.cs | 289 ++++++++++++++++++ 6 files changed, 529 insertions(+), 6 deletions(-) create mode 100644 tests/CodeyBox.Tests/PatchWorkItemAgentClassTests.cs 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; + } +}