feat(review): a Reviewer agent reviews every change before it is tested

docs/plans/autonomy-levels.md

@@ -13,7 +13,7 @@ Every ticket flows through three phases (owner, 2026-10-21); the knowledge graph
 |---|---|---|---|
 | **1. Agentic Discovery** | idea → research → spec → tickets with acceptance criteria and dependencies | New idea → spec → tickets, dependencies (#240) | a research agent (market / UX / technical), reading the customer's own documents, asking back when the idea is ambiguous |
 | **2. Agentic Coding** | ticket → code → pull request; continues from review and CI | on a computer or the servers, scoped, Stop, live steps, Ask for changes, Fix the failing checks | autopilot over a board in dependency order |
-| **3. Agentic Testing** | accepts a PR against its criteria: unit tests, Playwright on a real stack, data checks, performance → evidence → merge, or back to Coding with findings | done by hand on the lab | the acceptance agent + merge policy |
+| **3. Agentic Review + Testing** | the **Reviewer agent** reads the diff against the ticket and the repo's rules (correctness, security/isolation, tests) → approve, or must-fix findings back to Coding; then the **Testing agent** accepts it against its criteria: unit tests, Playwright on a real stack, data checks → evidence → merge, or back to Coding | Reviewer (#264) + Testing (#244) + Autopilot merge policy, both posting evidence on the PR | review of a cloud worker's change; a person's review folded in |
 
 **Knowledge graph** (ADR-0008, `AgentOs.Modules.Memory`): Discovery records decisions and requirements, Coding records
 changed files, PRs and review lessons, Testing records failures, fixes and evidence — and each phase reads what the
@@ -40,6 +40,8 @@ others learned. The board shows each ticket's phase (Discovery → Coding → Te
    `docs/ACCEPTANCE.md`, run the unit tests, bring the stack up and run the Playwright specs, run data checks
    (e.g. the CBL corpus), and post the evidence on the PR. Pass → approve; fail → *Ask for changes* with the
    findings.
+3b. ✅ (#264) **Reviewer agent.** Between green CI and acceptance testing: a read-only agent reviews the diff in
+   context and posts its review on the PR; must-fix findings go back to the Coding agent (same 3 bounded rounds).
 4. ✅ (#242, *Build and merge*: on green CI; acceptance agent next) **Merge policy.** Per project: merge on green + acceptance pass (squash, delete branch), or wait for a person.
 5. **Environment care.** The known stack landmines (stale Keycloak/Postgres volumes, orphaned containers, ports)
    handled by the acceptance agent's stack step, not by a person.

src/AgentOs.Domain/Sessions/IReviewAgent.cs

@@ -0,0 +1,30 @@
+// Agentic review: before a proposed change is tested, a second agent reads its diff against the ticket and either
+// approves it or asks the Coding agent for changes — the code review a person would otherwise do on the PR.
+
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace AgentOs.Domain.Sessions;
+
+/// <summary>One review finding. <see cref="Severity"/> is <c>must-fix</c> (blocks the change), <c>should-fix</c> or <c>nit</c>.</summary>
+public sealed record ReviewFinding(string Severity, string File, int? Line, string Comment)
+{
+    /// <summary>True for a finding that blocks the change.</summary>
+    public bool Blocks => Severity == "must-fix";
+}
+
+/// <summary>The Reviewer agent's verdict. <see cref="Ran"/> is false when it could not review at all (no computer, …).</summary>
+public sealed record ReviewVerdict(bool Ran, bool Approved, IReadOnlyList<ReviewFinding> Findings, string Summary, string? Error)
+{
+    /// <summary>The findings that block the change.</summary>
+    public IEnumerable<ReviewFinding> Blocking => Findings.Where(f => f.Blocks);
+}
+
+/// <summary>Reviews a proposed change on a paired computer, read-only: it never edits, commits or pushes.</summary>
+public interface IReviewAgent
+{
+    /// <summary>Review the change on <c>request.Repo.ExistingBranch</c> against its ticket.</summary>
+    Task<ReviewVerdict> RunAsync(AcceptanceRequest request, CancellationToken ct = default);
+}

src/AgentOs.Modules.Pipeline/Agents/DependencyInjection.cs

@@ -64,6 +64,7 @@ public static IServiceCollection AddAgents(this IServiceCollection services, ICo
         services.AddTransient<IRequirementAgent, RequirementAgent>();
         services.AddTransient<AgentOs.Domain.Sessions.IIssueWorkAgent, IssueWorkAgent>();
         services.AddTransient<AgentOs.Domain.Sessions.IAcceptanceAgent, AcceptanceAgent>();
+        services.AddTransient<AgentOs.Domain.Sessions.IReviewAgent, ReviewAgent>();
         services.AddTransient<ICodingAgent, CodingAgent>();
         services.AddTransient<ITestingAgent, TestingAgent>();
         services.AddTransient<IQaAgent, QaAgent>();

src/AgentOs.Modules.Pipeline/Agents/ReviewAgent.cs

@@ -0,0 +1,177 @@
+// Agentic review — the Reviewer agent. Between the Coding agent and the Testing agent: it reads the proposed change's
+// diff against its ticket and the repository's own rules, the way a careful reviewer would, and either approves it or
+// returns must-fix findings that go back to the Coding agent. It runs on the member's paired computer under scoped
+// autonomy (it needs the checkout to read code in context and may build) and never edits, commits or pushes.
+
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Linq;
+using System.Text.Json;
+using System.Threading;
+using System.Threading.Tasks;
+using AgentOs.Domain.Llm;
+using AgentOs.Domain.Sessions;
+using Microsoft.Extensions.Logging;
+
+namespace AgentOs.Modules.Pipeline.Agents;
+
+/// <inheritdoc cref="IReviewAgent"/>
+public sealed class ReviewAgent(ILlmClientFactory factory, ILogger<ReviewAgent> logger, ISessionRunFeed? feed = null) : IReviewAgent
+{
+    // Reading a diff in context is quicker than testing it; a build fits too.
+    private static readonly TimeSpan RunTimeout = TimeSpan.FromMinutes(20);
+
+    /// <inheritdoc />
+    public async Task<ReviewVerdict> RunAsync(AcceptanceRequest request, CancellationToken ct = default)
+    {
+        ArgumentNullException.ThrowIfNull(request);
+        if (string.IsNullOrWhiteSpace(request.Repo.ExistingBranch))
+        {
+            return NotRun("There is no proposed change (branch) to review.");
+        }
+        var repoLabel = $"{request.Repo.Owner}/{request.Repo.Repo}";
+        Emit(request, SessionRunEventKind.Running, $"Reviewing #{request.IssueNumber} on your computer.", repoLabel);
+
+        ILlmClient llm;
+        try { llm = factory.Create("RemoteAgent"); }
+        catch (Exception ex) when (ex is LlmException or ArgumentException or NotSupportedException or InvalidOperationException)
+        {
+            return NotRun($"No computer to review on: {ex.Message}");
+        }
+
+        var llmRequest = new LlmRequest(
+            SystemPrompt: Instructions(request) + "\n\n" + UntrustedText.SystemRule,
+            UserPrompt: AcceptanceAgent.User(request).Replace("Test the change against these criteria and reply with the JSON verdict.",
+                "Review the change against this ticket and reply with the JSON verdict.", StringComparison.Ordinal),
+            Model: "claude-sonnet-4-6",
+            MaxTokens: 32768,
+            Timeout: RunTimeout,
+            Cli: request.CliProfile)
+        {
+            Autonomy = WorkspaceAutonomy.Scoped,
+            Progress = new InlineProgress<string>(step =>
+            {
+                if (TaskSteps.LabelFor(step) is { } label) { Emit(request, SessionRunEventKind.Step, "Reviewing — " + label, repoLabel); }
+            }),
+        };
+        try
+        {
+            var response = await llm.SendAsync(llmRequest, ct).ConfigureAwait(false);
+            return Parse(response.Content ?? string.Empty);
+        }
+        catch (Exception ex) when (ex is LlmException or TimeoutException or InvalidOperationException or System.Net.Http.HttpRequestException)
+        {
+            logger.LogWarning(ex, "Review run failed for session {SessionId}", request.SessionId);
+            return NotRun(ex.Message);
+        }
+    }
+
+    private static ReviewVerdict NotRun(string why) => new(false, false, [], why, why);
+
+    private void Emit(AcceptanceRequest r, SessionRunEventKind kind, string message, string repo) =>
+        feed?.Publish(new SessionRunEvent(r.TenantId, r.SessionId, kind, message, DateTimeOffset.UtcNow, repo));
+
+    internal static string Instructions(AcceptanceRequest r) => string.Format(
+        CultureInfo.InvariantCulture,
+        """
+        You are the Reviewer Agent of AgentOS.NET. You review a proposed change before it is tested, the way a careful
+        senior reviewer would on its pull request, on the developer's own machine with your native shell and file tools.
+        You NEVER change the code: do not edit tracked files, do not commit, do not push, do not open or comment on pull
+        requests.
+
+        1. Get the change. The runner starts you in a fresh, empty directory made for this run:
+           git clone https://github.com/{0}/{1}.git .
+           git fetch origin {2}
+           git checkout {2}
+           git diff origin/{3}...HEAD
+        2. Read the ticket (user message) and the repository's rules (AGENTS.md, CLAUDE.md, README, docs it points to).
+        3. Review the diff in context — open the files around each change. Look for, in this order:
+           - correctness: does it do what the ticket asks; broken edge cases; wrong logic; regressions;
+           - security and data isolation: secrets in code, injection, missing authorization, cross-tenant/school reads;
+           - tests: is the new behaviour covered; are tests meaningful (not asserting nothing);
+           - the repository's own conventions and rules (language, structure, naming) where it states them.
+           You may build the project to confirm a suspicion. Do not run the full end-to-end stack — testing comes next.
+        4. Grade each finding:
+           - "must-fix": a real defect, security/isolation hole, missing behaviour the ticket requires, or a broken
+             repository rule. Only these block the change.
+           - "should-fix": a clear improvement that is not a defect.
+           - "nit": style or taste. Keep these few.
+           Do not invent problems; an approval with no findings is a good outcome.
+
+        Reply ONLY with this JSON (no other text):
+        {{"verdict":"approve","findings":[{{"severity":"must-fix","file":"<path>","line":42,"comment":"<what is wrong and what to do>"}}],"summary":"<one or two sentences>"}}
+        "verdict" is "approve" when there is no must-fix finding, otherwise "changes".
+        """,
+        r.Repo.Owner, r.Repo.Repo, r.Repo.ExistingBranch, r.Repo.DefaultBranch);
+
+    /// <summary>Reads the agent's JSON verdict. An approval is only trusted when no finding is must-fix.</summary>
+    internal static ReviewVerdict Parse(string content)
+    {
+        var start = content.LastIndexOf("{\"verdict\"", StringComparison.Ordinal);
+        if (start < 0) { start = content.IndexOf('{', StringComparison.Ordinal); }
+        var end = content.LastIndexOf('}');
+        if (start < 0 || end <= start) { return NotRun("The Reviewer agent did not return a verdict."); }
+        try
+        {
+            using var doc = JsonDocument.Parse(content[start..(end + 1)]);
+            var root = doc.RootElement;
+            var findings = root.TryGetProperty("findings", out var fs) && fs.ValueKind == JsonValueKind.Array
+                ? fs.EnumerateArray().Select(f => new ReviewFinding(
+                    Severity(Str(f, "severity")), Str(f, "file"),
+                    f.TryGetProperty("line", out var l) && l.ValueKind == JsonValueKind.Number && l.TryGetInt32(out var n) && n > 0 ? n : null,
+                    Str(f, "comment"))).Where(f => f.Comment.Length > 0).ToList()
+                : [];
+            var claimed = string.Equals(Str(root, "verdict"), "approve", StringComparison.OrdinalIgnoreCase);
+            return new ReviewVerdict(true, claimed && !findings.Any(f => f.Blocks), findings, Str(root, "summary"), null);
+        }
+        catch (JsonException)
+        {
+            return NotRun("The Reviewer agent's verdict was not valid JSON.");
+        }
+
+        static string Str(JsonElement e, string name) =>
+            e.TryGetProperty(name, out var v) && v.ValueKind == JsonValueKind.String ? v.GetString() ?? "" : "";
+
+        static string Severity(string s) => s.Trim().ToLowerInvariant() switch
+        {
+            "must-fix" or "must" or "blocker" => "must-fix",
+            "should-fix" or "should" => "should-fix",
+            _ => "nit",
+        };
+    }
+
+    /// <summary>The review as a pull-request comment (Markdown).</summary>
+    public static string Comment(int issueNumber, ReviewVerdict verdict)
+    {
+        ArgumentNullException.ThrowIfNull(verdict);
+        var sb = new System.Text.StringBuilder();
+        sb.Append(verdict.Approved ? "### ✅ AgentOS.NET review — approved" : "### 🔁 AgentOS.NET review — changes requested")
+          .Append(" (#").Append(issueNumber.ToString(CultureInfo.InvariantCulture)).Append(")\n\n");
+        if (!string.IsNullOrWhiteSpace(verdict.Summary)) { sb.Append(verdict.Summary.Trim()).Append("\n\n"); }
+        if (verdict.Findings.Count > 0)
+        {
+            sb.Append("| Severity | Where | Finding |\n|---|---|---|\n");
+            foreach (var f in verdict.Findings.OrderBy(f => f.Severity switch { "must-fix" => 0, "should-fix" => 1, _ => 2 }))
+            {
+                sb.Append("| ").Append(f.Severity).Append(" | `").Append(Cell(Where(f))).Append("` | ").Append(Cell(f.Comment)).Append(" |\n");
+            }
+        }
+        sb.Append("\n<sub>Run by the AgentOS.NET Reviewer agent on a paired computer. It does not change the code; must-fix findings go back to the Coding agent.</sub>\n");
+        return sb.ToString();
+
+        static string Cell(string s) => s.Replace("|", "\\|", StringComparison.Ordinal).Replace("\n", " ", StringComparison.Ordinal).Trim();
+    }
+
+    /// <summary>The must-fix findings, as the change request sent back to the Coding agent.</summary>
+    public static string ChangeRequest(ReviewVerdict verdict)
+    {
+        ArgumentNullException.ThrowIfNull(verdict);
+        var lines = verdict.Blocking.Select(f => $"- {Where(f)}: {f.Comment}").Prepend(verdict.Summary);
+        return "Code review asked for changes. Fix every must-fix finding (or explain in the pull request why it is not a defect):\n"
+            + string.Join("\n", lines.Where(l => !string.IsNullOrWhiteSpace(l)));
+    }
+
+    private static string Where(ReviewFinding f) =>
+        string.IsNullOrWhiteSpace(f.File) ? "(general)" : f.Line is { } line ? $"{f.File}:{line.ToString(CultureInfo.InvariantCulture)}" : f.File;
+}

src/AgentOs.Modules.Sessions/Persistence/Repositories/SessionRepository.cs

@@ -230,7 +230,8 @@ public async Task<bool> RecomputeSessionStatusForTenantAsync(string tenantId, Gu
             : repos.Any(r => r.Status == "Blocked") ? "Blocked"   // QA gate refused the PR (plan 0.5)
             : repos.All(r => r.Status == "Merged") ? "Merged"               // every proposed change is in
             // Done = every repo has a proposed change, tested or not (Accepted / Rejected by the Testing agent).
-            : repos.All(r => r.Status is "Done" or "Merged" or "Accepted" or "Rejected") ? "Done"
+            // (or reviewed: Reviewed / ChangesRequested by the Reviewer agent).
+            : repos.All(r => r.Status is "Done" or "Merged" or "Accepted" or "Rejected" or "Reviewed" or "ChangesRequested") ? "Done"
             : "Running";
 
         // Mirror the first PR + first error onto the parent for back-compat display.

src/AgentOs.Web/Components/Pages/BoardApp.razor

@@ -2039,6 +2039,9 @@ else if (_tab == Tab.Sessions)
         "Blocked" => "Needs your review",   // the QA gate refused to open the change
         "Done" => "Ready for review",
         "Merged" => "Done",
+        "Reviewing" => "Being reviewed",
+        "Reviewed" => "Review passed",
+        "ChangesRequested" => "Review asked for changes",
         "Testing" => "Being tested",
         "Accepted" => "Passed testing",
         "Rejected" => "Failed testing",