From 96bd830eb1d40fb1d9acefa3ffc8325e452a4260 Mon Sep 17 00:00:00 2001 From: Adam Frisby Date: Sun, 13 Sep 2026 10:20:54 +0000 Subject: [PATCH] Add async human deployment-review auditor (operator as reviewer) Parks code-clean deployment iterations on NeedsOperatorInput (slot released, only the deployment held bounded by recipe MaxLifetime), notifies the operator with endpoint+expiry+criteria, and resumes on verdict: approve passes, reject-with-notes blocks into rework, expiry fails closed. Covers auditor kind/targets/cost/ordering, durable review store with CAS verdicts, expiry sweeper, verdict endpoints, and deterministic tests. CodeyBox-Prompt-Revision: 1 Co-Authored-By: CodeyBox --- docs/quality/audit.md | 21 + docs/reference/api.md | 24 + src/CodeyBox.Api/Program.cs | 31 + src/CodeyBox.Api/WorkItemEndpoints.cs | 210 +++++ src/CodeyBox.Audit/AuditPhaseLadder.cs | 3 +- src/CodeyBox.Audit/AuditorOrdering.cs | 13 + .../HumanDeploymentReviewAuditor.cs | 95 +++ src/CodeyBox.Core/HumanDeploymentReview.cs | 174 ++++ .../HumanDeploymentReviewPolicy.cs | 247 ++++++ src/CodeyBox.Core/IDeploymentDriver.cs | 11 + src/CodeyBox.Core/WellKnownAuditorNames.cs | 24 + src/CodeyBox.Deployment/DeploymentManager.cs | 11 + .../DeploymentAuditScope.cs | 17 + .../HumanDeploymentReviewSweeper.cs | 141 +++ .../HumanReviewExpiry.cs | 126 +++ src/CodeyBox.Orchestrator/PipelineRunner.cs | 800 +++++++++++++++++- .../SqliteHumanDeploymentReviewStore.cs | 323 +++++++ .../ProjectAuditorComposer.cs | 26 +- .../DeploymentAuditPhaseTests.cs | 5 + .../DeploymentAuditScopeTests.cs | 5 + .../DeploymentLeakReaperTests.cs | 5 + .../HumanDeploymentReviewEndpointTests.cs | 294 +++++++ .../HumanDeploymentReviewPolicyTests.cs | 353 ++++++++ .../HumanDeploymentReviewTests.cs | 622 ++++++++++++++ tests/CodeyBox.Tests/TestSupport.cs | 4 + 25 files changed, 3565 insertions(+), 20 deletions(-) create mode 100644 src/CodeyBox.Audit/HumanDeploymentReviewAuditor.cs create mode 100644 src/CodeyBox.Core/HumanDeploymentReview.cs create mode 100644 src/CodeyBox.Core/HumanDeploymentReviewPolicy.cs create mode 100644 src/CodeyBox.Orchestrator/HumanDeploymentReviewSweeper.cs create mode 100644 src/CodeyBox.Orchestrator/HumanReviewExpiry.cs create mode 100644 src/CodeyBox.Orchestrator/SqliteHumanDeploymentReviewStore.cs create mode 100644 tests/CodeyBox.Tests/HumanDeploymentReviewEndpointTests.cs create mode 100644 tests/CodeyBox.Tests/HumanDeploymentReviewPolicyTests.cs create mode 100644 tests/CodeyBox.Tests/HumanDeploymentReviewTests.cs diff --git a/docs/quality/audit.md b/docs/quality/audit.md index 4bdd686c0..f2e1cebe5 100644 --- a/docs/quality/audit.md +++ b/docs/quality/audit.md @@ -179,6 +179,27 @@ deployment-stage content: a deployment-targeted tool auditor can execute them against the endpoint, but the stage never blocks on the E2E chain itself. +A human reviewer (`Kind: "human"`, name `human:deployment-review`) closes +the loop as the operator acting through the standard auditor seam. Enabled +explicitly via `CodeyBox:HumanReview:Enabled` (default off, hot-reloadable; +`SweepInterval` tunes the expiry sweeper); a project drops it via +`ExcludedAuditors` by name. When composed, every code-clean iteration with +automated deployment probes also clean parks instead of passing: the item +moves to `NeedsOperatorInput` (releasing the worker slot and audit sandbox +while keeping only the deployment alive, bounded by the recipe's +`MaxLifetime`), the operator is notified with the deployment endpoint, +expiry, and acceptance criteria (backing question +`human-deployment-review-{iteration}` plus a `work_item.question_asked` / +`work_item.needs_operator_input` webhook pair), and the verdict resumes the +iteration. Approve passes and tears the deployment down immediately; +reject-with-notes yields blocking `Error` findings feeding the normal +rework loop (a fresh deployment per iteration); an undecided review past +its deadline fails closed as `expired unreviewed` — silence never passes. +Verdicts arrive via `POST /workitems/{id}/deployment-review/approve`, +`.../reject` (notes required), or by answering the backing question with +`approve` (any other text rejects with that text as notes). A late verdict +is refused with 410 and fails closed through the same expiry path. + ### .NET gates need a writable NuGet home `dotnet` materialises its per-user NuGet settings directory diff --git a/docs/reference/api.md b/docs/reference/api.md index 19b1436eb..4697ca906 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -1023,6 +1023,30 @@ When the question was already dismissed (idempotent no-op): * Returns `400 Bad Request` when `questionId` or `reason` is empty. * Returns `404 Not Found` when the work item or question does not exist. +### `GET /workitems/{id}/deployment-review` + +Returns the pending human deployment review for a parked iteration +(endpoint, expiry, backing question, brief with acceptance criteria), or +`404` when no review is outstanding. + +### `POST /workitems/{id}/deployment-review/approve` + +Approves the pending human deployment review; the iteration passes and the +held deployment is torn down immediately. Optional `note` (≤ 4000 chars) +is recorded on the verdict. + +### `POST /workitems/{id}/deployment-review/reject` + +Rejects the pending review with required `notes` (≤ 4000 chars) describing +what fails; the notes become blocking findings feeding the normal rework +loop. Returns `400` when notes are missing, `404` when no review is +pending, `409` when the item is not awaiting operator input or the review +is already decided, and `410` when the review expired unreviewed (the +expiry fails closed through teardown + dismiss + re-queue). Answering the +backing question `human-deployment-review-{iteration}` via `POST /answer` +verdicts identically: exactly `approve` approves, any other text rejects +with that text as notes. + ### `DELETE /workitems/{id}` Cancel a work item or close out a terminal-failure item. diff --git a/src/CodeyBox.Api/Program.cs b/src/CodeyBox.Api/Program.cs index b97278675..f904174fb 100644 --- a/src/CodeyBox.Api/Program.cs +++ b/src/CodeyBox.Api/Program.cs @@ -2746,6 +2746,19 @@ static Func DotnetTestRunOptionsAccessor(IServiceProvider sp) return () => monitor.CurrentValue; }); +// Human deployment reviewer (operator as reviewer through the auditor seam). +// Hot-reloadable via CodeyBox:HumanReview, explicit opt-in (default off); the +// accessor mirrors the Func pattern so the +// ProjectAuditorComposer observes the same live IOptionsMonitor snapshot and +// composes the reviewer into the deployment stage when enabled. +builder.Services.Configure( + builder.Configuration.GetSection("CodeyBox:HumanReview")); +builder.Services.AddSingleton>(sp => +{ + var monitor = sp.GetRequiredService>(); + return () => monitor.CurrentValue; +}); + builder.Services.AddSingleton(new RequiredAuditorPolicy( builder.Configuration.GetSection("CodeyBox:RequiredAuditors").Get() ?? [])); builder.Services.AddSingleton(); @@ -3038,6 +3051,23 @@ static Func DotnetTestRunOptionsAccessor(IServiceProvider sp) opts.StateDatabasePath, sp.GetRequiredService()); }); +builder.Services.AddSingleton(sp => +{ + var opts = sp.GetRequiredService>().Value; + return new SqliteHumanDeploymentReviewStore( + opts.StateDatabasePath, + sp.GetRequiredService()); +}); +builder.Services.AddHostedService(sp => new HumanDeploymentReviewSweeper( + sp.GetService(), + sp.GetService(), + sp.GetService(), + sp.GetService(), + sp.GetService(), + sp.GetService(), + sp.GetService>(), + TimeProvider.System, + sp.GetService>())); builder.Services.AddSingleton(sp => { var opts = sp.GetRequiredService>().Value; @@ -3619,6 +3649,7 @@ static Func DotnetTestRunOptionsAccessor(IServiceProvider sp) jobTrackExporter: sp.GetService(), deploymentManager: sp.GetService(), deploymentSubstrates: sp.GetService(), + humanReviews: sp.GetService(), staleBaseReworkRouter: sp.GetRequiredService(), flakeEscalation: sp.GetService(), flakeEscalationOptions: sp.GetRequiredService())); diff --git a/src/CodeyBox.Api/WorkItemEndpoints.cs b/src/CodeyBox.Api/WorkItemEndpoints.cs index 390957739..3d0444a37 100644 --- a/src/CodeyBox.Api/WorkItemEndpoints.cs +++ b/src/CodeyBox.Api/WorkItemEndpoints.cs @@ -32,6 +32,9 @@ public static void Map(WebApplication app) group.MapGet("/{id}/questions", GetQuestionsAsync); group.MapPost("/{id}/answer", AnswerQuestionAsync); group.MapPost("/{id}/dismiss-question", DismissQuestionAsync); + group.MapGet("/{id}/deployment-review", GetDeploymentReviewAsync); + group.MapPost("/{id}/deployment-review/approve", ApproveDeploymentReviewAsync); + group.MapPost("/{id}/deployment-review/reject", RejectDeploymentReviewAsync); group.MapGet("/{id}/stdout-tail", GetStdoutTailAsync); group.MapPost("/{id}/uncancel", UncancelAsync); group.MapPost("/{id}/resume", ResumeAsync); @@ -2044,6 +2047,7 @@ private static async Task AnswerQuestionAsync( ITaskQueue queue, IWebhookDispatcher webhooks, IProjectRepository projects, + IHumanDeploymentReviewStore? reviews, CancellationToken ct) { if (questionStore is null) return Results.Json(new { error = "question store not configured" }, statusCode: 503); @@ -2081,12 +2085,53 @@ await webhooks.PublishAsync(new WebhookEvent Details = new QuestionAnsweredDetails(item.Id.ToString(), item.ProjectId.Value, req.QuestionId, redactedAnswer, AnsweredBy: null), }, ct); + // A human-review backing question carries the operator's verdict: + // exactly "approve" approves, any other answer rejects with the text + // as notes. Recorded here so the generic answer path (API or CLI) + // verdicts like the dedicated endpoints below. + await TryRecordHumanReviewVerdictAsync(item.Id, req.QuestionId, redactedAnswer, reviews, ct); + // Transition out of NeedsOperatorInput if all questions are now resolved. await MaybeResumeFromNeedsOperatorInputAsync(item, store, questionStore, queue, webhooks, project, ct); return Results.Ok(new { status = "answered" }); } + /// + /// Interprets an answer to a human-review backing question as a verdict. + /// Best-effort: the answer itself is already persisted, so a missing + /// review store, an already-decided review, or an expired review simply + /// leaves the verdict unrecorded (the resume path still fails closed on + /// expiry). + /// + private static async Task TryRecordHumanReviewVerdictAsync( + WorkItemId itemId, + string questionId, + string answer, + IHumanDeploymentReviewStore? reviews, + CancellationToken ct) + { + if (reviews is null || !HumanDeploymentReviewPolicy.IsReviewQuestion(questionId)) + return; + var review = await reviews.GetActiveForWorkItemAsync(itemId.ToString(), ct); + if (review is null + || review.Status != HumanDeploymentReviewStatus.Pending + || !string.Equals(review.QuestionId, questionId, StringComparison.Ordinal)) + return; + var now = DateTimeOffset.UtcNow; + if (now >= review.Deadline) + return; + var approved = HumanDeploymentReviewPolicy.IsApprovalAnswer(answer); + await reviews.RecordVerdictAsync( + review.WorkItemId, + review.Iteration, + approved, + approved ? null : HumanDeploymentReviewPolicy.TruncateNotes(answer), + decidedBy: null, + now, + ct); + } + private static async Task DismissQuestionAsync( string id, DismissQuestionRequest req, @@ -2137,6 +2182,157 @@ await webhooks.PublishAsync(new WebhookEvent return Results.Ok(new { status = "dismissed" }); } + // ── Human deployment-review verdict endpoints ──────────────────────────── + // + // The operator acts as a reviewer through the standard auditor seam: + // approve records a pass, reject-with-notes records blocking findings + // that feed the normal rework loop. A verdict past the review deadline + // is refused with 410 and fails closed through the shared expiry path. + // The generic POST /answer endpoint verdicts identically (answering the + // backing question with "approve" approves; any other text rejects). + + private static async Task GetDeploymentReviewAsync( + string id, + IWorkItemStore store, + IHumanDeploymentReviewStore? reviews, + CancellationToken ct) + { + if (reviews is null) return Results.Json(new { error = "human review store not configured" }, statusCode: 503); + var (item, err) = await ResolveWorkItemAsync(id, store, ct); + if (err is not null) return err; + + var review = await reviews.GetActiveForWorkItemAsync(item!.Id.ToString(), ct); + if (review is null) + return Results.NotFound(new { error = "no pending human deployment review for this work item" }); + + return Results.Ok(new DeploymentReviewDto( + review.WorkItemId, + review.Iteration, + review.DeploymentId, + review.Deadline, + review.RequestedAt, + review.QuestionId, + review.Status.ToString(), + review.Brief)); + } + + private static async Task ApproveDeploymentReviewAsync( + string id, + ApproveDeploymentReviewRequest? req, + IWorkItemStore store, + IHumanDeploymentReviewStore? reviews, + IWorkItemQuestionStore? questionStore, + IDeploymentManager? deployments, + ITaskQueue queue, + IWebhookDispatcher webhooks, + IProjectRepository projects, + CancellationToken ct) + { + if (req?.Note is { Length: > 4000 }) + return Results.BadRequest(new { error = "note must be <= 4000 chars" }); + return await RecordDeploymentReviewVerdictAsync( + id, approved: true, notes: req?.Note, store, reviews, questionStore, + deployments, queue, webhooks, projects, ct); + } + + private static async Task RejectDeploymentReviewAsync( + string id, + RejectDeploymentReviewRequest? req, + IWorkItemStore store, + IHumanDeploymentReviewStore? reviews, + IWorkItemQuestionStore? questionStore, + IDeploymentManager? deployments, + ITaskQueue queue, + IWebhookDispatcher webhooks, + IProjectRepository projects, + CancellationToken ct) + { + if (string.IsNullOrWhiteSpace(req?.Notes)) + return Results.BadRequest(new { error = "notes describing what fails are required" }); + if (req!.Notes.Length > 4000) + return Results.BadRequest(new { error = "notes must be <= 4000 chars" }); + return await RecordDeploymentReviewVerdictAsync( + id, approved: false, notes: req.Notes, store, reviews, questionStore, + deployments, queue, webhooks, projects, ct); + } + + private static async Task RecordDeploymentReviewVerdictAsync( + string id, + bool approved, + string? notes, + IWorkItemStore store, + IHumanDeploymentReviewStore? reviews, + IWorkItemQuestionStore? questionStore, + IDeploymentManager? deployments, + ITaskQueue queue, + IWebhookDispatcher webhooks, + IProjectRepository projects, + CancellationToken ct) + { + if (reviews is null) return Results.Json(new { error = "human review store not configured" }, statusCode: 503); + if (questionStore is null) return Results.Json(new { error = "question store not configured" }, statusCode: 503); + + var (item, err) = await ResolveWorkItemAsync(id, store, ct); + if (err is not null) return err; + + if (item!.State != WorkItemState.NeedsOperatorInput) + return Results.Conflict(new { error = "work item is not waiting for operator input" }); + + var review = await reviews.GetActiveForWorkItemAsync(item.Id.ToString(), ct); + if (review is null) + return Results.NotFound(new { error = "no pending human deployment review for this work item" }); + if (review.Status != HumanDeploymentReviewStatus.Pending) + return Results.Conflict(new { error = $"review is already {review.Status.ToString().ToLowerInvariant()}; resume is pending" }); + + var now = DateTimeOffset.UtcNow; + if (now >= review.Deadline) + { + // Late verdict: refuse and fail closed through the shared expiry + // path (teardown + dismiss + re-queue) so silence past the + // deadline never becomes an implicit pass. + await HumanReviewExpiry.ExpireAsync( + reviews, deployments, store, questionStore, queue, webhooks, + review, now, ct: ct); + return Results.Json(new { error = "review expired unreviewed" }, statusCode: 410); + } + + var recorded = await reviews.RecordVerdictAsync( + review.WorkItemId, + review.Iteration, + approved, + approved ? notes : HumanDeploymentReviewPolicy.TruncateNotes(notes ?? string.Empty), + decidedBy: null, + now, + ct); + if (!recorded) + return Results.Conflict(new { error = "review was decided concurrently" }); + + // Answer the backing question so the Q&A trail shows the verdict; + // the idempotent no-op branch below covers a concurrent answer. + var question = await questionStore.GetAsync(item.Id.ToString(), review.QuestionId, ct); + if (question is { State: "open" }) + { + var answerText = approved ? "approve" : HumanDeploymentReviewPolicy.TruncateNotes(notes ?? string.Empty); + await questionStore.AnswerAsync(item.Id.ToString(), review.QuestionId, answerText, answeredBy: null, ct); + var project = await projects.GetAsync(item.ProjectId, ct); + await webhooks.PublishAsync(new WebhookEvent + { + Event = "work_item.question_answered", + WorkItem = item, + Project = project, + Details = new QuestionAnsweredDetails( + item.Id.ToString(), item.ProjectId.Value, review.QuestionId, answerText, AnsweredBy: null), + }, ct); + await MaybeResumeFromNeedsOperatorInputAsync(item, store, questionStore, queue, webhooks, project, ct); + } + + return Results.Ok(new + { + status = approved ? "approved" : "rejected", + iteration = review.Iteration, + }); + } + /// /// When a work item is in NeedsOperatorInput state and all its questions are now /// resolved (answered or dismissed), transitions back to WorkComplete and re-enqueues. @@ -2818,6 +3014,20 @@ public sealed record AnswerQuestionRequest(string QuestionId, string Answer); public sealed record DismissQuestionRequest(string QuestionId, string Reason); +public sealed record ApproveDeploymentReviewRequest(string? Note); + +public sealed record RejectDeploymentReviewRequest(string? Notes); + +public sealed record DeploymentReviewDto( + string WorkItemId, + int Iteration, + string DeploymentId, + DateTimeOffset ExpiresAt, + DateTimeOffset RequestedAt, + string QuestionId, + string Status, + string Brief); + public sealed record QuestionDto( string Id, string WorkItemId, diff --git a/src/CodeyBox.Audit/AuditPhaseLadder.cs b/src/CodeyBox.Audit/AuditPhaseLadder.cs index 25976b4e4..52b3c9fa4 100644 --- a/src/CodeyBox.Audit/AuditPhaseLadder.cs +++ b/src/CodeyBox.Audit/AuditPhaseLadder.cs @@ -25,7 +25,8 @@ public static AuditCostClass CostClassOf(IAuditor auditor) if (auditor.Role == AuditorRole.BuildTestGate) return AuditCostClass.MechanicalGate; if (auditor.Required.HasFlag(AuditCapabilities.AgentCredentials) - || string.Equals(auditor.Kind, "llm", StringComparison.OrdinalIgnoreCase)) + || string.Equals(auditor.Kind, "llm", StringComparison.OrdinalIgnoreCase) + || string.Equals(auditor.Kind, WellKnownAuditorKinds.Human, StringComparison.OrdinalIgnoreCase)) return AuditCostClass.Reviewer; return AuditCostClass.Tool; } diff --git a/src/CodeyBox.Audit/AuditorOrdering.cs b/src/CodeyBox.Audit/AuditorOrdering.cs index 894b064ef..c23b7da41 100644 --- a/src/CodeyBox.Audit/AuditorOrdering.cs +++ b/src/CodeyBox.Audit/AuditorOrdering.cs @@ -5,12 +5,25 @@ namespace CodeyBox.Audit; /// /// Single source of truth for audit-panel ordering. This lives in the audit /// layer so Core stays limited to neutral auditor contract metadata. +/// Tiers derive from declared capabilities only — never from concrete types. /// public static class AuditorOrdering { + /// + /// True when the auditor is a human reviewer (declared + /// Kind = "human"). Human reviewers park the pipeline awaiting an + /// operator verdict, so they sort after every automated auditor. + /// + public static bool IsHuman(IAuditor auditor) + { + ArgumentNullException.ThrowIfNull(auditor); + return string.Equals(auditor.Kind, WellKnownAuditorKinds.Human, StringComparison.OrdinalIgnoreCase); + } + public static int TierOf(IAuditor auditor) => auditor.Role == AuditorRole.BuildTestGate ? 0 : auditor.CanShortCircuitOnBlockingFinding ? 1 + : IsHuman(auditor) ? 4 : auditor.Required.HasFlag(AuditCapabilities.AgentCredentials) ? 3 : 2; } diff --git a/src/CodeyBox.Audit/HumanDeploymentReviewAuditor.cs b/src/CodeyBox.Audit/HumanDeploymentReviewAuditor.cs new file mode 100644 index 000000000..695a1cc57 --- /dev/null +++ b/src/CodeyBox.Audit/HumanDeploymentReviewAuditor.cs @@ -0,0 +1,95 @@ +using CodeyBox.Core; + +namespace CodeyBox.Audit; + +/// +/// Human deployment reviewer: the operator acting as a reviewer through the +/// standard auditor seam. Declares Kind = "human" and the +/// deployment target (composable with other targets for future +/// human-review surfaces); the kind composes with target filtering like any +/// other auditor, so ComposeForTarget(..., Deployment) selects it and +/// ExcludedAuditors removes it by name. +/// +/// The reviewer never runs inline: always throws +/// because a human verdict cannot be +/// produced inside an audit sandbox. The pipeline detects human-kind +/// auditors in the deployment stage and takes the async park/resume path +/// instead — provision the deployment, park the item at +/// NeedsOperatorInput (releasing the worker slot and audit sandbox +/// while keeping only the deployment alive), notify the operator with the +/// endpoint + expiry + acceptance criteria, and on verdict resume the +/// iteration, record the verdict like any , and +/// tear the deployment down immediately. Approve passes; reject-with-notes +/// yields blocking findings feeding the normal rework loop; an undecided +/// review past the recipe's max lifetime fails closed as 'expired +/// unreviewed'. The pipeline never calls ; the throw +/// below is a backstop so a future caller that runs it inline fails loudly +/// instead of recording a fake pass. +/// +public sealed class HumanDeploymentReviewAuditor : IAuditor +{ + private readonly HumanDeploymentReviewOptions _options; + + public HumanDeploymentReviewAuditor(HumanDeploymentReviewOptions options) + { + ArgumentNullException.ThrowIfNull(options); + if (string.IsNullOrWhiteSpace(options.Name)) + throw new ArgumentException("Human review requires a non-empty Name.", nameof(options)); + _options = options; + } + + public string Name => _options.Name; + + public string Kind => WellKnownAuditorKinds.Human; + + public AuditCapabilities Required => AuditCapabilities.None; + + public IReadOnlySet Targets => AuditTargets.DeploymentOnly; + + public Task RunAsync( + ISandbox sandbox, + string workingDirectory, + AuditContext context, + CancellationToken ct = default) + { + throw new AuditUnavailableException( + $"'{Name}' cannot run inline: a human verdict arrives asynchronously through the " + + "operator park/resume path (approve/reject via the deployment-review endpoints or " + + "by answering the review question). The pipeline handles human-kind auditors in the " + + "deployment stage without invoking them; reaching this method is a wiring bug, " + + "reported as an incomplete iteration rather than a pass."); + } +} + +/// +/// Options for . Bound from the +/// CodeyBox:HumanReview configuration section and read through an +/// IOptionsMonitor so operators can toggle or retune the reviewer +/// without a restart. Operational values only — no literals in source. +/// +public sealed class HumanDeploymentReviewOptions +{ + /// + /// When false the human reviewer is not composed into the audit panel at + /// all (today's behaviour is unchanged). When true it is composed into + /// the deployment stage of every project with deployment auditing + /// enabled; a project still opts out via ExcludedAuditors by name. + /// Default false — human review is an explicit opt-in. + /// + public bool Enabled { get; set; } + + /// + /// Stable auditor name used for logs, findings, audit reports, and + /// ExcludedAuditors removal. Defaults to + /// human:deployment-review. + /// + public string Name { get; set; } = WellKnownAuditorNames.HumanDeploymentReview; + + /// + /// How often the expiry sweeper scans for undecided reviews past their + /// deadline. Default 30 seconds. Set to to + /// disable sweep-driven expiry (the resume path still fails closed when + /// it observes an expired review). + /// + public TimeSpan SweepInterval { get; set; } = TimeSpan.FromSeconds(30); +} diff --git a/src/CodeyBox.Core/HumanDeploymentReview.cs b/src/CodeyBox.Core/HumanDeploymentReview.cs new file mode 100644 index 000000000..ff3691626 --- /dev/null +++ b/src/CodeyBox.Core/HumanDeploymentReview.cs @@ -0,0 +1,174 @@ +namespace CodeyBox.Core; + +/// +/// Lifecycle status of a human deployment review. A review is created +/// when the audit iteration parks; the operator's +/// verdict moves it to or ; +/// the sweeper (or the resume path) moves an undecided review past its +/// deadline to — fail-closed, never an implicit pass. +/// marks that the audit loop has folded the outcome +/// into its verdict and torn the deployment down. +/// +public enum HumanDeploymentReviewStatus +{ + Pending = 0, + Approved = 1, + Rejected = 2, + Expired = 3, +} + +/// +/// Durable record of one parked human deployment review for a single audit +/// iteration. The deployment stays alive in the deployment manager's active +/// set (keyed by ) while the worker slot and audit +/// sandbox are released; on verdict or expiry the audit loop re-attaches by +/// id, records an -shaped outcome like any auditor, +/// and tears the deployment down immediately. +/// +public sealed record HumanDeploymentReview +{ + /// Work item under review (WorkItemId.ToString()). + public required string WorkItemId { get; init; } + + /// Audit iteration that parked. + public required int Iteration { get; init; } + + /// + /// Id of the live deployment held in the manager's active set while the + /// verdict is pending. Survives only in-process; after a restart the id + /// no longer resolves and the resume path treats the review as expired. + /// + public required string DeploymentId { get; init; } + + /// JSON-serialized . + public required string EndpointJson { get; init; } + + /// + /// Absolute time the deployment must be torn down by (start + the + /// recipe's max lifetime). An undecided review at this time expires + /// fail-closed with an 'expired unreviewed' blocking finding. + /// + public required DateTimeOffset Deadline { get; init; } + + /// When the review was requested (park time, UTC). + public required DateTimeOffset RequestedAt { get; init; } + + /// + /// Operator brief: deployment endpoint, expiry, and what to verify (the + /// item's acceptance criteria). Also used as the backing question text. + /// Bounded at creation; never contains secrets. + /// + public required string Brief { get; init; } + + /// Question id of the backing operator question. + public required string QuestionId { get; init; } + + /// JSON-serialized names of the human auditors awaiting verdict. + public required string HumanAuditorsJson { get; init; } + + /// + /// JSON-serialized code-stage findings already collected before parking + /// (blocking-free by construction — parking requires a clean code + /// stage), so the resumed iteration's report stays complete without + /// re-running quota-spending code auditors. + /// + public required string CodeFindingsJson { get; init; } + + /// + /// JSON-serialized code-stage completed auditor names, so the resumed + /// iteration's completion set still covers the auditors that ran before + /// the park (the merge gate validates the persisted snapshot against + /// the scheduled panel). + /// + public required string CodeCompletedJson { get; init; } + + /// + /// JSON-serialized automated deployment-stage findings already collected + /// before parking, so the resume path merges them without re-running + /// quota-spending auditors against the held deployment. + /// + public required string AutomatedFindingsJson { get; init; } + + /// JSON-serialized completed automated auditor names. + public required string AutomatedCompletedJson { get; init; } + + /// JSON-serialized incomplete automated auditor names. + public required string AutomatedIncompleteJson { get; init; } + + /// AgentKind value that ran automated auditors, if any. + public string? ActiveAuditAgentKind { get; init; } + + public bool DeclaredShortCircuitBlocking { get; init; } + + public bool IncompleteVerdict { get; init; } + + public HumanDeploymentReviewStatus Status { get; init; } = HumanDeploymentReviewStatus.Pending; + + /// Reject notes / approval note recorded with the verdict. + public string? Notes { get; init; } + + public DateTimeOffset? DecidedAt { get; init; } + + /// Operator identity that recorded the verdict, when known. + public string? DecidedBy { get; init; } + + /// When the audit loop consumed the outcome (null = not yet). + public DateTimeOffset? ConsumedAt { get; init; } +} + +/// +/// Durable store for parked human deployment reviews. Implementations must be +/// safe for concurrent calls and perform compare-and-set transitions (a +/// verdict, expiry, or consume applies only from the expected prior state). +/// +public interface IHumanDeploymentReviewStore +{ + /// + /// Inserts a pending review. When a consumed review already exists for the + /// same (work item, iteration) it is replaced with the fresh pending row + /// (supports re-review of the same iteration after a manual replay); + /// when a non-consumed row exists it is returned unchanged. + /// Returns the effective row. + /// + Task GetOrCreatePendingAsync(HumanDeploymentReview review, CancellationToken ct = default); + + /// Returns the review for (work item, iteration), or null. + Task TryGetAsync(string workItemId, int iteration, CancellationToken ct = default); + + /// + /// Returns the newest non-consumed review for a work item, or null. + /// At most one such row exists: parking always returns the worker slot, + /// so a second park cannot start before the first is consumed. + /// + Task GetActiveForWorkItemAsync(string workItemId, CancellationToken ct = default); + + /// + /// Records the operator verdict. Applies only when the row is still + /// ; returns false + /// (leaving the row untouched) otherwise. + /// + Task RecordVerdictAsync( + string workItemId, + int iteration, + bool approved, + string? notes, + string? decidedBy, + DateTimeOffset decidedAt, + CancellationToken ct = default); + + /// + /// Marks a pending review expired. Applies only from + /// ; returns false otherwise. + /// + Task MarkExpiredAsync(string workItemId, int iteration, DateTimeOffset expiredAt, CancellationToken ct = default); + + /// + /// Marks a decided/expired review consumed by the audit loop. Idempotent: + /// always succeeds, stamping + /// when not already set. + /// + Task MarkConsumedAsync(string workItemId, int iteration, DateTimeOffset consumedAt, CancellationToken ct = default); + + /// Pending reviews whose deadline has passed (sweeper input). + Task> ListExpiredPendingAsync(DateTimeOffset now, CancellationToken ct = default); +} diff --git a/src/CodeyBox.Core/HumanDeploymentReviewPolicy.cs b/src/CodeyBox.Core/HumanDeploymentReviewPolicy.cs new file mode 100644 index 000000000..e1e5373a5 --- /dev/null +++ b/src/CodeyBox.Core/HumanDeploymentReviewPolicy.cs @@ -0,0 +1,247 @@ +using System.Text.Json; + +namespace CodeyBox.Core; + +/// +/// Pure policy for the human deployment reviewer: the operator brief, the +/// answer↔verdict mapping, and the verdict→findings mapping. All decisions +/// here are pure functions of their inputs so they are trivially +/// unit-testable; the orchestrator owns the IO (park, notify, teardown). +/// +public static class HumanDeploymentReviewPolicy +{ + /// Maximum prompt characters embedded in the operator brief. + public const int MaxPromptChars = 2000; + + /// Maximum acceptance-criteria entries embedded in the brief. + public const int MaxCriteriaEntries = 20; + + /// Maximum characters per acceptance-criteria entry. + public const int MaxCriteriaEntryChars = 500; + + /// Maximum characters for reject notes recorded with a verdict. + public const int MaxNotesChars = 4000; + + private static readonly JsonSerializerOptions FindingJsonOptions = new() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }; + + /// + /// Builds the backing operator question id for an audit iteration. + /// Per-iteration so each verdict is recorded against its own question row. + /// + public static string QuestionIdFor(int iteration) + => $"{WellKnownAuditorNames.HumanDeploymentReviewQuestionPrefix}-{iteration}"; + + /// + /// True when is a human-review backing + /// question: the bare marker or the marker plus a numeric iteration + /// suffix (human-deployment-review-3). Compared by exact shape — + /// the id is then resolved through the review store by exact equality, + /// never trusted as an iteration number. + /// + public static bool IsReviewQuestion(string? questionId) + { + if (questionId is null) + return false; + if (questionId.Equals( + WellKnownAuditorNames.HumanDeploymentReviewQuestionPrefix, + StringComparison.Ordinal)) + return true; + var head = WellKnownAuditorNames.HumanDeploymentReviewQuestionPrefix + "-"; + if (!questionId.StartsWith(head, StringComparison.Ordinal)) + return false; + var suffix = questionId[head.Length..]; + return suffix.Length > 0 && suffix.All(char.IsAsciiDigit); + } + + /// + /// Maps an operator answer to a verdict. Exactly "approve" (trimmed, + /// case-insensitive) approves; any other text rejects with the text as + /// the review notes. Exact equality only — never substring. + /// + public static bool IsApprovalAnswer(string? answer) + => string.Equals(answer?.Trim(), "approve", StringComparison.OrdinalIgnoreCase); + + /// + /// Human-readable one-line description of a deployment endpoint for + /// notifications. Prefers the URL, then host:port, then artifact path. + /// + public static string DescribeEndpoint(DeploymentEndpoint? endpoint) + { + if (endpoint is null) + return "(endpoint unavailable)"; + if (!string.IsNullOrWhiteSpace(endpoint.Url)) + return endpoint.Url!; + if (!string.IsNullOrWhiteSpace(endpoint.Host) && endpoint.Port is { } port) + return $"{endpoint.Host}:{port}"; + if (!string.IsNullOrWhiteSpace(endpoint.Path)) + return endpoint.Path!; + return $"({endpoint.Kind} endpoint)"; + } + + /// + /// Builds the operator brief: what is deployed where, when the review + /// expires, what to verify (acceptance criteria), and how to record the + /// verdict. Bounded: the prompt and criteria are truncated to the + /// Max* caps above. The item prompt is operator-authored and the + /// criteria are pipeline-assembled; the endpoint is driver-provided — + /// all are rendered as plain text, never executed. + /// + public static string BuildBrief( + string workItemTitle, + string workItemPrompt, + string endpointDescription, + DateTimeOffset deadline, + IReadOnlyList<(string Name, string Description)> acceptanceCriteria, + int iteration, + string deploymentId) + { + var safeTitle = string.IsNullOrWhiteSpace(workItemTitle) ? "(untitled)" : workItemTitle.Trim(); + var prompt = workItemPrompt ?? string.Empty; + var trimmedPrompt = prompt.Length > MaxPromptChars + ? prompt[..MaxPromptChars] + "… [truncated]" + : prompt; + + var brief = new System.Text.StringBuilder(); + brief.AppendLine($"Human deployment review requested (audit iteration {iteration})."); + brief.AppendLine($"Deployment: {deploymentId} at {endpointDescription}."); + brief.AppendLine($"This review expires at {deadline:O} — an undecided review fails closed as 'expired unreviewed'."); + brief.AppendLine(); + brief.AppendLine($"Work item: {safeTitle}"); + if (!string.IsNullOrWhiteSpace(trimmedPrompt)) + { + brief.AppendLine("Original request:"); + brief.AppendLine(trimmedPrompt); + brief.AppendLine(); + } + + var criteria = acceptanceCriteria ?? []; + if (criteria.Count > 0) + { + brief.AppendLine("Acceptance criteria to verify against the live deployment:"); + var shown = 0; + foreach (var (name, description) in criteria) + { + if (shown >= MaxCriteriaEntries) + { + brief.AppendLine($"- … [{criteria.Count - shown} more, truncated]"); + break; + } + + var entry = string.IsNullOrWhiteSpace(description) ? name : $"{name}: {description}"; + if (entry.Length > MaxCriteriaEntryChars) + entry = entry[..MaxCriteriaEntryChars] + "… [truncated]"; + brief.AppendLine($"- {entry}"); + shown++; + } + + brief.AppendLine(); + } + else + { + brief.AppendLine("No linked acceptance criteria — verify the live deployment satisfies the request above."); + brief.AppendLine(); + } + + brief.Append("Record the verdict with the deployment-review approve endpoint, "); + brief.Append("the reject endpoint with notes describing what fails, "); + brief.Append("or answer this question with \"approve\" to approve (any other answer rejects with that text as notes)."); + return brief.ToString(); + } + + /// + /// Maps a human verdict to audit findings with full blocking authority: + /// approve yields no findings (pass); reject and expiry yield Error + /// findings that flow into the normal rework loop. Never demoted — + /// a human reviewer is an objective gate like any deployment probe. + /// + public static IReadOnlyList BuildVerdictFindings( + string auditorName, + HumanDeploymentReviewStatus status, + string? notes, + DateTimeOffset? deadline = null) + { + if (string.IsNullOrWhiteSpace(auditorName)) + throw new ArgumentException("Auditor name must be non-empty.", nameof(auditorName)); + + return status switch + { + HumanDeploymentReviewStatus.Approved => [], + HumanDeploymentReviewStatus.Rejected => [new AuditFinding( + auditorName, + AuditSeverity.Error, + "Human review rejected the deployment", + string.IsNullOrWhiteSpace(notes) + ? "The operator rejected the verification deployment without notes." + : TruncateNotes(notes!))], + HumanDeploymentReviewStatus.Expired => [new AuditFinding( + auditorName, + AuditSeverity.Error, + "Human review expired unreviewed", + deadline is { } d + ? $"No operator verdict was recorded before the review deadline {d:O}. Silence never passes: the deployment was torn down and the iteration fails closed." + : "No operator verdict was recorded before the review deadline. Silence never passes: the deployment was torn down and the iteration fails closed.")], + _ => throw new ArgumentOutOfRangeException( + nameof(status), + status, + "Only a decided or expired review maps to findings; pending reviews must park, not complete."), + }; + } + + /// Serializes automated deployment-stage findings for the review row. + public static string SerializeFindings(IReadOnlyList findings) + { + var dtos = findings.Select(f => new StoredFinding( + f.AuditorName, (int)f.Severity, f.Title, f.Description, f.Location)).ToList(); + return JsonSerializer.Serialize(dtos, FindingJsonOptions); + } + + /// + /// Deserializes stored findings. Throws + /// on corrupt payloads so the caller fails closed instead of passing. + /// + public static IReadOnlyList DeserializeFindings(string json) + { + List? dtos; + try + { + dtos = JsonSerializer.Deserialize>(json, FindingJsonOptions); + } + catch (JsonException ex) + { + throw new InvalidOperationException("Stored automated deployment findings are corrupt.", ex); + } + + if (dtos is null) + throw new InvalidOperationException("Stored automated deployment findings are corrupt."); + return dtos.Select(d => new AuditFinding( + d.AuditorName, (AuditSeverity)d.Severity, d.Title, d.Description, d.Location)).ToList(); + } + + public static string SerializeStrings(IReadOnlyList values) + => JsonSerializer.Serialize(values, FindingJsonOptions); + + public static IReadOnlyList DeserializeStrings(string json) + { + try + { + return JsonSerializer.Deserialize>(json, FindingJsonOptions) ?? []; + } + catch (JsonException ex) + { + throw new InvalidOperationException("Stored human-review string list is corrupt.", ex); + } + } + + public static string TruncateNotes(string notes) + => notes.Length > MaxNotesChars ? notes[..MaxNotesChars] + "… [truncated]" : notes; + + private sealed record StoredFinding( + string AuditorName, + int Severity, + string Title, + string Description, + string? Location); +} diff --git a/src/CodeyBox.Core/IDeploymentDriver.cs b/src/CodeyBox.Core/IDeploymentDriver.cs index 4c3e999cb..ca3fd9abe 100644 --- a/src/CodeyBox.Core/IDeploymentDriver.cs +++ b/src/CodeyBox.Core/IDeploymentDriver.cs @@ -564,6 +564,17 @@ Task StartAsync( DeploymentContext context, CancellationToken ct = default); + /// + /// Looks up a live deployment previously started through + /// that has not been disposed yet. Used by the + /// human-review resume path to re-attach to the deployment held while + /// the operator verdict was pending. Returns false when the id is + /// unknown or the handle was already disposed (e.g. after an + /// orchestrator restart) — the caller then fails closed instead of + /// reviewing a deployment that no longer exists. + /// + bool TryGetActive(string deploymentId, out IDeploymentHandle? handle); + /// /// Snapshot of currently-active deployments. Used by the leak reaper /// and operator-facing /deployments endpoints (link 2). diff --git a/src/CodeyBox.Core/WellKnownAuditorNames.cs b/src/CodeyBox.Core/WellKnownAuditorNames.cs index 62b92d092..d47a22b96 100644 --- a/src/CodeyBox.Core/WellKnownAuditorNames.cs +++ b/src/CodeyBox.Core/WellKnownAuditorNames.cs @@ -7,4 +7,28 @@ namespace CodeyBox.Core; public static class WellKnownAuditorNames { public const string BuildScript = "process:build-script"; + + /// + /// Stable name for the human deployment reviewer. Composed like any other + /// auditor and removable per project via ExcludedAuditors. + /// + public const string HumanDeploymentReview = "human:deployment-review"; + + /// + /// Stable prefix for the operator question backing a human deployment + /// review. The full question id appends the audit iteration + /// (human-deployment-review-3) so each iteration's verdict is + /// recorded against its own question row. + /// + public const string HumanDeploymentReviewQuestionPrefix = "human-deployment-review"; +} + +/// +/// Stable values the pipeline branches on by +/// declaration. A human reviewer completes asynchronously through the +/// operator park/resume path rather than inline like tool/llm auditors. +/// +public static class WellKnownAuditorKinds +{ + public const string Human = "human"; } diff --git a/src/CodeyBox.Deployment/DeploymentManager.cs b/src/CodeyBox.Deployment/DeploymentManager.cs index be8a45e01..cc4de6694 100644 --- a/src/CodeyBox.Deployment/DeploymentManager.cs +++ b/src/CodeyBox.Deployment/DeploymentManager.cs @@ -80,6 +80,17 @@ public IReadOnlyList GetActive() return result; } + public bool TryGetActive(string deploymentId, out IDeploymentHandle? handle) + { + handle = null; + if (string.IsNullOrWhiteSpace(deploymentId)) + return false; + if (!_active.TryGetValue(deploymentId, out var tracked)) + return false; + handle = tracked; + return true; + } + private void Untrack(string id) => _active.TryRemove(id, out _); private sealed class TrackedDeployment( diff --git a/src/CodeyBox.Orchestrator/DeploymentAuditScope.cs b/src/CodeyBox.Orchestrator/DeploymentAuditScope.cs index 63f6e9ef0..609c3f10c 100644 --- a/src/CodeyBox.Orchestrator/DeploymentAuditScope.cs +++ b/src/CodeyBox.Orchestrator/DeploymentAuditScope.cs @@ -114,6 +114,23 @@ public CancellationTokenSource LinkLifetime( return cts; } + /// + /// Transfers teardown ownership of the deployment away from this scope + /// without tearing it down: subsequent calls + /// are no-ops and the handle stays live in the deployment manager's + /// active set. Used by the human-review park path, which keeps only the + /// deployment alive while the operator verdict is pending (bounded by + /// the recipe's max lifetime); the resume path or the expiry sweeper + /// tears it down by re-attaching through the manager. + /// + public void Detach() + { + Volatile.Write(ref _disposed, 1); + _log.LogInformation( + "Deployment-stage audit detached deployment {DeploymentId}: teardown ownership moved to the human-review record", + _handle.Id); + } + /// /// Tears the deployment down. Idempotent: repeated calls are no-ops, so /// abort, cancel, timeout, and normal paths can all dispose without diff --git a/src/CodeyBox.Orchestrator/HumanDeploymentReviewSweeper.cs b/src/CodeyBox.Orchestrator/HumanDeploymentReviewSweeper.cs new file mode 100644 index 000000000..b81bb65ef --- /dev/null +++ b/src/CodeyBox.Orchestrator/HumanDeploymentReviewSweeper.cs @@ -0,0 +1,141 @@ +using CodeyBox.Audit; +using CodeyBox.Core; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CodeyBox.Orchestrator; + +/// +/// Fail-closed backstop for parked human deployment reviews. Periodically +/// scans for undecided reviews past their deadline and, for each: tears the +/// held deployment down (bounded by the recipe's max lifetime — a forgotten +/// review must not hold a VM), marks the review expired, dismisses the +/// backing question as "expired unreviewed", and re-queues the item so the +/// audit loop resumes and records the blocking expiry finding through the +/// normal rework path. Silence never becomes an implicit pass. +/// +/// All dependencies are optional: unwired compositions no-op. Per-item +/// failures are caught and logged so one poisoned review cannot kill the +/// sweep. Reviews that already carry a verdict are never touched — the +/// resume path owns them. +/// +public sealed class HumanDeploymentReviewSweeper : BackgroundService +{ + private readonly IHumanDeploymentReviewStore? _reviews; + private readonly IDeploymentManager? _deployments; + private readonly IWorkItemStore? _store; + private readonly IWorkItemQuestionStore? _questions; + private readonly ITaskQueue? _queue; + private readonly IWebhookDispatcher? _webhooks; + private readonly Func _options; + private readonly TimeProvider _clock; + private readonly ILogger _log; + + public HumanDeploymentReviewSweeper( + IHumanDeploymentReviewStore? reviews = null, + IDeploymentManager? deployments = null, + IWorkItemStore? store = null, + IWorkItemQuestionStore? questions = null, + ITaskQueue? queue = null, + IWebhookDispatcher? webhooks = null, + Func? options = null, + TimeProvider? clock = null, + ILogger? log = null) + { + _reviews = reviews; + _deployments = deployments; + _store = store; + _questions = questions; + _queue = queue; + _webhooks = webhooks; + _options = options ?? (() => new HumanDeploymentReviewOptions()); + _clock = clock ?? TimeProvider.System; + _log = log ?? NullLogger.Instance; + } + + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + while (!stoppingToken.IsCancellationRequested) + { + var interval = _options().SweepInterval; + if (interval <= TimeSpan.Zero) + return; + try + { + await Task.Delay(interval, stoppingToken).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + break; + } + + await SweepOnceAsync(stoppingToken).ConfigureAwait(false); + } + } + + /// + /// Expires every undecided review past its deadline. Deterministic entry + /// point for tests (the background loop above just calls it on a timer). + /// + internal async Task SweepOnceAsync(CancellationToken ct) + { + if (_reviews is null) + return 0; + var now = _clock.GetUtcNow(); + var expired = await _reviews.ListExpiredPendingAsync(now, ct).ConfigureAwait(false); + var count = 0; + foreach (var review in expired) + { + try + { + if (await ExpireAsync(review, now, ct).ConfigureAwait(false)) + count++; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _log.LogWarning( + ex, + "Human-review sweeper failed to expire review for work item {WorkItemId} iteration {Iteration}", + review.WorkItemId, review.Iteration); + } + } + + return count; + } + + private async Task ExpireAsync(HumanDeploymentReview review, DateTimeOffset now, CancellationToken ct) + { + if (_reviews is null) + return false; + try + { + return await HumanReviewExpiry.ExpireAsync( + _reviews, + _deployments, + _store, + _questions, + _queue, + _webhooks, + review, + now, + _log, + ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _log.LogWarning( + ex, + "Human-review sweeper failed to expire review for work item {WorkItemId} iteration {Iteration}", + review.WorkItemId, review.Iteration); + return false; + } + } +} + +/// Structured payload for the resume webhook after a review expiry. +public sealed record HumanReviewExpiredDetails( + string WorkItemId, + int Iteration, + string DeploymentId, + DateTimeOffset ExpiredAt); diff --git a/src/CodeyBox.Orchestrator/HumanReviewExpiry.cs b/src/CodeyBox.Orchestrator/HumanReviewExpiry.cs new file mode 100644 index 000000000..4122bad4d --- /dev/null +++ b/src/CodeyBox.Orchestrator/HumanReviewExpiry.cs @@ -0,0 +1,126 @@ +using CodeyBox.Core; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CodeyBox.Orchestrator; + +/// +/// Shared fail-closed expiry for parked human deployment reviews, used by +/// both the background sweeper and the verdict endpoints (a late verdict +/// races the same path). For an undecided review past its deadline: tears +/// the held deployment down first (bounding the VM even if later steps +/// fail), marks the review expired, dismisses the backing question as +/// "expired unreviewed", and re-queues the item so the audit loop resumes +/// and records the blocking expiry finding. A verdict that wins the +/// compare-and-set race keeps its authority — expiry never overwrites a +/// recorded decision. +/// +public static class HumanReviewExpiry +{ + /// + /// Expires when it is still pending. Returns + /// false when the review already carries a verdict (the caller honours + /// the recorded decision instead). + /// + public static async Task ExpireAsync( + IHumanDeploymentReviewStore reviews, + IDeploymentManager? deployments, + IWorkItemStore? store, + IWorkItemQuestionStore? questions, + ITaskQueue? queue, + IWebhookDispatcher? webhooks, + HumanDeploymentReview review, + DateTimeOffset now, + ILogger? log = null, + CancellationToken ct = default) + { + ArgumentNullException.ThrowIfNull(reviews); + ArgumentNullException.ThrowIfNull(review); + var logger = log ?? NullLogger.Instance; + + // Bound the deployment first: even if every step below fails, the VM + // is gone and silence cannot become an implicit pass. + if (deployments is not null + && deployments.TryGetActive(review.DeploymentId, out var handle) + && handle is not null) + { + try + { + await handle.DisposeAsync().ConfigureAwait(false); + logger.LogInformation( + "Human-review expiry tore down deployment {DeploymentId} for work item {WorkItemId}", + review.DeploymentId, review.WorkItemId); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogWarning( + ex, + "Human-review expiry failed to tear down deployment {DeploymentId}; continuing with expiry", + review.DeploymentId); + } + } + + // CAS: a verdict that raced the expiry wins; the resume path owns it. + if (!await reviews.MarkExpiredAsync(review.WorkItemId, review.Iteration, now, ct).ConfigureAwait(false)) + return false; + + if (questions is not null) + { + try + { + await questions.DismissAsync( + review.WorkItemId, review.QuestionId, "expired unreviewed", ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogWarning(ex, "Human-review expiry failed to dismiss question {QuestionId}", review.QuestionId); + } + } + + if (store is null || queue is null) + return true; + + WorkItemId itemId; + try + { + itemId = WorkItemId.Parse(review.WorkItemId); + } + catch (Exception ex) when (ex is FormatException or ArgumentException or OverflowException) + { + logger.LogWarning(ex, "Human-review expiry could not parse work item id {WorkItemId}", review.WorkItemId); + return true; + } + + var current = await store.GetAsync(itemId, ct).ConfigureAwait(false); + if (current is null || current.State != WorkItemState.NeedsOperatorInput) + return true; + + var open = questions is null + ? [] + : await questions.ListByWorkItemAsync(review.WorkItemId, ct).ConfigureAwait(false); + if (open.Any(q => q.State == "open")) + return true; + + var resumed = current.With( + WorkItemState.WorkComplete, + $"Human deployment review expired unreviewed at {now:O}; resuming for fail-closed audit."); + if (!await store.TryUpdateIfStateAsync(resumed, WorkItemState.NeedsOperatorInput, ct).ConfigureAwait(false)) + return true; + + AuditLog.WorkItemTransitioned(itemId, "WorkComplete (human review expired unreviewed)"); + await queue.EnqueueAsync(itemId, ct).ConfigureAwait(false); + if (webhooks is not null) + { + await webhooks.PublishAsync(new WebhookEvent + { + Event = "work_item.work_complete", + WorkItem = resumed, + Project = null, + Details = new HumanReviewExpiredDetails( + review.WorkItemId, review.Iteration, review.DeploymentId, now), + }, CancellationToken.None).ConfigureAwait(false); + } + + return true; + } +} diff --git a/src/CodeyBox.Orchestrator/PipelineRunner.cs b/src/CodeyBox.Orchestrator/PipelineRunner.cs index c45ea8836..bbd81baa9 100644 --- a/src/CodeyBox.Orchestrator/PipelineRunner.cs +++ b/src/CodeyBox.Orchestrator/PipelineRunner.cs @@ -221,6 +221,11 @@ public sealed partial class PipelineRunner : IPipelineRunner // project actually enables the phase with a recipe and auditors. private readonly IDeploymentManager? _deploymentManager; private readonly IDeploymentSubstrateProvider? _deploymentSubstrates; + // Optional durable store for parked human deployment reviews. Null in + // compositions/tests that don't exercise human review; when null, a + // deployment stage containing a human-kind auditor records a + // configuration-shaped AuditUnavailableException (never a fake pass). + private readonly IHumanDeploymentReviewStore? _humanReviews; private readonly IMergeScopeResolver _mergeScopeResolver; private readonly Func _dispatchClaimIdFactory; private readonly string _disabledHostHooksPath; @@ -384,6 +389,10 @@ public PipelineRunner( // composition roots that enable deployment-stage auditing wire both. IDeploymentManager? deploymentManager = null, IDeploymentSubstrateProvider? deploymentSubstrates = null, + // Durable store for parked human deployment reviews. Null disables + // the human park/resume path; a human-kind auditor without a store + // fails loudly instead of parking into nowhere. + IHumanDeploymentReviewStore? humanReviews = null, StaleBaseConflictReworkRouter? staleBaseReworkRouter = null, NonDeterministicTestEscalationService? flakeEscalation = null, NonDeterministicTestEscalationSnapshot? flakeEscalationOptions = null) @@ -464,6 +473,7 @@ public PipelineRunner( _jobTrackExporter = jobTrackExporter; _deploymentManager = deploymentManager; _deploymentSubstrates = deploymentSubstrates; + _humanReviews = humanReviews; _mergeScopeResolver = mergeScopeResolver ?? NullMergeScopeResolver.Instance; _availability = availability; // Prefer the DI-injected handler when supplied: keeps the registry @@ -10901,6 +10911,47 @@ private async Task RunAuditLoopAsync( .ToList(); var startIteration = auditHistory.Count == 0 ? 1 : auditHistory.Max(h => h.Iteration) + 1; + // Human-review resume: a parked iteration never reached a verdict, so + // any persisted rows at/after its iteration are partial crash-style + // residue (in-progress snapshots, partial code findings), not rework + // evidence. Drop them from the in-memory history so the loop restarts + // at the parked iteration and the deployment stage consumes the + // recorded verdict (or re-parks while undecided) instead of + // misfiring the missing-audit resume-rework path. Earlier complete + // iterations stay as rework context. The stale rows are overwritten + // when the resumed iteration persists its final snapshot (the store + // upserts by (work item, attempt, iteration)). The check runs + // whenever the review store is wired (one indexed read per audit-loop + // entry — the loop itself only receives code-target auditors, so a + // human-kind auditor is never in `auditors`; the deployment stage + // composes its own panel). A mid-flight knob change that uncomposes + // the reviewer orphans the pending review to the expiry sweeper. + var humanResumeReview = await LoadActiveHumanReviewAsync(item.Id, ct); + if (humanResumeReview is not null + && currentWorkAttemptStartedAt is not null + && humanResumeReview.RequestedAt < currentWorkAttemptStartedAt) + { + // Stale across work attempts: a retry re-ran the work phase after + // the park, so the reviewed code may be gone. A verdict must + // never apply across attempts — expire the review fail-closed + // (bounded teardown, no silent pass) and audit the fresh code. + _log.LogWarning( + "Work item {Id}: human deployment review for iteration {Iteration} predates the current work attempt; expiring it and auditing fresh", + item.Id, humanResumeReview.Iteration); + await ExpireStaleHumanReviewAsync(item, humanResumeReview, ct).ConfigureAwait(false); + humanResumeReview = null; + } + + if (humanResumeReview is not null && humanResumeReview.Iteration >= startIteration) + { + var dropped = auditHistory.RemoveAll(h => h.Iteration >= humanResumeReview.Iteration); + if (dropped > 0) + _log.LogInformation( + "Work item {Id}: superseded {Count} partial audit-progress row(s) at/after parked human-review iteration {Iteration}", + item.Id, dropped, humanResumeReview.Iteration); + startIteration = auditHistory.Count == 0 ? 1 : auditHistory.Max(h => h.Iteration) + 1; + } + if (startIteration > maxIterations) return await HandleExhaustedPersistedAuditHistoryAsync(item, project, auditHistory, ct); @@ -10914,19 +10965,27 @@ private async Task RunAuditLoopAsync( if (hostShutdownToken.IsCancellationRequested) throw new OperationCanceledException(hostShutdownToken); - if (iteration > 1) + // Human-review resume: the parked iteration was code-clean, so + // re-running the rebase, mechanical fixers, and code stage would + // burn quota, mutate the reviewed tree, and risk diverging from + // the held deployment. Skip them; the deployment stage below + // consumes the recorded verdict (or re-parks while undecided). + var resumeHumanReview = humanResumeReview is not null + && humanResumeReview.Iteration == iteration; + if (!resumeHumanReview && iteration > 1) await MaybeIncrementalRebaseAsync(item, runner, repoId, baseBranch, workBranch, project, ct); - await RunMechanicalFixersAsync( - item, - project, - repoId, - baseBranch, - workBranch, - auditors, - iteration, - ct, - hostShutdownToken); + if (!resumeHumanReview) + await RunMechanicalFixersAsync( + item, + project, + repoId, + baseBranch, + workBranch, + auditors, + iteration, + ct, + hostShutdownToken); // Per-iteration audit phase scope. Disposed explicitly before the // rework scope (below) so codeybox.phase.duration_ms{phase=audit} @@ -11006,6 +11065,15 @@ await PersistAuditProgressAsync( _opts.TimeProvider.GetUtcNow()), progressCt).ConfigureAwait(false); }; + // Code stage is skipped on human-review resume (see above); the + // dangling open brace below is closed after the rebalance with + // the resume defaults. Indentation is intentionally unchanged to + // keep the diff reviewable. itemWasPlanned is declared here + // because the loop tail (metrics tags) reads it on both paths. + bool itemWasPlanned; + IReadOnlyList blocking; + if (!resumeHumanReview) + { try { revisionForCtx = await TryLookupIterationRevisionAsync(item.Id, iteration, ct); @@ -11122,13 +11190,32 @@ await PersistAuditProgressAsync( // reviewed. Objective gates keep full blocking authority; unplanned // items are unaffected. See PlannedItemAuditRebalance. var pipelineTuning = _pipelineTuning.Current; - var itemWasPlanned = HasReviewedPlanArtifact(item); - var blocking = PlannedItemAuditRebalance.SelectBlocking( + itemWasPlanned = HasReviewedPlanArtifact(item); + blocking = PlannedItemAuditRebalance.SelectBlocking( findings, project.Audit.FailingSeverity, itemWasPlanned, pipelineTuning.PlannedItemAuditRebalanceEnabled, pipelineTuning.PlannedItemAdvisoryAuditors).ToList(); + } + else + { + // Human-review resume defaults: the code stage was clean at + // park time, so the resumed iteration reuses a clean code + // verdict. Stored code findings are merged by the deployment + // stage consume path below (it owns the held deployment and + // the parked code outcome together). + findings = []; + activeAuditAgentKind = null; + declaredShortCircuitBlocking = false; + incompleteVerdict = false; + completedAuditors = []; + incompleteAuditors = []; + requiredBuildFinding = null; + iterationAttributions = []; + blocking = []; + itemWasPlanned = HasReviewedPlanArtifact(item); + } // Cost-ordered ladder rung 2: deployment stage (lazy). // Runs only when the code stage above reached a complete verdict @@ -11145,7 +11232,7 @@ await PersistAuditProgressAsync( DeploymentStageOutcome? deploymentStage; try { - deploymentStage = await RunDeploymentStageAsync( + var humanStage = await RunDeploymentStageAsync( item, project, runner, @@ -11158,8 +11245,21 @@ await PersistAuditProgressAsync( auditShortCircuitEnabled, progressUpdateWithPreCollected, scheduledAuditorNames, + findings, + completedAuditors, codeStageClean: true, auditPhase.Token); + if (humanStage.Parked) + { + // The iteration parked for human review: the worker + // slot and audit sandbox are released here (this + // return unwinds them) while only the deployment stays + // alive. Resume happens on verdict or expiry. + auditPhaseScope.Dispose(); + return true; + } + + deploymentStage = humanStage.Outcome; } catch (OperationCanceledException oce) when (oce is not PhaseCancellationException) { @@ -12724,7 +12824,7 @@ private sealed record DeploymentStageOutcome( /// incomplete iteration, never a fake pass. A lost handle across an /// orchestrator restart is swept by the deployment leak reaper. /// - private async Task RunDeploymentStageAsync( + private async Task<(DeploymentStageOutcome? Outcome, bool Parked)> RunDeploymentStageAsync( WorkItem item, Project project, IAgentRunner runner, @@ -12737,6 +12837,8 @@ private sealed record DeploymentStageOutcome( bool auditShortCircuitEnabled, Func progressUpdate, List scheduledAuditorNames, + IReadOnlyList codeFindings, + IReadOnlyList codeCompletedAuditors, bool codeStageClean, CancellationToken auditToken) { @@ -12753,7 +12855,7 @@ private sealed record DeploymentStageOutcome( iteration, item.Id, decision.Reason); - return null; + return (null, false); } var recipe = project.Deployment!; @@ -12769,6 +12871,33 @@ private sealed record DeploymentStageOutcome( var ordered = auditShortCircuitEnabled ? AuditPhaseLadder.OrderDeploymentStage(deploymentAuditors) : deploymentAuditors; + var humanAuditors = ordered.Where(IsHumanKindAuditor).ToList(); + if (humanAuditors.Count > 0) + { + // Async human review: provision, park, and resume. Automated + // deployment auditors still run inline (before parking and never + // after), but human-kind auditors never run inline — the verdict + // arrives via the operator park/resume path. + return await RunHumanDeploymentStageAsync( + item, + project, + runner, + repoId, + baseBranch, + workBranch, + iteration, + promptRevisionAtDispatch, + priorBlockingFindings, + auditShortCircuitEnabled, + progressUpdate, + scheduledAuditorNames, + codeFindings, + codeCompletedAuditors, + humanAuditors, + ordered.Where(a => !IsHumanKindAuditor(a)).ToList(), + auditToken).ConfigureAwait(false); + } + _log.LogInformation( "Audit iteration {Iteration} for work item {WorkItemId}: provisioning one '{Kind}' deployment for {Count} deployment auditor(s)", iteration, @@ -12825,7 +12954,7 @@ private sealed record DeploymentStageOutcome( var blocking = collection.Findings .Where(f => f.Severity >= project.Audit.FailingSeverity) .ToList(); - return new DeploymentStageOutcome( + return (new DeploymentStageOutcome( collection.Findings, blocking, collection.CompletedAuditors ?? [], @@ -12833,7 +12962,627 @@ private sealed record DeploymentStageOutcome( collection.ActiveAuditAgentKind, collection.DeclaredShortCircuitBlocking, collection.IncompleteVerdict, - deployment.Endpoint); + deployment.Endpoint), false); + } + + /// + /// True when the auditor is a human reviewer (declared + /// Kind = "human"). Human reviewers never run inline; the + /// deployment stage parks for their verdict instead. + /// + private static bool IsHumanKindAuditor(IAuditor auditor) + => string.Equals(auditor.Kind, WellKnownAuditorKinds.Human, StringComparison.OrdinalIgnoreCase); + + /// + /// Loads the work item's active (non-consumed) human review, if any. + /// Null when the store is unwired or no review is outstanding. + /// + private Task LoadActiveHumanReviewAsync(WorkItemId id, CancellationToken ct) + => _humanReviews is null + ? Task.FromResult(null) + : _humanReviews.GetActiveForWorkItemAsync(id.ToString(), ct); + + /// + /// Deployment stage with at least one human-kind auditor. Automated + /// deployment auditors run inline against a freshly provisioned + /// deployment; when they are clean the iteration parks for the human + /// verdict (releasing the worker slot and audit sandbox while keeping + /// only the deployment alive), and a resumed iteration consumes the + /// recorded verdict or expiry instead of provisioning. Returns + /// Parked: true when the caller must unwind the iteration + /// (worker slot released); the deployment stays live in the manager's + /// active set in that case and is torn down on verdict or expiry. + /// + private async Task<(DeploymentStageOutcome? Outcome, bool Parked)> RunHumanDeploymentStageAsync( + WorkItem item, + Project project, + IAgentRunner runner, + string repoId, + string baseBranch, + string workBranch, + int iteration, + int? promptRevisionAtDispatch, + IReadOnlyList? priorBlockingFindings, + bool auditShortCircuitEnabled, + Func progressUpdate, + List scheduledAuditorNames, + IReadOnlyList codeFindings, + IReadOnlyList codeCompletedAuditors, + IReadOnlyList humanAuditors, + IReadOnlyList automatedAuditors, + CancellationToken auditToken) + { + if (_humanReviews is null) + { + throw new AuditUnavailableException( + $"audit iteration {iteration} requires the human deployment-review stage " + + $"({humanAuditors.Count} human auditor(s)) but no IHumanDeploymentReviewStore is wired " + + "into the pipeline. Wire the review store or remove the human auditor."); + } + + if (_questionStore is null) + { + throw new AuditUnavailableException( + $"audit iteration {iteration} requires the human deployment-review stage but no " + + "IWorkItemQuestionStore is wired into the pipeline — the operator verdict travels " + + "through the question/answer plumbing. Wire the question store or remove the human auditor."); + } + + var humanNames = humanAuditors.Select(a => a.Name).ToList(); + var now = _opts.TimeProvider.GetUtcNow(); + var existing = await _humanReviews.TryGetAsync(item.Id.ToString(), iteration, auditToken) + .ConfigureAwait(false); + if (existing is { ConsumedAt: not null }) + existing = null; + existing ??= await _humanReviews.GetActiveForWorkItemAsync(item.Id.ToString(), auditToken) + .ConfigureAwait(false) is { ConsumedAt: null } stale + ? stale + : null; + if (existing is not null) + { + if (existing.Iteration != iteration) + { + // Unreachable without manual state surgery (the loop only + // advances past a consumed or re-parked iteration): never + // apply a verdict to an iteration whose code it did not + // review. Expire it fail-closed and provision fresh. + _log.LogWarning( + "Work item {Id}: human deployment review for iteration {ReviewIteration} does not match loop iteration {Iteration}; expiring it and auditing fresh", + item.Id, existing.Iteration, iteration); + await ExpireStaleHumanReviewAsync(item, existing, auditToken).ConfigureAwait(false); + existing = null; + } + } + + if (existing is not null) + { + // Resume path: consume the verdict/expiry, or re-park while + // undecided — never provision a second deployment for the parked + // iteration. + return await ConsumeOrReparkHumanReviewAsync( + item, project, existing, humanNames, now, auditToken).ConfigureAwait(false); + } + + var recipe = project.Deployment!; + var requestedAt = _opts.TimeProvider.GetUtcNow(); + _log.LogInformation( + "Audit iteration {Iteration} for work item {WorkItemId}: provisioning one '{Kind}' deployment for {Auto} automated + {Human} human deployment auditor(s)", + iteration, + item.Id, + recipe.Kind, + automatedAuditors.Count, + humanAuditors.Count); + + await using var deployment = await DeploymentAuditScope.ProvisionAsync( + _deploymentManager!, + _deploymentSubstrates!, + project, + recipe, + () => _opts.TimeProvider.GetUtcNow(), + _log, + auditToken).ConfigureAwait(false); + using var lifetimeCts = deployment.LinkLifetime(auditToken, () => _opts.TimeProvider.GetUtcNow()); + + scheduledAuditorNames.AddRange(automatedAuditors.Select(a => a.Name)); + scheduledAuditorNames.AddRange(humanNames); + var deploymentCtx = new AuditContext( + item.Id, + workBranch, + baseBranch, + iteration, + item.Prompt, + ModelId: item.ModelId, + ReasoningMode: item.ReasoningMode, + PromptRevisionAtDispatch: promptRevisionAtDispatch, + BuildScriptRequired: project.Audit.BuildScriptRequired, + ProjectId: project.Id.Value, + Target: AuditTarget.Deployment, + PlanArtifact: item.PlanArtifact, + PriorBlockingFindings: priorBlockingFindings, + DeploymentEndpoint: deployment.Endpoint); + + var collection = await CollectFindingsAsync( + item, + project, + runner, + automatedAuditors, + repoId, + deploymentCtx, + auditShortCircuitEnabled, + BuildTestGateEvidence.None, + progressUpdate, + lifetimeCts.Token).ConfigureAwait(false); + + var blocking = collection.Findings + .Where(f => f.Severity >= project.Audit.FailingSeverity) + .ToList(); + if (blocking.Count > 0 || collection.IncompleteVerdict) + { + // Automated stage dirty or incomplete: ordinary outcome, no human + // park — the operator is only asked to review deployments the + // automated probes accept. The scope tears the deployment down. + return (new DeploymentStageOutcome( + collection.Findings, + blocking, + collection.CompletedAuditors ?? [], + collection.IncompleteAuditors ?? [], + collection.ActiveAuditAgentKind, + collection.DeclaredShortCircuitBlocking, + collection.IncompleteVerdict, + deployment.Endpoint), false); + } + + var deadline = DeploymentAuditPolicy.DeadlineFor(recipe, requestedAt); + var brief = await BuildHumanReviewBriefAsync(item, deployment.Endpoint, deadline, iteration, deployment.Handle.Id, auditToken) + .ConfigureAwait(false); + var pending = new HumanDeploymentReview + { + WorkItemId = item.Id.ToString(), + Iteration = iteration, + DeploymentId = deployment.Handle.Id, + EndpointJson = JsonSerializer.Serialize(deployment.Endpoint), + Deadline = deadline, + RequestedAt = requestedAt, + Brief = brief, + QuestionId = HumanDeploymentReviewPolicy.QuestionIdFor(iteration), + HumanAuditorsJson = HumanDeploymentReviewPolicy.SerializeStrings(humanNames), + CodeFindingsJson = HumanDeploymentReviewPolicy.SerializeFindings(codeFindings), + CodeCompletedJson = HumanDeploymentReviewPolicy.SerializeStrings(codeCompletedAuditors), + AutomatedFindingsJson = HumanDeploymentReviewPolicy.SerializeFindings(collection.Findings), + AutomatedCompletedJson = HumanDeploymentReviewPolicy.SerializeStrings( + collection.CompletedAuditors ?? []), + AutomatedIncompleteJson = HumanDeploymentReviewPolicy.SerializeStrings( + collection.IncompleteAuditors ?? []), + ActiveAuditAgentKind = collection.ActiveAuditAgentKind?.Value, + DeclaredShortCircuitBlocking = collection.DeclaredShortCircuitBlocking, + IncompleteVerdict = collection.IncompleteVerdict, + }; + var effective = await _humanReviews.GetOrCreatePendingAsync(pending, auditToken).ConfigureAwait(false); + + var parked = await ParkForHumanReviewAsync(item, project, effective, iteration, auditToken) + .ConfigureAwait(false); + if (!parked) + { + // Lost a concurrent state transition: the scope tears the + // deployment down on unwind, and the loud incomplete verdict + // below keeps the human gate from being skipped silently. + throw new AuditUnavailableException( + $"audit iteration {iteration} could not park for human deployment review: " + + "the work item state changed concurrently. The deployment was torn down; " + + "re-queue the item to restart the review."); + } + + // Parked: keep ONLY the deployment alive. Detaching transfers + // teardown ownership to the review record — the resume path or the + // expiry sweeper tears it down by re-attaching through the manager. + deployment.Detach(); + return (null, true); + } + + /// + /// Resume path for a parked iteration: consumes a decided/expired review + /// into a deployment-stage outcome (tearing the held deployment down + /// immediately), or re-parks while the review is still undecided without + /// provisioning again. + /// + private async Task<(DeploymentStageOutcome? Outcome, bool Parked)> ConsumeOrReparkHumanReviewAsync( + WorkItem item, + Project project, + HumanDeploymentReview review, + IReadOnlyList humanNames, + DateTimeOffset now, + CancellationToken ct) + { + if (review.Status == HumanDeploymentReviewStatus.Pending && now >= review.Deadline) + { + if (await _humanReviews!.MarkExpiredAsync( + review.WorkItemId, review.Iteration, now, ct).ConfigureAwait(false)) + { + review = review with + { + Status = HumanDeploymentReviewStatus.Expired, + DecidedAt = now, + }; + _log.LogWarning( + "Work item {Id}: human deployment review for iteration {Iteration} expired unreviewed at {Deadline}; failing closed", + item.Id, review.Iteration, review.Deadline); + } + else + { + // A verdict raced the expiry: honour the recorded verdict. + review = await _humanReviews.TryGetAsync( + review.WorkItemId, review.Iteration, ct).ConfigureAwait(false) + ?? review; + } + } + + if (review.Status == HumanDeploymentReviewStatus.Pending) + { + // Still undecided and within deadline: re-park without + // provisioning. The held deployment must still be alive — after + // an orchestrator restart the handle is gone and the review + // fails closed instead of verifying a dead endpoint. + if (_deploymentManager is null + || !_deploymentManager.TryGetActive(review.DeploymentId, out _)) + { + _log.LogWarning( + "Work item {Id}: held deployment {DeploymentId} for human review is gone; expiring the review", + item.Id, review.DeploymentId); + await _humanReviews!.MarkExpiredAsync( + review.WorkItemId, review.Iteration, now, ct).ConfigureAwait(false); + review = review with + { + Status = HumanDeploymentReviewStatus.Expired, + DecidedAt = now, + }; + } + else + { + var reparked = await ParkForHumanReviewAsync(item, project, review, review.Iteration, ct) + .ConfigureAwait(false); + return (null, reparked); + } + } + + var outcome = BuildConsumedHumanOutcome(project, review, humanNames); + await TearDownHeldDeploymentAsync(item, review, ct).ConfigureAwait(false); + await _humanReviews!.MarkConsumedAsync(review.WorkItemId, review.Iteration, now, ct) + .ConfigureAwait(false); + return (outcome, false); + } + + /// + /// Merges the parked code + automated findings with the human verdict + /// into the iteration's deployment-stage outcome. Corrupt stored payloads + /// fail closed with a blocking finding rather than passing. + /// + private static DeploymentStageOutcome BuildConsumedHumanOutcome( + Project project, + HumanDeploymentReview review, + IReadOnlyList humanNames) + { + var storedHumanNames = TryDeserializeStrings(review.HumanAuditorsJson); + var auditorName = storedHumanNames.Count > 0 + ? storedHumanNames[0] + : humanNames.Count > 0 + ? humanNames[0] + : WellKnownAuditorNames.HumanDeploymentReview; + var completedHumans = storedHumanNames.Count > 0 ? storedHumanNames : humanNames; + + IReadOnlyList codeFindings = []; + IReadOnlyList automatedFindings = []; + var corrupt = new List(); + try + { + codeFindings = HumanDeploymentReviewPolicy.DeserializeFindings(review.CodeFindingsJson); + } + catch (InvalidOperationException ex) + { + corrupt.Add(new AuditFinding( + auditorName, AuditSeverity.Error, + "Stored human-review code findings are unreadable", + $"The parked code-stage findings for iteration {review.Iteration} could not be decoded ({ex.Message}). Failing closed: the deployment was torn down.")); + } + + try + { + automatedFindings = HumanDeploymentReviewPolicy.DeserializeFindings(review.AutomatedFindingsJson); + } + catch (InvalidOperationException ex) + { + corrupt.Add(new AuditFinding( + auditorName, AuditSeverity.Error, + "Stored human-review deployment findings are unreadable", + $"The parked automated deployment-stage findings for iteration {review.Iteration} could not be decoded ({ex.Message}). Failing closed: the deployment was torn down.")); + } + + var verdictFindings = HumanDeploymentReviewPolicy.BuildVerdictFindings( + auditorName, review.Status, review.Notes, review.Deadline); + IReadOnlyList findings = + [.. codeFindings, .. automatedFindings, .. verdictFindings, .. corrupt]; + var blocking = findings + .Where(f => f.Severity >= project.Audit.FailingSeverity) + .ToList(); + + DeploymentEndpoint? endpoint; + try + { + endpoint = JsonSerializer.Deserialize(review.EndpointJson); + } + catch (JsonException) + { + endpoint = null; + } + + IReadOnlyList automatedCompleted; + IReadOnlyList automatedIncomplete; + IReadOnlyList codeCompleted; + try + { + automatedCompleted = HumanDeploymentReviewPolicy.DeserializeStrings(review.AutomatedCompletedJson); + automatedIncomplete = HumanDeploymentReviewPolicy.DeserializeStrings(review.AutomatedIncompleteJson); + codeCompleted = HumanDeploymentReviewPolicy.DeserializeStrings(review.CodeCompletedJson); + } + catch (InvalidOperationException) + { + automatedCompleted = []; + automatedIncomplete = []; + codeCompleted = []; + blocking.Add(new AuditFinding( + auditorName, AuditSeverity.Error, + "Stored human-review auditor lists are unreadable", + $"The parked auditor lists for iteration {review.Iteration} could not be decoded. Failing closed: the deployment was torn down.")); + } + + return new DeploymentStageOutcome( + findings, + blocking, + [.. codeCompleted, .. automatedCompleted, .. completedHumans], + automatedIncomplete, + string.IsNullOrWhiteSpace(review.ActiveAuditAgentKind) + ? null + : new AgentKind(review.ActiveAuditAgentKind), + review.DeclaredShortCircuitBlocking, + review.IncompleteVerdict, + endpoint ?? new DeploymentEndpoint { Kind = DeploymentEndpointKind.Http }); + } + + private static IReadOnlyList TryDeserializeStrings(string json) + { + try + { + return HumanDeploymentReviewPolicy.DeserializeStrings(json); + } + catch (InvalidOperationException) + { + return []; + } + } + + /// + /// Retires a review that must never be consumed: stale across work + /// attempts, or addressing an iteration the loop has left. Tears the + /// held deployment down (bounded), marks the review expired, consumes it + /// out of the active set, and dismisses the backing question — without + /// applying its verdict anywhere. Conservative by construction: a + /// discarded approval only ever causes a re-review, never a pass. + /// + private async Task ExpireStaleHumanReviewAsync( + WorkItem item, HumanDeploymentReview review, CancellationToken ct) + { + var now = _opts.TimeProvider.GetUtcNow(); + await TearDownHeldDeploymentAsync(item, review, ct).ConfigureAwait(false); + if (_humanReviews is null) + return; + await _humanReviews.MarkExpiredAsync(review.WorkItemId, review.Iteration, now, ct) + .ConfigureAwait(false); + await _humanReviews.MarkConsumedAsync(review.WorkItemId, review.Iteration, now, ct) + .ConfigureAwait(false); + if (_questionStore is not null) + { + try + { + await _questionStore.DismissAsync( + review.WorkItemId, review.QuestionId, "superseded", ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _log.LogWarning( + ex, "Work item {Id}: failed to dismiss superseded human-review question {QuestionId}", + item.Id, review.QuestionId); + } + } + } + + /// + /// Best-effort teardown of the deployment held for a consumed review. + /// Missing handles (restart, sweeper) are fine — teardown is idempotent + /// and the verdict stands regardless. + /// + private async Task TearDownHeldDeploymentAsync( WorkItem item, HumanDeploymentReview review, CancellationToken ct) + { + if (_deploymentManager is null + || !_deploymentManager.TryGetActive(review.DeploymentId, out var handle) + || handle is null) + { + _log.LogInformation( + "Work item {Id}: held deployment {DeploymentId} already gone; consume continues without teardown", + item.Id, review.DeploymentId); + return; + } + + try + { + await handle.DisposeAsync().ConfigureAwait(false); + _log.LogInformation( + "Work item {Id}: tore down held deployment {DeploymentId} after consuming human review", + item.Id, review.DeploymentId); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + // Teardown failure must not flip the recorded verdict: the loop + // continues with the verdict outcome and the leak reaper remains + // the safety net for the orphaned substrate. + _log.LogWarning( + ex, + "Work item {Id}: failed to tear down held deployment {DeploymentId}; verdict stands, leak reaper owns the substrate", + item.Id, review.DeploymentId); + } + } + + /// + /// Parks the iteration for human review: creates the backing operator + /// question (endpoint + expiry + acceptance criteria), notifies via the + /// existing question/webhook plumbing, and transitions the item to + /// NeedsOperatorInput — releasing the worker slot and audit + /// sandbox. NeedsOperatorInput is outside both watchdogs' + /// watched states, so a parked-on-human item is never flagged stalled; + /// the transition message annotates the park. Returns false when a + /// concurrent state change made the transition impossible. + /// + private async Task ParkForHumanReviewAsync( + WorkItem item, + Project project, + HumanDeploymentReview review, + int iteration, + CancellationToken ct) + { + var utcNow = _opts.TimeProvider.GetUtcNow(); + var created = await _questionStore!.CreateIfNotExistsAsync(new WorkItemQuestion + { + Id = Guid.NewGuid().ToString(), + WorkItemId = item.Id.ToString(), + QuestionId = review.QuestionId, + QuestionText = review.Brief, + AskedAt = utcNow, + }, ct).ConfigureAwait(false); + + var fresh = await _store.GetAsync(item.Id, ct).ConfigureAwait(false) ?? item; + if (created) + { + AuditLog.WorkItemTransitioned(item.Id, $"question_asked:{review.QuestionId}"); + await _webhooks.PublishAsync(new WebhookEvent + { + Event = "work_item.question_asked", + WorkItem = fresh, + Project = project, + Details = new QuestionAskedDetails( + item.Id.ToString(), project.Id.Value, review.QuestionId, review.Brief), + }, CancellationToken.None).ConfigureAwait(false); + } + + var endpointDescription = HumanDeploymentReviewPolicy.DescribeEndpoint(TryParseEndpoint(review.EndpointJson)); + var message = + $"Parked for human deployment review (iteration {iteration}): deployment {review.DeploymentId} " + + $"at {endpointDescription} awaiting operator verdict; expires {review.Deadline:O}."; + var parked = false; + await RunBoundedPostAgentAsync(item.Id, "audit-human-review-park", ct, async transitionCt => + { + var current = await _store.GetAsync(item.Id, transitionCt).ConfigureAwait(false) ?? item; + if (current.State != WorkItemState.Auditing) + { + _log.LogInformation( + "Work item {Id} left Auditing concurrently ({State}); skipping human-review park", + item.Id, current.State); + return; + } + + var updated = await _store.TryUpdateIfStateAsync( + current.With(WorkItemState.NeedsOperatorInput, message), + WorkItemState.Auditing, + transitionCt).ConfigureAwait(false); + if (!updated) + { + _log.LogInformation( + "Work item {Id} state changed concurrently; skipping human-review park", + item.Id); + return; + } + + parked = true; + _log.LogWarning( + "Work item {Id} parked at iteration {Iteration} for human deployment review: {DeploymentId} expires {Deadline}", + item.Id, iteration, review.DeploymentId, review.Deadline); + AuditLog.WorkItemTransitioned(item.Id, $"NeedsOperatorInput (human deployment review; {review.DeploymentId})"); + CodeyBoxMeters.PipelineTransitions.Add(1, + new KeyValuePair("to_state", WorkItemState.NeedsOperatorInput.ToString())); + + await _webhooks.PublishAsync(new WebhookEvent + { + Event = "work_item.needs_operator_input", + WorkItem = current.With(WorkItemState.NeedsOperatorInput, message), + Project = project, + Details = new HumanReviewParkedDetails( + item.Id.ToString(), + project.Id.Value, + iteration, + review.DeploymentId, + endpointDescription, + review.Deadline, + review.QuestionId), + }, CancellationToken.None).ConfigureAwait(false); + }).ConfigureAwait(false); + + return parked; + } + + private static DeploymentEndpoint? TryParseEndpoint(string json) + { + try + { + return JsonSerializer.Deserialize(json); + } + catch (JsonException) + { + return null; + } + } + + /// + /// Builds the operator brief from the live endpoint, the deadline, and + /// the item's acceptance criteria (linked test cases, bounded). Pure + /// assembly — see + /// for the truncation contract. + /// + private async Task BuildHumanReviewBriefAsync( + WorkItem item, + DeploymentEndpoint endpoint, + DateTimeOffset deadline, + int iteration, + string deploymentId, + CancellationToken ct) + { + var fresh = await _store.GetAsync(item.Id, ct).ConfigureAwait(false) ?? item; + var criteria = new List<(string Name, string Description)>(); + if (_testCaseStore is not null) + { + try + { + await foreach (var testCase in _testCaseStore + .ListByWorkItemAsync(item.Id.ToString(), ct).ConfigureAwait(false)) + { + if (testCase.IsArchived) continue; + criteria.Add((testCase.Name, testCase.Description)); + if (criteria.Count >= HumanDeploymentReviewPolicy.MaxCriteriaEntries) break; + } + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + // Criteria are advisory context for the operator, not the + // verdict: a store failure degrades the brief instead of the + // review. + _log.LogWarning(ex, "Work item {Id}: failed to load test cases for human-review brief", item.Id); + } + } + + return HumanDeploymentReviewPolicy.BuildBrief( + fresh.Title, + fresh.Prompt, + HumanDeploymentReviewPolicy.DescribeEndpoint(endpoint), + deadline, + criteria, + iteration, + deploymentId); } private async Task CollectFindingsAsync( @@ -22434,6 +23183,21 @@ internal sealed record QuestionAskedDetails( string QuestionId, string QuestionText); +/// +/// Structured payload for work_item.needs_operator_input when the +/// park reason is a human deployment review: the operator's endpoint, +/// expiry, and backing question in one place (the full acceptance-criteria +/// brief travels on the question itself). +/// +internal sealed record HumanReviewParkedDetails( + string WorkItemId, + string ProjectId, + int Iteration, + string DeploymentId, + string Endpoint, + DateTimeOffset ExpiresAt, + string QuestionId); + public sealed record QuestionAnsweredDetails( string WorkItemId, string ProjectId, diff --git a/src/CodeyBox.Orchestrator/SqliteHumanDeploymentReviewStore.cs b/src/CodeyBox.Orchestrator/SqliteHumanDeploymentReviewStore.cs new file mode 100644 index 000000000..152d30fe2 --- /dev/null +++ b/src/CodeyBox.Orchestrator/SqliteHumanDeploymentReviewStore.cs @@ -0,0 +1,323 @@ +using System.Globalization; +using Microsoft.Data.Sqlite; +using CodeyBox.Core; + +namespace CodeyBox.Orchestrator; + +/// +/// SQLite-backed store for parked human deployment reviews. Shares the state +/// database file (same WAL/busy-timeout conventions as +/// ) so reviews live alongside the +/// work items and questions they reference. +/// +public sealed class SqliteHumanDeploymentReviewStore : IHumanDeploymentReviewStore, IDisposable +{ + private readonly SqliteConnection _conn; + private readonly SqliteDatabaseWriteGate _writeLock; + + public SqliteHumanDeploymentReviewStore( + string path, + SqliteDatabaseWriteGateFactory? writeGateFactory = null) + { + var dir = Path.GetDirectoryName(path); + if (!string.IsNullOrEmpty(dir)) Directory.CreateDirectory(dir); + + _conn = new SqliteConnection($"Data Source={path}"); + _writeLock = SqliteDatabaseWriteGateFactory.Resolve(writeGateFactory).ForPath(path); + _writeLock.Wait(); + try + { + _conn.Open(); + + using (var pragmaCmd = _conn.CreateCommand()) + { + pragmaCmd.CommandText = "PRAGMA journal_mode=WAL; PRAGMA busy_timeout=30000; PRAGMA foreign_keys=ON;"; + pragmaCmd.ExecuteNonQuery(); + } + + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + CREATE TABLE IF NOT EXISTS human_deployment_reviews ( + work_item_id TEXT NOT NULL, + iteration INTEGER NOT NULL, + deployment_id TEXT NOT NULL, + endpoint_json TEXT NOT NULL, + deadline TEXT NOT NULL, + requested_at TEXT NOT NULL, + brief TEXT NOT NULL, + question_id TEXT NOT NULL, + code_findings_json TEXT NOT NULL DEFAULT '[]', + code_completed_json TEXT NOT NULL DEFAULT '[]', + human_auditors_json TEXT NOT NULL, + automated_findings_json TEXT NOT NULL, + automated_completed_json TEXT NOT NULL, + automated_incomplete_json TEXT NOT NULL, + active_audit_agent_kind TEXT, + declared_short_circuit INTEGER NOT NULL DEFAULT 0, + incomplete_verdict INTEGER NOT NULL DEFAULT 0, + status INTEGER NOT NULL DEFAULT 0, + notes TEXT, + decided_at TEXT, + decided_by TEXT, + consumed_at TEXT, + PRIMARY KEY (work_item_id, iteration) + ); + CREATE INDEX IF NOT EXISTS idx_human_reviews_pending + ON human_deployment_reviews(status, deadline); + """; + cmd.ExecuteNonQuery(); + } + finally + { + _writeLock.Release(); + } + } + + public async Task GetOrCreatePendingAsync(HumanDeploymentReview review, CancellationToken ct = default) + { + ArgumentNullException.ThrowIfNull(review); + await _writeLock.WaitAsync(ct); + try + { + var existing = await TryGetAsync(review.WorkItemId, review.Iteration, ct).ConfigureAwait(false); + if (existing is not null && existing.ConsumedAt is null) + return existing; + + if (existing is not null) + { + using var delete = _conn.CreateCommand(); + delete.CommandText = """ + DELETE FROM human_deployment_reviews + WHERE work_item_id = $wid AND iteration = $iter; + """; + delete.Parameters.AddWithValue("$wid", review.WorkItemId); + delete.Parameters.AddWithValue("$iter", review.Iteration); + await delete.ExecuteNonQueryAsync(ct).ConfigureAwait(false); + } + + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + INSERT INTO human_deployment_reviews + (work_item_id, iteration, deployment_id, endpoint_json, deadline, + requested_at, brief, question_id, code_findings_json, code_completed_json, human_auditors_json, + automated_findings_json, automated_completed_json, automated_incomplete_json, + active_audit_agent_kind, declared_short_circuit, incomplete_verdict, + status, notes, decided_at, decided_by, consumed_at) + VALUES ($wid, $iter, $dep, $ep, $deadline, $requested, $brief, $qid, + $codefindings, $codecompleted, $humans, $findings, $completed, $incomplete, $agent, + $shortcircuit, $incompleteverdict, $status, $notes, $decidedat, $decidedby, $consumedat); + """; + Bind(cmd, review); + await cmd.ExecuteNonQueryAsync(ct).ConfigureAwait(false); + return review; + } + finally + { + _writeLock.Release(); + } + } + + public async Task TryGetAsync(string workItemId, int iteration, CancellationToken ct = default) + { + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + SELECT * FROM human_deployment_reviews + WHERE work_item_id = $wid AND iteration = $iter; + """; + cmd.Parameters.AddWithValue("$wid", workItemId); + cmd.Parameters.AddWithValue("$iter", iteration); + using var reader = await cmd.ExecuteReaderAsync(ct).ConfigureAwait(false); + return await reader.ReadAsync(ct).ConfigureAwait(false) ? Read(reader) : null; + } + + public async Task GetActiveForWorkItemAsync(string workItemId, CancellationToken ct = default) + { + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + SELECT * FROM human_deployment_reviews + WHERE work_item_id = $wid AND consumed_at IS NULL + ORDER BY iteration DESC + LIMIT 1; + """; + cmd.Parameters.AddWithValue("$wid", workItemId); + using var reader = await cmd.ExecuteReaderAsync(ct).ConfigureAwait(false); + return await reader.ReadAsync(ct).ConfigureAwait(false) ? Read(reader) : null; + } + + public async Task RecordVerdictAsync( + string workItemId, + int iteration, + bool approved, + string? notes, + string? decidedBy, + DateTimeOffset decidedAt, + CancellationToken ct = default) + { + await _writeLock.WaitAsync(ct); + try + { + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + UPDATE human_deployment_reviews + SET status = $status, + notes = $notes, + decided_at = $decidedat, + decided_by = $decidedby + WHERE work_item_id = $wid AND iteration = $iter AND status = 0; + """; + cmd.Parameters.AddWithValue("$status", (int)(approved + ? HumanDeploymentReviewStatus.Approved + : HumanDeploymentReviewStatus.Rejected)); + cmd.Parameters.AddWithValue("$notes", (object?)notes ?? DBNull.Value); + cmd.Parameters.AddWithValue("$decidedat", decidedAt.ToString("O")); + cmd.Parameters.AddWithValue("$decidedby", (object?)decidedBy ?? DBNull.Value); + cmd.Parameters.AddWithValue("$wid", workItemId); + cmd.Parameters.AddWithValue("$iter", iteration); + return await cmd.ExecuteNonQueryAsync(ct).ConfigureAwait(false) > 0; + } + finally + { + _writeLock.Release(); + } + } + + public async Task MarkExpiredAsync(string workItemId, int iteration, DateTimeOffset expiredAt, CancellationToken ct = default) + { + await _writeLock.WaitAsync(ct); + try + { + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + UPDATE human_deployment_reviews + SET status = 3, + decided_at = $decidedat + WHERE work_item_id = $wid AND iteration = $iter AND status = 0; + """; + cmd.Parameters.AddWithValue("$decidedat", expiredAt.ToString("O")); + cmd.Parameters.AddWithValue("$wid", workItemId); + cmd.Parameters.AddWithValue("$iter", iteration); + return await cmd.ExecuteNonQueryAsync(ct).ConfigureAwait(false) > 0; + } + finally + { + _writeLock.Release(); + } + } + + public async Task MarkConsumedAsync(string workItemId, int iteration, DateTimeOffset consumedAt, CancellationToken ct = default) + { + await _writeLock.WaitAsync(ct); + try + { + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + UPDATE human_deployment_reviews + SET consumed_at = $consumedat + WHERE work_item_id = $wid AND iteration = $iter AND consumed_at IS NULL; + """; + cmd.Parameters.AddWithValue("$consumedat", consumedAt.ToString("O")); + cmd.Parameters.AddWithValue("$wid", workItemId); + cmd.Parameters.AddWithValue("$iter", iteration); + await cmd.ExecuteNonQueryAsync(ct).ConfigureAwait(false); + } + finally + { + _writeLock.Release(); + } + } + + public async Task> ListExpiredPendingAsync(DateTimeOffset now, CancellationToken ct = default) + { + using var cmd = _conn.CreateCommand(); + cmd.CommandText = """ + SELECT * FROM human_deployment_reviews + WHERE status = 0 AND deadline <= $now + ORDER BY deadline ASC; + """; + cmd.Parameters.AddWithValue("$now", now.ToString("O")); + using var reader = await cmd.ExecuteReaderAsync(ct).ConfigureAwait(false); + var results = new List(); + while (await reader.ReadAsync(ct).ConfigureAwait(false)) + results.Add(Read(reader)); + return results; + } + + public void Dispose() + { + _conn.Dispose(); + _writeLock.Dispose(); + } + + private static void Bind(SqliteCommand cmd, HumanDeploymentReview review) + { + cmd.Parameters.AddWithValue("$wid", review.WorkItemId); + cmd.Parameters.AddWithValue("$iter", review.Iteration); + cmd.Parameters.AddWithValue("$dep", review.DeploymentId); + cmd.Parameters.AddWithValue("$ep", review.EndpointJson); + cmd.Parameters.AddWithValue("$deadline", review.Deadline.ToString("O")); + cmd.Parameters.AddWithValue("$requested", review.RequestedAt.ToString("O")); + cmd.Parameters.AddWithValue("$brief", review.Brief); + cmd.Parameters.AddWithValue("$qid", review.QuestionId); + cmd.Parameters.AddWithValue("$codefindings", review.CodeFindingsJson); + cmd.Parameters.AddWithValue("$codecompleted", review.CodeCompletedJson); + cmd.Parameters.AddWithValue("$humans", review.HumanAuditorsJson); + cmd.Parameters.AddWithValue("$findings", review.AutomatedFindingsJson); + cmd.Parameters.AddWithValue("$completed", review.AutomatedCompletedJson); + cmd.Parameters.AddWithValue("$incomplete", review.AutomatedIncompleteJson); + cmd.Parameters.AddWithValue("$agent", (object?)review.ActiveAuditAgentKind ?? DBNull.Value); + cmd.Parameters.AddWithValue("$shortcircuit", review.DeclaredShortCircuitBlocking ? 1 : 0); + cmd.Parameters.AddWithValue("$incompleteverdict", review.IncompleteVerdict ? 1 : 0); + cmd.Parameters.AddWithValue("$status", (int)review.Status); + cmd.Parameters.AddWithValue("$notes", (object?)review.Notes ?? DBNull.Value); + cmd.Parameters.AddWithValue( + "$decidedat", + review.DecidedAt is { } decided ? decided.ToString("O") : DBNull.Value); + cmd.Parameters.AddWithValue("$decidedby", (object?)review.DecidedBy ?? DBNull.Value); + cmd.Parameters.AddWithValue( + "$consumedat", + review.ConsumedAt is { } consumed ? consumed.ToString("O") : DBNull.Value); + } + + private static HumanDeploymentReview Read(SqliteDataReader r) => new() + { + WorkItemId = r.GetString(r.GetOrdinal("work_item_id")), + Iteration = r.GetInt32(r.GetOrdinal("iteration")), + DeploymentId = r.GetString(r.GetOrdinal("deployment_id")), + EndpointJson = r.GetString(r.GetOrdinal("endpoint_json")), + Deadline = Parse(r, "deadline"), + RequestedAt = Parse(r, "requested_at"), + Brief = r.GetString(r.GetOrdinal("brief")), + QuestionId = r.GetString(r.GetOrdinal("question_id")), + CodeFindingsJson = Nullable(r, "code_findings_json") ?? "[]", + CodeCompletedJson = Nullable(r, "code_completed_json") ?? "[]", + HumanAuditorsJson = r.GetString(r.GetOrdinal("human_auditors_json")), + AutomatedFindingsJson = r.GetString(r.GetOrdinal("automated_findings_json")), + AutomatedCompletedJson = r.GetString(r.GetOrdinal("automated_completed_json")), + AutomatedIncompleteJson = r.GetString(r.GetOrdinal("automated_incomplete_json")), + ActiveAuditAgentKind = Nullable(r, "active_audit_agent_kind"), + DeclaredShortCircuitBlocking = r.GetInt32(r.GetOrdinal("declared_short_circuit")) != 0, + IncompleteVerdict = r.GetInt32(r.GetOrdinal("incomplete_verdict")) != 0, + Status = (HumanDeploymentReviewStatus)r.GetInt32(r.GetOrdinal("status")), + Notes = Nullable(r, "notes"), + DecidedAt = NullableDate(r, "decided_at"), + DecidedBy = Nullable(r, "decided_by"), + ConsumedAt = NullableDate(r, "consumed_at"), + }; + + private static DateTimeOffset Parse(SqliteDataReader r, string column) + => DateTimeOffset.Parse(r.GetString(r.GetOrdinal(column)), CultureInfo.InvariantCulture); + + private static string? Nullable(SqliteDataReader r, string column) + { + var ord = r.GetOrdinal(column); + return r.IsDBNull(ord) ? null : r.GetString(ord); + } + + private static DateTimeOffset? NullableDate(SqliteDataReader r, string column) + { + var ord = r.GetOrdinal(column); + return r.IsDBNull(ord) + ? null + : DateTimeOffset.Parse(r.GetString(ord), CultureInfo.InvariantCulture); + } +} diff --git a/src/CodeyBox.Projects/ProjectAuditorComposer.cs b/src/CodeyBox.Projects/ProjectAuditorComposer.cs index adb7cd587..75f2c7059 100644 --- a/src/CodeyBox.Projects/ProjectAuditorComposer.cs +++ b/src/CodeyBox.Projects/ProjectAuditorComposer.cs @@ -30,6 +30,7 @@ public sealed class ProjectAuditorComposer private readonly PresetCatalogOptions _catalogOptions; private readonly Func? _testRunOptions; private readonly Func? _planAdherenceOptions; + private readonly Func? _humanReviewOptions; private readonly IReadOnlyDictionary _registeredAuditorsByName; private readonly IReadOnlyDictionary _pluginAuditors; private readonly TestFailureAttributionOptionsSnapshot? _testFailureAttributionOptions; @@ -59,6 +60,15 @@ public sealed class ProjectAuditorComposer /// time). Null (the default used by tests) keeps the reviewer out of the /// panel entirely — the feature is off unless the host wires the accessor. /// + /// + /// Live accessor for hot-reloadable . + /// When non-null and is + /// true, a deployment-target is + /// composed into the deployment stage (the pipeline parks for the operator + /// verdict instead of running it inline). Null (the default used by + /// tests) keeps the reviewer out of the panel entirely. A project drops + /// it via ExcludedAuditors by name. + /// public ProjectAuditorComposer( IPresetCatalog catalog, IEnumerable registeredAuditors, @@ -68,12 +78,14 @@ public ProjectAuditorComposer( Func? planAdherenceOptions = null, TestFailureAttributionOptionsSnapshot? testFailureAttributionOptions = null, RequiredAuditorPolicy? requiredAuditorPolicy = null, - TestSelectionShadowConfig? testSelectionShadow = null) + TestSelectionShadowConfig? testSelectionShadow = null, + Func? humanReviewOptions = null) { _catalog = catalog; _catalogOptions = catalogOptions?.Clone() ?? new PresetCatalogOptions(); _testRunOptions = testRunOptions; _planAdherenceOptions = planAdherenceOptions; + _humanReviewOptions = humanReviewOptions; _testFailureAttributionOptions = testFailureAttributionOptions; _testSelectionShadow = testSelectionShadow; _logger = logger; @@ -220,6 +232,18 @@ private IReadOnlyList Compose( auditors.Add(new PlanAdherenceAuditor(ctx.Agent, planAdherence)); } + // Human deployment reviewer: a deployment-target auditor the operator + // completes asynchronously through the park/resume path (the pipeline + // never runs it inline). Config-gated by CodeyBox:HumanReview and + // further gated per project by DeploymentAuditEnabled (the stage + // toggle) plus ExcludedAuditors by name — ordinary auditor + // configuration, no bespoke optionality mechanism. + if (_humanReviewOptions?.Invoke() is { Enabled: true } humanReview + && !auditors.Any(a => a.Name.Equals(humanReview.Name, StringComparison.OrdinalIgnoreCase))) + { + auditors.Add(new HumanDeploymentReviewAuditor(humanReview)); + } + // Always include every registered plan-audit chain gate. Each is // plan-target only (filtered out of the code phase by ComposeForTarget) // and applies to any project's plan; a project that does not want a diff --git a/tests/CodeyBox.Tests/DeploymentAuditPhaseTests.cs b/tests/CodeyBox.Tests/DeploymentAuditPhaseTests.cs index 3d0090239..deef837a1 100644 --- a/tests/CodeyBox.Tests/DeploymentAuditPhaseTests.cs +++ b/tests/CodeyBox.Tests/DeploymentAuditPhaseTests.cs @@ -121,6 +121,11 @@ public Task StartAsync(DeploymentRecipe recipe, DeploymentCon } } public IReadOnlyList GetActive() => []; + public bool TryGetActive(string deploymentId, out IDeploymentHandle? handle) + { + handle = null; + return false; + } } private sealed class FakeSubstrates : IDeploymentSubstrateProvider diff --git a/tests/CodeyBox.Tests/DeploymentAuditScopeTests.cs b/tests/CodeyBox.Tests/DeploymentAuditScopeTests.cs index 1256a9be0..10925da8a 100644 --- a/tests/CodeyBox.Tests/DeploymentAuditScopeTests.cs +++ b/tests/CodeyBox.Tests/DeploymentAuditScopeTests.cs @@ -40,6 +40,11 @@ public Task StartAsync(DeploymentRecipe recipe, DeploymentCon return start(recipe, context, ct); } public IReadOnlyList GetActive() => []; + public bool TryGetActive(string deploymentId, out IDeploymentHandle? handle) + { + handle = null; + return false; + } } private sealed class FakeSubstrates : IDeploymentSubstrateProvider diff --git a/tests/CodeyBox.Tests/DeploymentLeakReaperTests.cs b/tests/CodeyBox.Tests/DeploymentLeakReaperTests.cs index adc063ef6..4f3ed85f7 100644 --- a/tests/CodeyBox.Tests/DeploymentLeakReaperTests.cs +++ b/tests/CodeyBox.Tests/DeploymentLeakReaperTests.cs @@ -460,6 +460,11 @@ public IReadOnlyList GetActive() throw new InvalidOperationException("active set unavailable"); return _active; } + public bool TryGetActive(string deploymentId, out IDeploymentHandle? handle) + { + handle = null; + return false; + } } private sealed class ThrowingListProvider : IDeploymentCleanupProvider diff --git a/tests/CodeyBox.Tests/HumanDeploymentReviewEndpointTests.cs b/tests/CodeyBox.Tests/HumanDeploymentReviewEndpointTests.cs new file mode 100644 index 000000000..dd40455fd --- /dev/null +++ b/tests/CodeyBox.Tests/HumanDeploymentReviewEndpointTests.cs @@ -0,0 +1,294 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text; +using System.Text.Json; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; +using CodeyBox.Core; +using CodeyBox.Orchestrator; +using CodeyBox.Projects; + +namespace CodeyBox.Tests; + +/// +/// HTTP-level tests for the human deployment-review verdict surface: +/// GET /workitems/{id}/deployment-review +/// POST /workitems/{id}/deployment-review/approve +/// POST /workitems/{id}/deployment-review/reject +/// plus the generic POST /answer path, which verdicts identically when it +/// addresses the backing review question. +/// +[Collection("GlobalSerilog")] +public sealed class HumanDeploymentReviewEndpointTests : IDisposable +{ + private readonly HumanReviewEndpointFactory _factory = new(); + private readonly HttpClient _client; + + public HumanDeploymentReviewEndpointTests() + { + _client = _factory.CreateClient(); + } + + public void Dispose() + { + _client.Dispose(); + _factory.Dispose(); + } + + private async Task CreateItemAsync(WorkItemState state = WorkItemState.NeedsOperatorInput) + { + var item = new WorkItem + { + Id = WorkItemId.New(), + ProjectId = new ProjectId(HumanReviewEndpointFactory.ProjectId), + Title = "Review me", + Prompt = "serve the widget", + State = state, + StartedAt = DateTimeOffset.UtcNow, + }; + await _factory.WorkItemStore.CreateAsync(item); + return item; + } + + private async Task CreateReviewAsync( + WorkItem item, int iteration = 1, TimeSpan? lifetime = null) + { + var now = DateTimeOffset.UtcNow; + var review = new HumanDeploymentReview + { + WorkItemId = item.Id.ToString(), + Iteration = iteration, + DeploymentId = "dep-9", + EndpointJson = """{"Kind":0,"Url":"http://127.0.0.1:9999"}""", + Deadline = now + (lifetime ?? TimeSpan.FromHours(1)), + RequestedAt = now, + Brief = "verify the widget against the criteria", + QuestionId = HumanDeploymentReviewPolicy.QuestionIdFor(iteration), + HumanAuditorsJson = """["human:deployment-review"]""", + CodeFindingsJson = "[]", + CodeCompletedJson = """["code:scripted"]""", + AutomatedFindingsJson = "[]", + AutomatedCompletedJson = """["deploy:smoke"]""", + AutomatedIncompleteJson = "[]", + }; + await _factory.ReviewStore.GetOrCreatePendingAsync(review); + await _factory.QuestionStore.CreateIfNotExistsAsync(new WorkItemQuestion + { + Id = Guid.NewGuid().ToString(), + WorkItemId = item.Id.ToString(), + QuestionId = review.QuestionId, + QuestionText = review.Brief, + }); + return review; + } + + private static StringContent JsonBody(string json) + => new(json, Encoding.UTF8, "application/json"); + + [Fact] + public async Task GetReview_Pending_ReturnsEndpointAndBrief() + { + var item = await CreateItemAsync(); + var review = await CreateReviewAsync(item); + + var resp = await _client.GetAsync($"/workitems/{item.Id}/deployment-review"); + Assert.Equal(HttpStatusCode.OK, resp.StatusCode); + var body = await resp.Content.ReadFromJsonAsync(); + Assert.Equal(review.DeploymentId, body.GetProperty("deploymentId").GetString()); + Assert.Equal(1, body.GetProperty("iteration").GetInt32()); + Assert.Equal("Pending", body.GetProperty("status").GetString()); + Assert.Contains("verify the widget", body.GetProperty("brief").GetString(), StringComparison.Ordinal); + } + + [Fact] + public async Task GetReview_None_Returns404() + { + var item = await CreateItemAsync(); + + var resp = await _client.GetAsync($"/workitems/{item.Id}/deployment-review"); + Assert.Equal(HttpStatusCode.NotFound, resp.StatusCode); + } + + [Fact] + public async Task Approve_RecordsVerdict_AnswersQuestion_Resumes() + { + var item = await CreateItemAsync(); + var review = await CreateReviewAsync(item); + + var resp = await _client.PostAsync( + $"/workitems/{item.Id}/deployment-review/approve", JsonBody("{}")); + Assert.Equal(HttpStatusCode.OK, resp.StatusCode); + var body = await resp.Content.ReadFromJsonAsync(); + Assert.Equal("approved", body.GetProperty("status").GetString()); + + var decided = await _factory.ReviewStore.TryGetAsync(item.Id.ToString(), 1); + Assert.Equal(HumanDeploymentReviewStatus.Approved, decided!.Status); + var q = await _factory.QuestionStore.GetAsync(item.Id.ToString(), review.QuestionId); + Assert.Equal("answered", q!.State); + Assert.Equal("approve", q.AnswerText); + var resumed = await _factory.WorkItemStore.GetAsync(item.Id); + Assert.Equal(WorkItemState.WorkComplete, resumed!.State); + } + + [Fact] + public async Task Reject_RequiresNotes_AndRecordsThem() + { + var item = await CreateItemAsync(); + await CreateReviewAsync(item); + + var missing = await _client.PostAsync( + $"/workitems/{item.Id}/deployment-review/reject", JsonBody("{}")); + Assert.Equal(HttpStatusCode.BadRequest, missing.StatusCode); + + var tooLong = await _client.PostAsync( + $"/workitems/{item.Id}/deployment-review/reject", + JsonBody(JsonSerializer.Serialize(new { notes = new string('x', 4001) }))); + Assert.Equal(HttpStatusCode.BadRequest, tooLong.StatusCode); + + var ok = await _client.PostAsync( + $"/workitems/{item.Id}/deployment-review/reject", + JsonBody(JsonSerializer.Serialize(new { notes = "header is wrong" }))); + Assert.Equal(HttpStatusCode.OK, ok.StatusCode); + var decided = await _factory.ReviewStore.TryGetAsync(item.Id.ToString(), 1); + Assert.Equal(HumanDeploymentReviewStatus.Rejected, decided!.Status); + Assert.Equal("header is wrong", decided.Notes); + } + + [Fact] + public async Task Approve_WhenExpired_Returns410AndFailsClosed() + { + var item = await CreateItemAsync(); + var review = await CreateReviewAsync(item, lifetime: TimeSpan.FromMinutes(-5)); + + var resp = await _client.PostAsync( + $"/workitems/{item.Id}/deployment-review/approve", JsonBody("{}")); + Assert.Equal(HttpStatusCode.Gone, resp.StatusCode); + + var expired = await _factory.ReviewStore.TryGetAsync(item.Id.ToString(), 1); + Assert.Equal(HumanDeploymentReviewStatus.Expired, expired!.Status); + var q = await _factory.QuestionStore.GetAsync(item.Id.ToString(), review.QuestionId); + Assert.Equal("dismissed", q!.State); + var resumed = await _factory.WorkItemStore.GetAsync(item.Id); + Assert.Equal(WorkItemState.WorkComplete, resumed!.State); + } + + [Fact] + public async Task GenericAnswer_Approve_VerdictsLikeDedicatedEndpoint() + { + var item = await CreateItemAsync(); + var review = await CreateReviewAsync(item); + + var resp = await _client.PostAsync( + $"/workitems/{item.Id}/answer", + JsonBody(JsonSerializer.Serialize(new + { + questionId = review.QuestionId, + answer = "approve", + }))); + Assert.Equal(HttpStatusCode.OK, resp.StatusCode); + + var decided = await _factory.ReviewStore.TryGetAsync(item.Id.ToString(), 1); + Assert.Equal(HumanDeploymentReviewStatus.Approved, decided!.Status); + } + + [Fact] + public async Task GenericAnswer_OtherText_RejectsWithNotes() + { + var item = await CreateItemAsync(); + var review = await CreateReviewAsync(item); + + var resp = await _client.PostAsync( + $"/workitems/{item.Id}/answer", + JsonBody(JsonSerializer.Serialize(new + { + questionId = review.QuestionId, + answer = "the footer overlaps on mobile", + }))); + Assert.Equal(HttpStatusCode.OK, resp.StatusCode); + + var decided = await _factory.ReviewStore.TryGetAsync(item.Id.ToString(), 1); + Assert.Equal(HumanDeploymentReviewStatus.Rejected, decided!.Status); + Assert.Equal("the footer overlaps on mobile", decided.Notes); + var resumed = await _factory.WorkItemStore.GetAsync(item.Id); + Assert.Equal(WorkItemState.WorkComplete, resumed!.State); + } +} + +internal sealed class HumanReviewEndpointFactory : WebApplicationFactory +{ + public const string ProjectId = "test-project"; + + private readonly string _dbPath = Path.Combine( + Path.GetTempPath(), $"codeybox-human-ep-{Guid.NewGuid():N}.db"); + + public SqliteWorkItemStore WorkItemStore { get; } + public SqliteWorkItemQuestionStore QuestionStore { get; } + public SqliteHumanDeploymentReviewStore ReviewStore { get; } + + public HumanReviewEndpointFactory() + { + WorkItemStore = new SqliteWorkItemStore(_dbPath); + QuestionStore = new SqliteWorkItemQuestionStore(_dbPath); + ReviewStore = new SqliteHumanDeploymentReviewStore(_dbPath); + } + + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder.UseEnvironment("Development"); + builder.ConfigureAppConfiguration((_, cfg) => + { + var tmp = Path.GetTempPath(); + cfg.AddInMemoryCollection(new Dictionary + { + ["CodeyBox:DangerouslyDisableAuth"] = "true", + ["CodeyBox:StateDatabasePath"] = _dbPath, + ["CodeyBox:GitRootDirectory"] = Path.Combine(tmp, $"test-git-{Guid.NewGuid():N}"), + ["CodeyBox:AuditLog:Path"] = Path.Combine(tmp, $"test-log-{Guid.NewGuid():N}-.json"), + ["CodeyBox:AuditLog:AuditPath"] = Path.Combine(tmp, $"test-audit-{Guid.NewGuid():N}-.json"), + }); + }); + builder.ConfigureTestServices(services => + { + services.RemoveAll(); + + services.RemoveAll(); + services.AddSingleton(WorkItemStore); + + services.RemoveAll(); + services.AddSingleton(QuestionStore); + + services.RemoveAll(); + services.AddSingleton(ReviewStore); + + services.RemoveAll(); + services.AddSingleton(new InMemoryProjectRepository( + new Project + { + Id = new CodeyBox.Core.ProjectId(ProjectId), + DisplayName = "Test Project", + RepositoryUrl = "https://github.com/test/repo", + DefaultAgent = AgentKind.Claude, + DefaultBaseBranch = "main", + AllowAgentQuestions = true, + })); + }); + } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + WorkItemStore.Dispose(); + QuestionStore.Dispose(); + ReviewStore.Dispose(); + try { File.Delete(_dbPath); } catch { } + } + + base.Dispose(disposing); + } +} diff --git a/tests/CodeyBox.Tests/HumanDeploymentReviewPolicyTests.cs b/tests/CodeyBox.Tests/HumanDeploymentReviewPolicyTests.cs new file mode 100644 index 000000000..9a65d5c3d --- /dev/null +++ b/tests/CodeyBox.Tests/HumanDeploymentReviewPolicyTests.cs @@ -0,0 +1,353 @@ +using CodeyBox.Audit; +using CodeyBox.Audit.Presets; +using CodeyBox.Core; +using CodeyBox.Orchestrator; +using CodeyBox.Projects; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CodeyBox.Tests; + +/// +/// Unit tests for the human deployment-review seam: the pure policy +/// (brief, answer mapping, verdict findings, endpoint display, finding +/// round-trips), the auditor's declaration (kind/targets/cost/ordering), +/// composition (opt-in knob, per-project exclusion), and the durable store's +/// compare-and-set transitions. +/// +public sealed class HumanDeploymentReviewPolicyTests : IDisposable +{ + private readonly string _dbPath = Path.Combine( + Path.GetTempPath(), $"codeybox-human-policy-{Guid.NewGuid():N}.db"); + + public void Dispose() + { + try { File.Delete(_dbPath); } catch { } + } + + // ── Pure policy ───────────────────────────────────────────────────────── + + [Fact] + public void QuestionIdFor_IsStableAndPerIteration() + { + Assert.Equal("human-deployment-review-1", HumanDeploymentReviewPolicy.QuestionIdFor(1)); + Assert.Equal("human-deployment-review-3", HumanDeploymentReviewPolicy.QuestionIdFor(3)); + Assert.NotEqual( + HumanDeploymentReviewPolicy.QuestionIdFor(1), + HumanDeploymentReviewPolicy.QuestionIdFor(2)); + } + + [Theory] + [InlineData("approve", true)] + [InlineData("APPROVE", true)] + [InlineData(" approve ", true)] + [InlineData("approved", false)] + [InlineData("approve this", false)] + [InlineData("no, the header is wrong", false)] + [InlineData("", false)] + [InlineData(null, false)] + public void IsApprovalAnswer_RequiresExactApprove(string? answer, bool expected) + { + // Exact equality only — never substring: "approved" must reject. + Assert.Equal(expected, HumanDeploymentReviewPolicy.IsApprovalAnswer(answer)); + } + + [Fact] + public void IsReviewQuestion_MatchesMarkerAndSuffixedIds() + { + Assert.True(HumanDeploymentReviewPolicy.IsReviewQuestion("human-deployment-review-1")); + Assert.True(HumanDeploymentReviewPolicy.IsReviewQuestion("human-deployment-review")); + Assert.False(HumanDeploymentReviewPolicy.IsReviewQuestion("q-001")); + Assert.False(HumanDeploymentReviewPolicy.IsReviewQuestion("human-deployment-review-1-evil")); + Assert.False(HumanDeploymentReviewPolicy.IsReviewQuestion(null)); + } + + [Fact] + public void BuildBrief_ContainsEndpointExpiryAndCriteria_AndBoundsInputs() + { + var criteria = Enumerable.Range(0, 30) + .Select(i => ($"criterion-{i}", new string('x', 600))) + .ToList(); + var brief = HumanDeploymentReviewPolicy.BuildBrief( + "Serve widgets", + new string('p', 5000), + "http://127.0.0.1:8080", + new DateTimeOffset(2026, 9, 12, 0, 0, 0, TimeSpan.Zero), + criteria, + iteration: 2, + deploymentId: "dep-7"); + + Assert.Contains("http://127.0.0.1:8080", brief, StringComparison.Ordinal); + Assert.Contains("2026-09-12", brief, StringComparison.Ordinal); + Assert.Contains("approve", brief, StringComparison.OrdinalIgnoreCase); + Assert.Contains("[truncated]", brief, StringComparison.Ordinal); + Assert.Contains("more, truncated", brief, StringComparison.Ordinal); + Assert.True(brief.Length < 20000, $"brief unexpectedly large: {brief.Length}"); + } + + [Fact] + public void BuildVerdictFindings_ApprovePasses_RejectAndExpiryBlock() + { + Assert.Empty(HumanDeploymentReviewPolicy.BuildVerdictFindings( + "human:deployment-review", HumanDeploymentReviewStatus.Approved, null)); + + var rejected = HumanDeploymentReviewPolicy.BuildVerdictFindings( + "human:deployment-review", HumanDeploymentReviewStatus.Rejected, "header is wrong"); + var reject = Assert.Single(rejected); + Assert.Equal(AuditSeverity.Error, reject.Severity); + Assert.Equal("human:deployment-review", reject.AuditorName); + Assert.Contains("header is wrong", reject.Description, StringComparison.Ordinal); + + var expired = HumanDeploymentReviewPolicy.BuildVerdictFindings( + "human:deployment-review", + HumanDeploymentReviewStatus.Expired, + null, + new DateTimeOffset(2026, 9, 12, 0, 0, 0, TimeSpan.Zero)); + var expiry = Assert.Single(expired); + Assert.Equal(AuditSeverity.Error, expiry.Severity); + Assert.Contains("expired unreviewed", expiry.Title, StringComparison.OrdinalIgnoreCase); + + Assert.Throws(() => + HumanDeploymentReviewPolicy.BuildVerdictFindings( + "human:deployment-review", HumanDeploymentReviewStatus.Pending, null)); + } + + [Fact] + public void DescribeEndpoint_PrefersUrlThenHostPortThenPath() + { + Assert.Equal( + "http://h:1", + HumanDeploymentReviewPolicy.DescribeEndpoint(new DeploymentEndpoint + { + Kind = DeploymentEndpointKind.Http, + Url = "http://h:1", + })); + Assert.Equal( + "example.com:8080", + HumanDeploymentReviewPolicy.DescribeEndpoint(new DeploymentEndpoint + { + Kind = DeploymentEndpointKind.Http, + Host = "example.com", + Port = 8080, + })); + Assert.Equal( + "(endpoint unavailable)", + HumanDeploymentReviewPolicy.DescribeEndpoint(null)); + } + + [Fact] + public void FindingsRoundTrip_PreservesShape_CorruptFailsClosed() + { + var findings = new List + { + new("deploy:smoke", AuditSeverity.Error, "health failed", "body", "http://h/health"), + new("deploy:tls", AuditSeverity.Warning, "weak cipher", "body2"), + }; + var json = HumanDeploymentReviewPolicy.SerializeFindings(findings); + var back = HumanDeploymentReviewPolicy.DeserializeFindings(json); + Assert.Equal(2, back.Count); + Assert.Equal("health failed", back[0].Title); + Assert.Equal(AuditSeverity.Warning, back[1].Severity); + Assert.Equal("http://h/health", back[0].Location); + + Assert.Throws(() => + HumanDeploymentReviewPolicy.DeserializeFindings("not-json{{{")); + var strings = HumanDeploymentReviewPolicy.DeserializeStrings( + HumanDeploymentReviewPolicy.SerializeStrings(["a", "b"])); + Assert.Equal(["a", "b"], strings); + Assert.Throws(() => + HumanDeploymentReviewPolicy.DeserializeStrings("not-json{{{")); + } + + // ── Auditor declaration ───────────────────────────────────────────────── + + [Fact] + public async Task HumanAuditor_NeverRunsInline() + { + var auditor = new HumanDeploymentReviewAuditor(new HumanDeploymentReviewOptions()); + Assert.Equal("human:deployment-review", auditor.Name); + Assert.Equal("human", auditor.Kind); + Assert.Equal(AuditCapabilities.None, auditor.Required); + Assert.Equal(AuditTargets.DeploymentOnly, auditor.Targets); + await Assert.ThrowsAsync(() => + auditor.RunAsync(null!, "/tmp", null!, CancellationToken.None)); + } + + [Fact] + public void HumanAuditor_CostClassAndOrdering_SortAfterAutomated() + { + var human = new HumanDeploymentReviewAuditor(new HumanDeploymentReviewOptions()); + Assert.Equal(AuditCostClass.Reviewer, AuditPhaseLadder.CostClassOf(human)); + Assert.True(AuditPhaseLadder.RunsInDeploymentStage(human)); + Assert.False(AuditPhaseLadder.RunsInCodeStage(human)); + Assert.True(AuditorOrdering.IsHuman(human)); + + var tool = new FakeToolAuditor(); + Assert.True(AuditorOrdering.TierOf(human) > AuditorOrdering.TierOf(tool)); + Assert.Equal( + new[] { tool.Name, human.Name }, + AuditPhaseLadder.OrderDeploymentStage([human, tool]).Select(a => a.Name)); + } + + private sealed class FakeToolAuditor : IAuditor + { + public string Name => "tool:fake"; + public string Kind => "tool"; + public AuditCapabilities Required => AuditCapabilities.None; + public Task RunAsync(ISandbox sandbox, string workingDirectory, AuditContext context, CancellationToken ct = default) + => Task.FromResult(new AuditResult(true, [])); + } + + // ── Composition ───────────────────────────────────────────────────────── + + private static ProjectAuditorComposer Composer(Func? humanReview) => + new(new PresetCatalog(), [], NullLogger.Instance, + catalogOptions: null, testRunOptions: null, planAdherenceOptions: null, + humanReviewOptions: humanReview); + + private sealed class FakeAgent : IAgentRunner + { + public AgentKind Kind => AgentKind.Codex; + + public Task RunAsync( + ISandbox sandbox, string workingDirectory, string prompt, AgentCredential? credential, + string? modelId = null, string? reasoningMode = null, CancellationToken ct = default, + Action? stdoutChunkCallback = null, bool captureStructuredStream = false) + => Task.FromResult(new AgentResult(true, "ok", "", null)); + } + + private static Project ProjectWith(params string[] excluded) => new() + { + Id = new ProjectId("alpha"), + DisplayName = "Alpha", + RepositoryUrl = "https://example.com/repo.git", + Audit = new ProjectAudit + { + AuditTypes = ["security"], + ExcludedAuditors = excluded, + }, + }; + + [Fact] + public void DeploymentTarget_Enabled_IncludesHumanReviewer() + { + var composer = Composer(() => new HumanDeploymentReviewOptions { Enabled = true }); + + var deployment = composer + .ComposeForTarget(ProjectWith(), new FakeAgent(), AuditTarget.Deployment) + .Select(a => a.Name) + .ToArray(); + Assert.Contains("human:deployment-review", deployment); + + var code = composer + .ComposeForTarget(ProjectWith(), new FakeAgent(), AuditTarget.Code) + .Select(a => a.Name) + .ToArray(); + Assert.DoesNotContain("human:deployment-review", code); + } + + [Fact] + public void NoAccessorOrDisabled_HumanReviewerAbsent() + { + foreach (var composer in new[] + { + Composer(humanReview: null), + Composer(() => new HumanDeploymentReviewOptions { Enabled = false }), + }) + { + var names = composer + .ComposeForTarget(ProjectWith(), new FakeAgent(), AuditTarget.Deployment) + .Select(a => a.Name) + .ToArray(); + Assert.DoesNotContain("human:deployment-review", names); + } + } + + [Fact] + public void ExcludedByName_HumanReviewerRemoved() + { + var composer = Composer(() => new HumanDeploymentReviewOptions { Enabled = true }); + + var names = composer + .ComposeForTarget(ProjectWith("human:deployment-review"), new FakeAgent(), AuditTarget.Deployment) + .Select(a => a.Name) + .ToArray(); + Assert.DoesNotContain("human:deployment-review", names); + } + + // ── Durable store ─────────────────────────────────────────────────────── + + private static HumanDeploymentReview Row(string wid = "item-1", int iteration = 1) => new() + { + WorkItemId = wid, + Iteration = iteration, + DeploymentId = "dep-1", + EndpointJson = """{"Kind":0}""", + Deadline = new DateTimeOffset(2026, 9, 12, 0, 0, 0, TimeSpan.Zero), + RequestedAt = new DateTimeOffset(2026, 9, 11, 0, 0, 0, TimeSpan.Zero), + Brief = "verify the widget", + QuestionId = "human-deployment-review-1", + HumanAuditorsJson = """["human:deployment-review"]""", + CodeFindingsJson = "[]", + CodeCompletedJson = """["code:scripted"]""", + AutomatedFindingsJson = "[]", + AutomatedCompletedJson = "[]", + AutomatedIncompleteJson = "[]", + }; + + [Fact] + public async Task Store_GetOrCreate_Verdict_Consume_ExpiryCas() + { + using var store = new SqliteHumanDeploymentReviewStore(_dbPath); + + var created = await store.GetOrCreatePendingAsync(Row()); + Assert.Equal(HumanDeploymentReviewStatus.Pending, created.Status); + var same = await store.GetOrCreatePendingAsync(Row()); + Assert.Equal(created.DeploymentId, same.DeploymentId); + + Assert.Null(await store.TryGetAsync("item-1", 2)); + var fetched = await store.TryGetAsync("item-1", 1); + Assert.Equal("dep-1", fetched!.DeploymentId); + var active = await store.GetActiveForWorkItemAsync("item-1"); + Assert.Equal(1, active!.Iteration); + + // Verdict applies once, from pending only. + Assert.True(await store.RecordVerdictAsync( + "item-1", 1, approved: false, "broken", "op", + new DateTimeOffset(2026, 9, 11, 1, 0, 0, TimeSpan.Zero))); + Assert.False(await store.RecordVerdictAsync( + "item-1", 1, approved: true, null, null, + new DateTimeOffset(2026, 9, 11, 2, 0, 0, TimeSpan.Zero))); + Assert.False(await store.MarkExpiredAsync( + "item-1", 1, new DateTimeOffset(2026, 9, 11, 3, 0, 0, TimeSpan.Zero))); + var decided = await store.TryGetAsync("item-1", 1); + Assert.Equal(HumanDeploymentReviewStatus.Rejected, decided!.Status); + Assert.Equal("broken", decided.Notes); + + // Consume is idempotent; the row leaves the active set. + await store.MarkConsumedAsync("item-1", 1, new DateTimeOffset(2026, 9, 11, 4, 0, 0, TimeSpan.Zero)); + await store.MarkConsumedAsync("item-1", 1, new DateTimeOffset(2026, 9, 11, 5, 0, 0, TimeSpan.Zero)); + Assert.Null(await store.GetActiveForWorkItemAsync("item-1")); + + // A consumed row can be replaced with a fresh pending review. + var fresh = await store.GetOrCreatePendingAsync(Row() with { DeploymentId = "dep-2" }); + Assert.Equal("dep-2", fresh.DeploymentId); + Assert.Equal(HumanDeploymentReviewStatus.Pending, fresh.Status); + } + + [Fact] + public async Task Store_ListExpiredPending_ReturnsOnlyUndecidedPastDeadline() + { + using var store = new SqliteHumanDeploymentReviewStore(_dbPath); + var now = new DateTimeOffset(2026, 9, 12, 0, 0, 0, TimeSpan.Zero); + await store.GetOrCreatePendingAsync(Row("expired-item") + with { Deadline = now.AddMinutes(-1) }); + await store.GetOrCreatePendingAsync(Row("fresh-item") + with { Deadline = now.AddMinutes(1) }); + await store.GetOrCreatePendingAsync(Row("decided-item") + with { Deadline = now.AddMinutes(-1) }); + await store.RecordVerdictAsync("decided-item", 1, true, null, null, now); + + var expired = await store.ListExpiredPendingAsync(now); + Assert.Equal(["expired-item"], expired.Select(r => r.WorkItemId).ToArray()); + } +} diff --git a/tests/CodeyBox.Tests/HumanDeploymentReviewTests.cs b/tests/CodeyBox.Tests/HumanDeploymentReviewTests.cs new file mode 100644 index 000000000..881384f73 --- /dev/null +++ b/tests/CodeyBox.Tests/HumanDeploymentReviewTests.cs @@ -0,0 +1,622 @@ +using Microsoft.Extensions.Logging.Abstractions; +using ControllableTimeProvider = Microsoft.Extensions.Time.Testing.FakeTimeProvider; +using CodeyBox.Audit; +using CodeyBox.Core; +using CodeyBox.Orchestrator; + +namespace CodeyBox.Tests; + +/// +/// Deployment verification chain (3/3): the operator as a reviewer through +/// the standard auditor seam. A human-kind deployment auditor parks the +/// audit iteration (no worker slot held, watchdog quiet, only the +/// deployment kept alive), notifies the operator with endpoint + expiry + +/// acceptance criteria, and on verdict resumes with approve passing, +/// reject-with-notes blocking into rework, or expiry failing closed — +/// tearing the deployment down immediately in every case. +/// +[Collection("GlobalSerilog")] +public sealed class HumanDeploymentReviewTests : IDisposable +{ + private readonly string _workspace; + private readonly TestSupport.AmbientGitConfigScope _gitConfigScope; + + public HumanDeploymentReviewTests() + { + _workspace = Directory.CreateTempSubdirectory("codeybox-human-review-").FullName; + _gitConfigScope = TestSupport.AmbientGitConfigScope.Clear(); + } + + public void Dispose() + { + _gitConfigScope.Dispose(); + try { Directory.Delete(_workspace, recursive: true); } catch { } + } + + private sealed record Outcome(bool Passed, IReadOnlyList Findings); + + private sealed class CodeAuditor(Queue plan) : IAuditor + { + public string Name => "code:scripted"; + public string Kind => "tool"; + public AuditCapabilities Required => AuditCapabilities.None; + public List SeenIterations { get; } = []; + public Task RunAsync(ISandbox sandbox, string workingDirectory, AuditContext context, CancellationToken ct = default) + { + SeenIterations.Add(context.Iteration); + var outcome = plan.Dequeue(); + return Task.FromResult(new AuditResult(outcome.Passed, outcome.Findings)); + } + } + + private sealed class DeploymentProbeAuditor(Queue plan) : IAuditor + { + public string Name => "deploy:smoke"; + public string Kind => "tool"; + public AuditCapabilities Required => AuditCapabilities.None; + public IReadOnlySet Targets => AuditTargets.DeploymentOnly; + public List SeenIterations { get; } = []; + public Task RunAsync(ISandbox sandbox, string workingDirectory, AuditContext context, CancellationToken ct = default) + { + SeenIterations.Add(context.Iteration); + var outcome = plan.Dequeue(); + return Task.FromResult(new AuditResult(outcome.Passed, outcome.Findings)); + } + } + + private sealed class LiveFakeManager : IDeploymentManager + { + private readonly Dictionary _live = new(); + private readonly object _gate = new(); + private int _starts; + private int _disposals; + public int StartCount { get { lock (_gate) return _starts; } } + public int DisposeCount { get { lock (_gate) return _disposals; } } + public int LiveCount { get { lock (_gate) return _live.Count; } } + public DeploymentEndpoint Endpoint { get; } = new() + { + Kind = DeploymentEndpointKind.Http, + Url = "http://127.0.0.1:18080", + }; + + public Task StartAsync(DeploymentRecipe recipe, DeploymentContext context, CancellationToken ct = default) + { + lock (_gate) + { + _starts++; + var id = $"dep-{_starts}"; + var captured = id; + var handle = new FakeHandle(Endpoint, id, () => + { + lock (_gate) + { + _disposals++; + _live.Remove(captured); + } + }); + _live[id] = handle; + return Task.FromResult(handle); + } + } + + public bool TryGetActive(string deploymentId, out IDeploymentHandle? handle) + { + lock (_gate) + { + if (_live.TryGetValue(deploymentId, out var h)) + { + handle = h; + return true; + } + + handle = null; + return false; + } + } + + public IReadOnlyList GetActive() + { + lock (_gate) + return _live.Values + .Select(h => new ActiveDeploymentInfo(h.Id, h.Kind, null, h.SubstrateId, DateTimeOffset.UtcNow, h.Endpoint)) + .ToList(); + } + + private sealed class FakeHandle(DeploymentEndpoint endpoint, string id, Action onDispose) : IDeploymentHandle + { + private bool _disposed; + public string Id { get; } = id; + public string Kind => DeploymentKinds.WebApp; + public DeploymentEndpoint Endpoint { get; } = endpoint; + public bool IsAlive => !_disposed; + public string? SubstrateId => "substrate-" + Id; + public Task HealthCheckAsync(CancellationToken ct = default) => Task.CompletedTask; + public Task ExecAsync(DeploymentCommand command, CancellationToken ct = default) + => Task.FromResult(new DeploymentCommandResult(0, string.Empty, string.Empty)); + public ValueTask DisposeAsync() + { + if (_disposed) return ValueTask.CompletedTask; + _disposed = true; + onDispose(); + return ValueTask.CompletedTask; + } + } + } + + private sealed class FakeSubstrates : IDeploymentSubstrateProvider + { + public string Name => "fake"; + public Task CreateAsync(DeploymentSubstrateSpec spec, CancellationToken ct = default) + => throw new NotSupportedException("FakeManager never reaches the substrate provider."); + } + + private sealed class RecordingWebhookDispatcher : IWebhookDispatcher + { + private readonly List _events = new(); + private readonly object _lock = new(); + public IReadOnlyList Events + { + get { lock (_lock) return _events.ToArray(); } + } + + public Task PublishAsync(WebhookEvent evt, CancellationToken ct) + { + lock (_lock) _events.Add(evt); + return Task.CompletedTask; + } + } + + private static DeploymentRecipe Recipe(TimeSpan? maxLifetime = null) => new() + { + Kind = DeploymentKinds.WebApp, + ImageReference = "img", + MaxLifetime = maxLifetime ?? TimeSpan.FromMinutes(30), + }; + + private static WorkItem NewItem() => new() + { + Id = WorkItemId.New(), + ProjectId = new ProjectId("test-project"), + Title = "human review test", + Prompt = "serve the widget over http", + BaseBranch = "main", + WorkBranch = "feature/x", + PushUpstream = false, + }; + + private static ProjectAudit AuditWithDeployment(bool enabled, int maxIterations = 3) => new() + { + MaxIterations = maxIterations, + AuditTypes = ["scripted"], + DeploymentAuditEnabled = enabled, + }; + + private static HumanDeploymentReviewAuditor HumanAuditor() => + new(new HumanDeploymentReviewOptions()); + + private static (string DbPath, SqliteWorkItemQuestionStore Questions, SqliteHumanDeploymentReviewStore Reviews) Stores() + { + var db = Path.Combine( + Path.GetTempPath(), $"codeybox-human-{Guid.NewGuid():N}.db"); + return (db, new SqliteWorkItemQuestionStore(db), new SqliteHumanDeploymentReviewStore(db)); + } + + private async Task ParkAsync( + TestPipeline tp, + SqliteHumanDeploymentReviewStore reviews, + WorkItem item, + CancellationToken ct = default) + { + await tp.Pipeline.RunAsync(item, ct); + var parked = await tp.Store.GetAsync(item.Id, ct); + Assert.Equal(WorkItemState.NeedsOperatorInput, parked!.State); + var review = await reviews.GetActiveForWorkItemAsync(item.Id.ToString(), ct); + Assert.NotNull(review); + return review!; + } + + /// + /// Mirrors the verdict endpoints: record the verdict, answer the backing + /// question, and resume the item. + /// + private static async Task ResumeWithVerdictAsync( + TestPipeline tp, + SqliteHumanDeploymentReviewStore reviews, + SqliteWorkItemQuestionStore questions, + WorkItemId id, + bool approved, + string? notes, + CancellationToken ct = default) + { + var review = await reviews.GetActiveForWorkItemAsync(id.ToString(), ct); + Assert.NotNull(review); + var now = DateTimeOffset.UtcNow; + Assert.True(await reviews.RecordVerdictAsync( + review!.WorkItemId, review.Iteration, approved, notes, "operator", now, ct)); + await questions.AnswerAsync(id.ToString(), review.QuestionId, approved ? "approve" : notes!, "operator", ct); + var parked = await tp.Store.GetAsync(id, ct); + Assert.True(await tp.Store.TryUpdateIfStateAsync( + parked!.With(WorkItemState.WorkComplete), WorkItemState.NeedsOperatorInput, ct)); + return await tp.Store.GetAsync(id, ct) ?? parked; + } + + [Fact] + public async Task ParksWithoutHoldingSlot_NotifiesOperator_WatchdogsQuiet() + { + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var code = new CodeAuditor(new Queue([new(true, [])])); + var probe = new DeploymentProbeAuditor(new Queue([new(true, [])])); + var manager = new LiveFakeManager(); + var webhooks = new RecordingWebhookDispatcher(); + var (db, questions, reviews) = Stores(); + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + auditors: [code, probe, HumanAuditor()], + projectAudit: AuditWithDeployment(enabled: true, maxIterations: 1), + deploymentRecipe: Recipe(), + deploymentManager: manager, + deploymentSubstrates: new FakeSubstrates(), + webhookDispatcher: webhooks, + stateDbPathOverride: db, + questionStore: questions, + humanReviewStore: reviews); + tp.Agent.WorkPlan.Enqueue(new FileWrite("a.txt", "v1")); + + var item = NewItem(); + await tp.Store.CreateAsync(item); + + // The pipeline returns (worker slot released) while the deployment + // stays alive: only the deployment is kept, nothing else. + await tp.Pipeline.RunAsync(item, CancellationToken.None); + + var parked = await tp.Store.GetAsync(item.Id); + Assert.Equal(WorkItemState.NeedsOperatorInput, parked!.State); + Assert.Contains("human deployment review", parked.LastError, StringComparison.OrdinalIgnoreCase); + Assert.Equal(1, manager.StartCount); + Assert.Equal(1, manager.LiveCount); + Assert.Equal(0, manager.DisposeCount); + Assert.Equal([1], probe.SeenIterations); + + // The review row bounds the deployment: deadline = start + recipe max. + var review = await reviews.GetActiveForWorkItemAsync(item.Id.ToString()); + Assert.NotNull(review); + Assert.Equal(HumanDeploymentReviewStatus.Pending, review!.Status); + Assert.Equal("human-deployment-review-1", review.QuestionId); + Assert.True(review.Deadline - review.RequestedAt >= TimeSpan.FromMinutes(29)); + + // The backing question carries endpoint + expiry + criteria. + var qs = await questions.ListByWorkItemAsync(item.Id.ToString()); + var q = Assert.Single(qs); + Assert.Equal(review.QuestionId, q.QuestionId); + Assert.Equal("open", q.State); + Assert.Contains("http://127.0.0.1:18080", q.QuestionText, StringComparison.Ordinal); + Assert.Contains("expires", q.QuestionText, StringComparison.OrdinalIgnoreCase); + + // The operator notification carries endpoint + expiry structurally. + var parkedEvent = webhooks.Events.SingleOrDefault(e => e.Event == "work_item.needs_operator_input"); + Assert.NotNull(parkedEvent); + var details = Assert.IsType(parkedEvent!.Details); + Assert.Equal("http://127.0.0.1:18080", details.Endpoint); + Assert.Equal(review.Deadline, details.ExpiresAt); + Assert.Equal(review.QuestionId, details.QuestionId); + Assert.Contains(webhooks.Events, e => e.Event == "work_item.question_asked"); + + // A parked-on-human item is not stalled: exempt from both watchdogs. + Assert.False(WorkerProgressWatchdog.IsWatchedState(WorkItemState.NeedsOperatorInput)); + Assert.False(WorkItemRecoveryPolicy.IsItemStaleWatchedState(WorkItemState.NeedsOperatorInput)); + Assert.DoesNotContain( + WorkItemState.NeedsOperatorInput, + (IEnumerable)WorkItemRecoveryPolicy.WorkerOccupiedStates); + } + + [Fact] + public async Task Approve_PassesAndTearsDownImmediately() + { + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var code = new CodeAuditor(new Queue([new(true, [])])); + var probe = new DeploymentProbeAuditor(new Queue([new(true, [])])); + var manager = new LiveFakeManager(); + var (db, questions, reviews) = Stores(); + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + auditors: [code, probe, HumanAuditor()], + projectAudit: AuditWithDeployment(enabled: true, maxIterations: 1), + deploymentRecipe: Recipe(), + deploymentManager: manager, + deploymentSubstrates: new FakeSubstrates(), + stateDbPathOverride: db, + questionStore: questions, + humanReviewStore: reviews); + tp.Agent.WorkPlan.Enqueue(new FileWrite("a.txt", "v1")); + + var item = NewItem(); + await tp.Store.CreateAsync(item); + await ParkAsync(tp, reviews, item); + + var resumed = await ResumeWithVerdictAsync(tp, reviews, questions, item.Id, approved: true, notes: null); + await tp.Pipeline.RunAsync(resumed, CancellationToken.None); + + var final = await tp.Store.GetAsync(item.Id); + Assert.Equal(WorkItemState.Done, final!.State); + // Approve tears the held deployment down immediately: exactly one + // provision, exactly one teardown, nothing live, review consumed. + Assert.Equal(1, manager.StartCount); + Assert.Equal(1, manager.DisposeCount); + Assert.Equal(0, manager.LiveCount); + var consumed = await reviews.TryGetAsync(item.Id.ToString(), 1); + Assert.NotNull(consumed); + Assert.Equal(HumanDeploymentReviewStatus.Approved, consumed!.Status); + Assert.NotNull(consumed.ConsumedAt); + // Approve contributes no findings: the probe never ran a second time. + Assert.Equal([1], probe.SeenIterations); + } + + [Fact] + public async Task Reject_NotesBecomeBlockingFindings_ReworkWithFreshDeployment() + { + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var code = new CodeAuditor(new Queue([new(true, []), new(true, [])])); + var probe = new DeploymentProbeAuditor(new Queue([new(true, []), new(true, [])])); + var manager = new LiveFakeManager(); + var (db, questions, reviews) = Stores(); + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + auditors: [code, probe, HumanAuditor()], + projectAudit: AuditWithDeployment(enabled: true, maxIterations: 2), + deploymentRecipe: Recipe(), + deploymentManager: manager, + deploymentSubstrates: new FakeSubstrates(), + stateDbPathOverride: db, + questionStore: questions, + humanReviewStore: reviews); + tp.Agent.WorkPlan.Enqueue(new FileWrite("a.txt", "v1")); + tp.Agent.WorkPlan.Enqueue(new FileWrite("a.txt", "v2-after-rework")); + + var item = NewItem(); + await tp.Store.CreateAsync(item); + await ParkAsync(tp, reviews, item); + + var resumed = await ResumeWithVerdictAsync( + tp, reviews, questions, item.Id, approved: false, notes: "smoke failed on /health"); + await tp.Pipeline.RunAsync(resumed, CancellationToken.None); + + // Reject is a blocking finding: teardown ran, rework ran, and the + // next iteration provisioned a FRESH deployment and parked again. + Assert.Equal(1, manager.DisposeCount); + Assert.Equal(2, manager.StartCount); + Assert.Equal(1, manager.LiveCount); + var rejected = await tp.Store.GetAsync(item.Id); + Assert.Equal(WorkItemState.NeedsOperatorInput, rejected!.State); + + var first = await reviews.TryGetAsync(item.Id.ToString(), 1); + Assert.NotNull(first); + Assert.Equal(HumanDeploymentReviewStatus.Rejected, first!.Status); + Assert.Equal("smoke failed on /health", first.Notes); + Assert.NotNull(first.ConsumedAt); + var second = await reviews.GetActiveForWorkItemAsync(item.Id.ToString()); + Assert.NotNull(second); + Assert.Equal(2, second!.Iteration); + Assert.Equal(HumanDeploymentReviewStatus.Pending, second.Status); + + // The persisted iteration-1 snapshot carries the blocking verdict. + var iterations = await tp.Store.GetIterationsAsync(item.Id); + var attempt = iterations + .Where(i => i.Iteration == AuditProgressIterationNumbers.WorkPhase) + .OrderByDescending(i => i.DispatchedAt) + .Select(i => (DateTimeOffset?)i.DispatchedAt) + .FirstOrDefault(); + var progress = await ((IAuditProgressStore)tp.Store).GetAuditProgressAsync(item.Id, attempt); + var iter1 = progress.Where(r => r.Iteration == 1).ToList(); + Assert.NotEmpty(iter1); + Assert.Contains( + iter1.SelectMany(r => r.BlockingFindingsDetails), + f => f.Title.Contains("rejected", StringComparison.OrdinalIgnoreCase) + && f.Description.Contains("smoke failed on /health", StringComparison.Ordinal)); + } + + [Fact] + public async Task Expiry_FailsClosedAndTearsDown() + { + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var fakeClock = new ControllableTimeProvider(DateTimeOffset.UtcNow.AddMinutes(5)); + var code = new CodeAuditor(new Queue([new(true, [])])); + var probe = new DeploymentProbeAuditor(new Queue([new(true, [])])); + var manager = new LiveFakeManager(); + var (db, questions, reviews) = Stores(); + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + auditors: [code, probe, HumanAuditor()], + projectAudit: AuditWithDeployment(enabled: true, maxIterations: 1), + deploymentRecipe: Recipe(TimeSpan.FromHours(1)), + deploymentManager: manager, + deploymentSubstrates: new FakeSubstrates(), + stateDbPathOverride: db, + questionStore: questions, + humanReviewStore: reviews, + pipelineOptions: new PipelineOptions + { + SandboxImageReference = "ignored", + AgentAllowedHosts = [], + TimeProvider = fakeClock, + }); + tp.Agent.WorkPlan.Enqueue(new FileWrite("a.txt", "v1")); + + var item = NewItem(); + await tp.Store.CreateAsync(item); + await ParkAsync(tp, reviews, item); + + // Silence past the deadline: no verdict recorded, operator never came. + fakeClock.Advance(TimeSpan.FromHours(2)); + var parked = await tp.Store.GetAsync(item.Id); + Assert.True(await tp.Store.TryUpdateIfStateAsync( + parked!.With(WorkItemState.WorkComplete), WorkItemState.NeedsOperatorInput)); + + var resumed = await tp.Store.GetAsync(item.Id); + await tp.Pipeline.RunAsync(resumed!, CancellationToken.None); + + // Fail-closed: blocking 'expired unreviewed' finding, teardown ran, + // the item failed instead of shipping unverified. + var final = await tp.Store.GetAsync(item.Id); + Assert.Equal(WorkItemState.AuditFailed, final!.State); + Assert.Contains("expired unreviewed", final.LastError, StringComparison.OrdinalIgnoreCase); + Assert.Equal(1, manager.DisposeCount); + Assert.Equal(0, manager.LiveCount); + var expired = await reviews.TryGetAsync(item.Id.ToString(), 1); + Assert.NotNull(expired); + Assert.Equal(HumanDeploymentReviewStatus.Expired, expired!.Status); + Assert.NotNull(expired.ConsumedAt); + } + + [Fact] + public async Task Sweeper_ExpiresUndecidedReview_TearsDownAndRequeues() + { + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var fakeClock = new ControllableTimeProvider(DateTimeOffset.UtcNow.AddMinutes(5)); + var code = new CodeAuditor(new Queue([new(true, [])])); + var probe = new DeploymentProbeAuditor(new Queue([new(true, [])])); + var manager = new LiveFakeManager(); + var (db, questions, reviews) = Stores(); + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + auditors: [code, probe, HumanAuditor()], + projectAudit: AuditWithDeployment(enabled: true, maxIterations: 1), + deploymentRecipe: Recipe(TimeSpan.FromHours(1)), + deploymentManager: manager, + deploymentSubstrates: new FakeSubstrates(), + stateDbPathOverride: db, + questionStore: questions, + humanReviewStore: reviews, + pipelineOptions: new PipelineOptions + { + SandboxImageReference = "ignored", + AgentAllowedHosts = [], + TimeProvider = fakeClock, + }); + tp.Agent.WorkPlan.Enqueue(new FileWrite("a.txt", "v1")); + + var item = NewItem(); + await tp.Store.CreateAsync(item); + var review = await ParkAsync(tp, reviews, item); + + fakeClock.Advance(TimeSpan.FromHours(2)); + var sweeper = new HumanDeploymentReviewSweeper( + reviews, manager, tp.Store, questions, tp.Queue, null, + () => new HumanDeploymentReviewOptions(), + fakeClock, + NullLogger.Instance); + var swept = await sweeper.SweepOnceAsync(CancellationToken.None); + + Assert.Equal(1, swept); + Assert.Equal(1, manager.DisposeCount); + Assert.Equal(0, manager.LiveCount); + var expired = await reviews.TryGetAsync(item.Id.ToString(), 1); + Assert.Equal(HumanDeploymentReviewStatus.Expired, expired!.Status); + var qs = await questions.ListByWorkItemAsync(item.Id.ToString()); + Assert.All(qs, q => Assert.NotEqual("open", q.State)); + var resumed = await tp.Store.GetAsync(item.Id); + Assert.Equal(WorkItemState.WorkComplete, resumed!.State); + var dequeued = await tp.Queue.DequeueAsync(CancellationToken.None); + Assert.Equal(item.Id, dequeued!.Value); + + // A verdict that raced the sweep keeps its authority: the sweep is a + // CAS no-op for decided reviews. + var secondSweep = await sweeper.SweepOnceAsync(CancellationToken.None); + Assert.Equal(0, secondSweep); + Assert.Equal(review.DeploymentId, expired.DeploymentId); + } + + [Fact] + public async Task ReparkWhileUndecided_ReusesHeldDeployment() + { + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var code = new CodeAuditor(new Queue([new(true, [])])); + var probe = new DeploymentProbeAuditor(new Queue([new(true, [])])); + var manager = new LiveFakeManager(); + var (db, questions, reviews) = Stores(); + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + auditors: [code, probe, HumanAuditor()], + projectAudit: AuditWithDeployment(enabled: true, maxIterations: 1), + deploymentRecipe: Recipe(), + deploymentManager: manager, + deploymentSubstrates: new FakeSubstrates(), + stateDbPathOverride: db, + questionStore: questions, + humanReviewStore: reviews); + tp.Agent.WorkPlan.Enqueue(new FileWrite("a.txt", "v1")); + + var item = NewItem(); + await tp.Store.CreateAsync(item); + var review = await ParkAsync(tp, reviews, item); + + // Operator heartbeat without a verdict (manual resume, still within + // deadline): the resume re-parks against the SAME live deployment — + // no second provision, no teardown. + var parked = await tp.Store.GetAsync(item.Id); + Assert.True(await tp.Store.TryUpdateIfStateAsync( + parked!.With(WorkItemState.WorkComplete), WorkItemState.NeedsOperatorInput)); + var resumed = await tp.Store.GetAsync(item.Id); + await tp.Pipeline.RunAsync(resumed!, CancellationToken.None); + + var reparked = await tp.Store.GetAsync(item.Id); + Assert.Equal(WorkItemState.NeedsOperatorInput, reparked!.State); + Assert.Equal(1, manager.StartCount); + Assert.Equal(0, manager.DisposeCount); + Assert.Equal(1, manager.LiveCount); + var same = await reviews.GetActiveForWorkItemAsync(item.Id.ToString()); + Assert.Equal(review.DeploymentId, same!.DeploymentId); + // The code stage did not re-run on resume: still one iteration seen. + Assert.Equal([1], code.SeenIterations); + } + + [Fact] + public async Task StaleReviewAcrossWorkAttempts_ExpiresAndAuditsFresh() + { + var seed = await TestSupport.CreateSeedRepoAsync(_workspace); + var code = new CodeAuditor(new Queue([new(true, []), new(true, [])])); + var probe = new DeploymentProbeAuditor(new Queue([new(true, []), new(true, [])])); + var manager = new LiveFakeManager(); + var (db, questions, reviews) = Stores(); + using var tp = TestSupport.BuildPipeline( + _workspace, seed, + auditors: [code, probe, HumanAuditor()], + projectAudit: AuditWithDeployment(enabled: true, maxIterations: 1), + deploymentRecipe: Recipe(), + deploymentManager: manager, + deploymentSubstrates: new FakeSubstrates(), + stateDbPathOverride: db, + questionStore: questions, + humanReviewStore: reviews); + tp.Agent.WorkPlan.Enqueue(new FileWrite("a.txt", "v1")); + + var item = NewItem(); + await tp.Store.CreateAsync(item); + var first = await ParkAsync(tp, reviews, item); + + // A retry re-ran the work phase after the park: the reviewed code may + // be gone, so the verdict must never apply. The resume expires the + // stale review fail-closed and audits the fresh tree instead. + await tp.Store.RecordIterationDispatchAsync( + item.Id, AuditProgressIterationNumbers.WorkPhase, 0, + DateTimeOffset.UtcNow.AddHours(1)); + var parked = await tp.Store.GetAsync(item.Id); + Assert.True(await tp.Store.TryUpdateIfStateAsync( + parked!.With(WorkItemState.WorkComplete), WorkItemState.NeedsOperatorInput)); + var resumed = await tp.Store.GetAsync(item.Id); + await tp.Pipeline.RunAsync(resumed!, CancellationToken.None); + + Assert.Equal(1, manager.DisposeCount); + Assert.Equal(2, manager.StartCount); + // The stale review was retired fail-closed (its deployment torn + // down) and replaced by a fresh iteration-1 review against the new + // deployment: the code stage re-ran (two code passes seen) and no + // verdict crossed the work-attempt boundary. + Assert.Equal([1, 1], code.SeenIterations); + var fresh = await reviews.GetActiveForWorkItemAsync(item.Id.ToString()); + Assert.NotNull(fresh); + Assert.Equal(HumanDeploymentReviewStatus.Pending, fresh!.Status); + Assert.NotEqual(first.DeploymentId, fresh.DeploymentId); + var reparked = await tp.Store.GetAsync(item.Id); + Assert.Equal(WorkItemState.NeedsOperatorInput, reparked!.State); + } +} diff --git a/tests/CodeyBox.Tests/TestSupport.cs b/tests/CodeyBox.Tests/TestSupport.cs index e9a342c14..afe216fac 100644 --- a/tests/CodeyBox.Tests/TestSupport.cs +++ b/tests/CodeyBox.Tests/TestSupport.cs @@ -220,6 +220,8 @@ public static TestPipeline BuildPipeline( DeploymentRecipe? deploymentRecipe = null, IDeploymentManager? deploymentManager = null, IDeploymentSubstrateProvider? deploymentSubstrates = null, + IWorkItemQuestionStore? questionStore = null, + IHumanDeploymentReviewStore? humanReviewStore = null, StalePullRequestSweeperOptions? staleBaseReworkOptions = null) { var gitRoot = Path.Combine(workspace, "repos-" + Guid.NewGuid().ToString("N")[..8]); @@ -418,6 +420,8 @@ public static TestPipeline BuildPipeline( mergeScopeResolver: mergeScopeResolver, deploymentManager: deploymentManager, deploymentSubstrates: deploymentSubstrates, + questionStore: questionStore, + humanReviews: humanReviewStore, staleBaseReworkRouter: staleBaseReworkRouter); return new TestPipeline(